From 20df215e637491b366f80db325bc36da7531fbd3 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 9 Sep 2024 10:46:04 -0600 Subject: [PATCH 01/20] add generation field to process --- .../Core/ApplicationPool/Group/InternalUtils.cpp | 4 ++-- src/agent/Core/ApplicationPool/Process.h | 11 ++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp b/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp index 034e43ce31..d8250332ad 100644 --- a/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp +++ b/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp @@ -178,7 +178,7 @@ Group::createNullProcessObject() { LockGuard l(context->memoryManagementSyncher); Process *process = context->processObjectPool.malloc(); Guard guard(context, process); - process = new (process) Process(&info, args); + process = new (process) Process(&info, info.group->restartsInitiated, args); process->shutdownNotRequired(); guard.clear(); return ProcessPtr(process, false); @@ -215,7 +215,7 @@ Group::createProcessObject(const SpawningKit::Spawner &spawner, LockGuard l(context->memoryManagementSyncher); Process *process = context->processObjectPool.malloc(); Guard guard(context, process); - process = new (process) Process(&info, spawnResult, args); + process = new (process) Process(&info, info.group->restartsInitiated, spawnResult, args); guard.clear(); return ProcessPtr(process, false); } diff --git a/src/agent/Core/ApplicationPool/Process.h b/src/agent/Core/ApplicationPool/Process.h index cd91f2e0e9..c5ad2700ae 100644 --- a/src/agent/Core/ApplicationPool/Process.h +++ b/src/agent/Core/ApplicationPool/Process.h @@ -384,6 +384,10 @@ class Process { /** Last time when a session was opened for this Process. */ unsigned long long lastUsed; + /** Which gereration of app processes this one belongs to, + inherited from the app group, incremented when a restart + is initiated*/ + const unsigned int generation; /** Number of sessions currently open. * @invariant session >= 0 */ @@ -446,8 +450,7 @@ class Process { /** Collected by Pool::collectAnalytics(). */ ProcessMetrics metrics; - - Process(const BasicGroupInfo *groupInfo, const Json::Value &args) + Process(const BasicGroupInfo *groupInfo, const unsigned int gen, const Json::Value &args) : info(this, groupInfo, args), socketsAcceptingHttpRequestsCount(0), spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), @@ -458,6 +461,7 @@ class Process { refcount(1), index(-1), lastUsed(spawnEndTime), + generation(gen), sessions(0), processed(0), lifeStatus(ALIVE), @@ -471,7 +475,7 @@ class Process { indexSocketsAcceptingHttpRequests(); } - Process(const BasicGroupInfo *groupInfo, const SpawningKit::Result &skResult, + Process(const BasicGroupInfo *groupInfo, const unsigned int gen, const SpawningKit::Result &skResult, const Json::Value &args) : info(this, groupInfo, skResult), socketsAcceptingHttpRequestsCount(0), @@ -483,6 +487,7 @@ class Process { refcount(1), index(-1), lastUsed(spawnEndTime), + generation(gen), sessions(0), processed(0), lifeStatus(ALIVE), From 744c2b289330fba25332cdfc2c832248dadf3122 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 9 Sep 2024 11:18:28 -0600 Subject: [PATCH 02/20] use process generation to inform routing --- .../Group/ProcessListManagement.cpp | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 94438ff766..88584b1b75 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -61,14 +61,20 @@ Process * Group::findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const { int leastBusyProcessIndex = -1; int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(); + unsigned int i, size = enabledProcessBusynessLevels.size(), highest_gen = 0; const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; for (i = 0; i < size; i++) { Process *process = enabledProcesses[i].get(); + highest_gen = max(highest_gen, process->generation); if (process->getStickySessionId() == id) { return process; - } else if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness) { + } else if (leastBusyProcessIndex == -1 || + enabledProcessBusynessLevels[i] < lowestBusyness || + (enabledProcessBusynessLevels[i] == lowestBusyness && process->generation > highest_gen)) { + if (process->generation > highest_gen) { + highest_gen = process->generation; + } leastBusyProcessIndex = i; lowestBusyness = enabledProcessBusynessLevels[i]; } @@ -88,13 +94,19 @@ Group::findProcessWithLowestBusyness(const ProcessList &processes) const { } int lowestBusyness = -1; + unsigned int highest_gen = 0; Process *leastBusyProcess = NULL; ProcessList::const_iterator it; ProcessList::const_iterator end = processes.end(); for (it = processes.begin(); it != end; it++) { Process *process = (*it).get(); int busyness = process->busyness(); - if (lowestBusyness == -1 || lowestBusyness > busyness) { + if (lowestBusyness == -1 || + lowestBusyness > busyness || + (busyness == lowestBusyness && process->generation > highest_gen)) { + if (process->generation > highest_gen) { + highest_gen = process->generation; + } lowestBusyness = busyness; leastBusyProcess = process; } @@ -113,11 +125,13 @@ Group::findEnabledProcessWithLowestBusyness() const { int leastBusyProcessIndex = -1; int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(); + unsigned int i, size = enabledProcessBusynessLevels.size(), highest_gen = 0; const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; for (i = 0; i < size; i++) { - if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness) { + if (leastBusyProcessIndex == -1 || + enabledProcessBusynessLevels[i] < lowestBusyness || + (enabledProcessBusynessLevels[i] == lowestBusyness && enabledProcesses[i]->generation > highest_gen)) { leastBusyProcessIndex = i; lowestBusyness = enabledProcessBusynessLevels[i]; } From b5dd97a9a981bd3395c018676e9c3fb3a0f6573e Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 9 Sep 2024 14:40:30 -0600 Subject: [PATCH 03/20] missed changes --- .../Core/ApplicationPool/Group/ProcessListManagement.cpp | 3 +++ test/cxx/Core/ApplicationPool/ProcessTest.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 88584b1b75..c3a1aa5d46 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -132,6 +132,9 @@ Group::findEnabledProcessWithLowestBusyness() const { if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness || (enabledProcessBusynessLevels[i] == lowestBusyness && enabledProcesses[i]->generation > highest_gen)) { + if (enabledProcesses[i]->generation > highest_gen) { + highest_gen = enabledProcesses[i]->generation; + } leastBusyProcessIndex = i; lowestBusyness = enabledProcessBusynessLevels[i]; } diff --git a/test/cxx/Core/ApplicationPool/ProcessTest.cpp b/test/cxx/Core/ApplicationPool/ProcessTest.cpp index 6e6518677b..b9d6c0727b 100644 --- a/test/cxx/Core/ApplicationPool/ProcessTest.cpp +++ b/test/cxx/Core/ApplicationPool/ProcessTest.cpp @@ -118,8 +118,10 @@ namespace tut { args["spawner_creation_time"] = 0; - ProcessPtr process(context.processObjectPool.construct( - &groupInfo, result, args), false); + Process *p = context.processObjectPool.malloc(); + p = new (p) Process(&groupInfo, 0, result, args); + ProcessPtr process(p, false); + process->shutdownNotRequired(); return process; } From df6f2ac3bd0abb25007b8504cad59f521cc80a69 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Sun, 22 Sep 2024 13:58:35 -0600 Subject: [PATCH 04/20] address feedback --- .../Group/ProcessListManagement.cpp | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index c3a1aa5d46..72124c7530 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -61,20 +61,17 @@ Process * Group::findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const { int leastBusyProcessIndex = -1; int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(), highest_gen = 0; + unsigned int i, size = enabledProcessBusynessLevels.size(), highestGeneration = 0; const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; for (i = 0; i < size; i++) { Process *process = enabledProcesses[i].get(); - highest_gen = max(highest_gen, process->generation); if (process->getStickySessionId() == id) { return process; - } else if (leastBusyProcessIndex == -1 || - enabledProcessBusynessLevels[i] < lowestBusyness || - (enabledProcessBusynessLevels[i] == lowestBusyness && process->generation > highest_gen)) { - if (process->generation > highest_gen) { - highest_gen = process->generation; - } + } else if (leastBusyProcessIndex == -1 || + enabledProcessBusynessLevels[i] < lowestBusyness || + (enabledProcessBusynessLevels[i] == lowestBusyness && process->generation > highestGeneration)) { + highestGeneration = std::max(highestGeneration, process->generation); leastBusyProcessIndex = i; lowestBusyness = enabledProcessBusynessLevels[i]; } @@ -94,7 +91,7 @@ Group::findProcessWithLowestBusyness(const ProcessList &processes) const { } int lowestBusyness = -1; - unsigned int highest_gen = 0; + unsigned int highestGeneration = 0; Process *leastBusyProcess = NULL; ProcessList::const_iterator it; ProcessList::const_iterator end = processes.end(); @@ -103,10 +100,8 @@ Group::findProcessWithLowestBusyness(const ProcessList &processes) const { int busyness = process->busyness(); if (lowestBusyness == -1 || lowestBusyness > busyness || - (busyness == lowestBusyness && process->generation > highest_gen)) { - if (process->generation > highest_gen) { - highest_gen = process->generation; - } + (busyness == lowestBusyness && process->generation > highestGeneration)) { + highestGeneration = std::max(highestGeneration, process->generation); lowestBusyness = busyness; leastBusyProcess = process; } @@ -125,16 +120,14 @@ Group::findEnabledProcessWithLowestBusyness() const { int leastBusyProcessIndex = -1; int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(), highest_gen = 0; + unsigned int i, size = enabledProcessBusynessLevels.size(), highestGeneration = 0; const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; for (i = 0; i < size; i++) { if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness || - (enabledProcessBusynessLevels[i] == lowestBusyness && enabledProcesses[i]->generation > highest_gen)) { - if (enabledProcesses[i]->generation > highest_gen) { - highest_gen = enabledProcesses[i]->generation; - } + (enabledProcessBusynessLevels[i] == lowestBusyness && enabledProcesses[i]->generation > highestGeneration)) { + highestGeneration = std::max(highestGeneration, enabledProcesses[i]->generation); leastBusyProcessIndex = i; lowestBusyness = enabledProcessBusynessLevels[i]; } From 38187b22dd1cbd6f1fb1e34a4933e2c6281d2f59 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Sun, 22 Sep 2024 14:40:02 -0600 Subject: [PATCH 05/20] add a test to process test suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit not that there’s a point in this test really --- test/cxx/Core/ApplicationPool/ProcessTest.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/cxx/Core/ApplicationPool/ProcessTest.cpp b/test/cxx/Core/ApplicationPool/ProcessTest.cpp index b9d6c0727b..e90f2fb1a0 100644 --- a/test/cxx/Core/ApplicationPool/ProcessTest.cpp +++ b/test/cxx/Core/ApplicationPool/ProcessTest.cpp @@ -14,6 +14,7 @@ namespace tut { SpawningKit::Context skContext; Context context; BasicGroupInfo groupInfo; + unsigned int generation; vector sockets; Pipe stdinFd, stdoutAndErrFd; FileDescriptor server1, server2, server3; @@ -34,6 +35,8 @@ namespace tut { groupInfo.group = NULL; groupInfo.name = "test"; + generation = 0; + struct sockaddr_in addr; socklen_t len = sizeof(addr); SpawningKit::Result::Socket socket; @@ -119,7 +122,7 @@ namespace tut { args["spawner_creation_time"] = 0; Process *p = context.processObjectPool.malloc(); - p = new (p) Process(&groupInfo, 0, result, args); + p = new (p) Process(&groupInfo, generation, result, args); ProcessPtr process(p, false); process->shutdownNotRequired(); @@ -240,4 +243,15 @@ namespace tut { && contents.find("stdout and err 4\n") != string::npos; ); } + + TEST_METHOD(6) { + set_test_name("Test generation is inherited from pool at construction time"); + // this test is pointless b/c the process is made at line 123 of this file, not in the regular codebase + ProcessPtr process1 = createProcess(); + ensure_equals(process1->generation, generation); + generation++; + ProcessPtr process2 = createProcess(); + ensure_equals(process2->generation, generation); + ensure(process1->generation != process2->generation); + } } From a376e8eb6317565626ed60ce385c16507ab1fb82 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 23 Sep 2024 14:58:53 -0600 Subject: [PATCH 06/20] add another test --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 32 +++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index c2bc70a65d..a4be14c735 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -632,6 +631,37 @@ namespace tut { ensure_equals(pool->getGroupCount(), 0u); } + TEST_METHOD(15) { + // Test that the process generation increments when the group restarts + Options options = createOptions(); + + // Spawn a process and opens a session with it. + pool->setMax(1); + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == 1; + ); + + // Close the session so that the process is now idle. + ProcessPtr process = currentSession->getProcess()->shared_from_this(); + currentSession.reset(); + unsigned int gen1 = process->generation; + + ensure(pool->restartGroupByName(options.appRoot)); + EVENTUALLY(5, + result = pool->getProcessCount() == 1; + ); + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == 2; + ); + + process = currentSession->getProcess()->shared_from_this(); + currentSession.reset(); + unsigned int gen2 = process->generation; + ensure_equals(gen1+1,gen2); + } + TEST_METHOD(17) { // Test that restartGroupByName() spawns more processes to ensure // that minProcesses and other constraints are met. From 824166d14a8fa8fee71bd9c2ca478236db5c9801 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 23 Sep 2024 15:34:09 -0600 Subject: [PATCH 07/20] test stub for proper testing of routing algo --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index a4be14c735..3bb0e4ce44 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -662,6 +662,35 @@ namespace tut { ensure_equals(gen1+1,gen2); } + TEST_METHOD(16) { + // Test that the correct process from the pool is routed + Options options = createOptions(); + ensureMinProcesses(2); + + // async restart the group + ensure(pool->restartGroupByName(options.appRoot)); + ensureMinProcesses(1); + + /* + Imagine we have these processes (ordered from oldest to newest): + + #. PID 1 (generation A, busyness 5) + #. PID 2 (generation A, busyness 3) + #. PID 3 (generation B, busyness 1) + + The algorithm should select PID 3 + */ + + /* + Imagine we have these processes (ordered from oldest to newest): + + #. PID 1 (generation A, busyness 1) + #. PID 2 (generation B, busyness 5) + + The algorithm should select PID 1 + */ + } + TEST_METHOD(17) { // Test that restartGroupByName() spawns more processes to ensure // that minProcesses and other constraints are met. From 60d254b57202cfb8671f7cf20d34d885c45f5035 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Fri, 27 Sep 2024 13:41:12 -0600 Subject: [PATCH 08/20] impl new algo --- src/agent/Core/ApplicationPool/Group.h | 6 +- .../Group/ProcessListManagement.cpp | 94 +++++++++---------- .../Group/SessionManagement.cpp | 8 +- src/agent/Core/ApplicationPool/Process.h | 21 ++--- 4 files changed, 64 insertions(+), 65 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group.h b/src/agent/Core/ApplicationPool/Group.h index fba45f8091..9f7362a1d4 100644 --- a/src/agent/Core/ApplicationPool/Group.h +++ b/src/agent/Core/ApplicationPool/Group.h @@ -223,9 +223,9 @@ class Group: public boost::enable_shared_from_this { /****** Process list management ******/ Process *findProcessWithStickySessionId(unsigned int id) const; - Process *findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const; - Process *findProcessWithLowestBusyness(const ProcessList &processes) const; - Process *findEnabledProcessWithLowestBusyness() const; + Process *findBestProcessPreferringStickySessionId(unsigned int id) const; + Process *findBestProcess(const ProcessList &processes) const; + Process *findBestEnabledProcess() const; void addProcessToList(const ProcessPtr &process, ProcessList &destination); void removeProcessFromList(const ProcessPtr &process, ProcessList &source); diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 72124c7530..0c0544d52f 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -58,81 +58,81 @@ Group::findProcessWithStickySessionId(unsigned int id) const { } Process * -Group::findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const { - int leastBusyProcessIndex = -1; - int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(), highestGeneration = 0; - const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; - - for (i = 0; i < size; i++) { - Process *process = enabledProcesses[i].get(); +Group::findBestProcessPreferringStickySessionId(unsigned int id) const { + Process *bestProcess = nullptr; + ProcessList::const_iterator it; + ProcessList::const_iterator end = enabledProcesses.end(); + for (it = enabledProcesses.begin(); it != end; it++) { + Process *process = (*it).get(); if (process->getStickySessionId() == id) { return process; - } else if (leastBusyProcessIndex == -1 || - enabledProcessBusynessLevels[i] < lowestBusyness || - (enabledProcessBusynessLevels[i] == lowestBusyness && process->generation > highestGeneration)) { - highestGeneration = std::max(highestGeneration, process->generation); - leastBusyProcessIndex = i; - lowestBusyness = enabledProcessBusynessLevels[i]; + } else if (bestProcess == nullptr || + process->generation > bestProcess->generation || + (process->generation == bestProcess->generation && process->spawnerCreationTime < bestProcess->spawnerCreationTime) || + (process->generation == bestProcess->generation && process->spawnerCreationTime == bestProcess->spawnerCreationTime && process->busyness() < bestProcess->busyness()) + ) { + bestProcess = process; } } - - if (leastBusyProcessIndex == -1) { - return NULL; - } else { - return enabledProcesses[leastBusyProcessIndex].get(); - } + return bestProcess; } Process * -Group::findProcessWithLowestBusyness(const ProcessList &processes) const { +Group::findBestProcess(const ProcessList &processes) const { if (processes.empty()) { - return NULL; + return nullptr; } - int lowestBusyness = -1; - unsigned int highestGeneration = 0; - Process *leastBusyProcess = NULL; + Process *bestProcess = nullptr; ProcessList::const_iterator it; ProcessList::const_iterator end = processes.end(); for (it = processes.begin(); it != end; it++) { Process *process = (*it).get(); - int busyness = process->busyness(); - if (lowestBusyness == -1 || - lowestBusyness > busyness || - (busyness == lowestBusyness && process->generation > highestGeneration)) { - highestGeneration = std::max(highestGeneration, process->generation); - lowestBusyness = busyness; - leastBusyProcess = process; + + if (bestProcess == nullptr || + process->generation > bestProcess->generation || + (process->generation == bestProcess->generation && process->spawnerCreationTime < bestProcess->spawnerCreationTime) || + (process->generation == bestProcess->generation && process->spawnerCreationTime == bestProcess->spawnerCreationTime && process->busyness() < bestProcess->busyness()) + ) { + bestProcess = process; } } - return leastBusyProcess; + return bestProcess; } /** - * Cache-optimized version of findProcessWithLowestBusyness() for the common case. + * Cache-optimized version of findBestProcess() for the common case. */ Process * -Group::findEnabledProcessWithLowestBusyness() const { +Group::findBestEnabledProcess() const { if (enabledProcesses.empty()) { - return NULL; + return nullptr; } - int leastBusyProcessIndex = -1; - int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(), highestGeneration = 0; + Process* bestProcess = nullptr; + unsigned long long bestProcessStartTime = 0; + unsigned int bestProcessGen = 0; + int bestProcessBusyness = 0; + unsigned int size = enabledProcessBusynessLevels.size(); const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; - for (i = 0; i < size; i++) { - if (leastBusyProcessIndex == -1 || - enabledProcessBusynessLevels[i] < lowestBusyness || - (enabledProcessBusynessLevels[i] == lowestBusyness && enabledProcesses[i]->generation > highestGeneration)) { - highestGeneration = std::max(highestGeneration, enabledProcesses[i]->generation); - leastBusyProcessIndex = i; - lowestBusyness = enabledProcessBusynessLevels[i]; + for (unsigned int i = 0; i < size; i++) { + Process *process = enabledProcesses.at(i).get(); + unsigned int gen = process->generation; + unsigned long long startTime = process->spawnerCreationTime; + int busyness = enabledProcessBusynessLevels[i]; + if (bestProcess == nullptr || + gen > bestProcess->generation || + (gen == bestProcessGen && startTime < bestProcessStartTime) || + (gen == bestProcessGen && startTime == bestProcessStartTime && busyness < bestProcessBusyness) + ) { + bestProcess = process; + bestProcessGen = gen; + bestProcessBusyness = busyness; + bestProcessStartTime = startTime; } } - return enabledProcesses[leastBusyProcessIndex].get(); + return bestProcess; } /** diff --git a/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp b/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp index a9e9510077..be17dca6c3 100644 --- a/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp @@ -58,14 +58,14 @@ Group::RouteResult Group::route(const Options &options) const { if (OXT_LIKELY(enabledCount > 0)) { if (options.stickySessionId == 0) { - Process *process = findEnabledProcessWithLowestBusyness(); + Process *process = findBestEnabledProcess(); if (process->canBeRoutedTo()) { return RouteResult(process); } else { return RouteResult(NULL, true); } } else { - Process *process = findProcessWithStickySessionIdOrLowestBusyness( + Process *process = findBestProcessPreferringStickySessionId( options.stickySessionId); if (process != NULL) { if (process->canBeRoutedTo()) { @@ -78,7 +78,7 @@ Group::route(const Options &options) const { } } } else { - Process *process = findProcessWithLowestBusyness(disablingProcesses); + Process *process = findBestProcess(disablingProcesses); if (process->canBeRoutedTo()) { return RouteResult(process); } else { @@ -304,7 +304,7 @@ Group::get(const Options &newOptions, const GetCallback &callback, assert(m_spawning || restarting() || poolAtFullCapacity()); if (disablingCount > 0 && !restarting()) { - Process *process = findProcessWithLowestBusyness(disablingProcesses); + Process *process = findBestProcess(disablingProcesses); assert(process != NULL); if (!process->isTotallyBusy()) { return newSession(process, newOptions.currentTime); diff --git a/src/agent/Core/ApplicationPool/Process.h b/src/agent/Core/ApplicationPool/Process.h index c5ad2700ae..611a384fd5 100644 --- a/src/agent/Core/ApplicationPool/Process.h +++ b/src/agent/Core/ApplicationPool/Process.h @@ -28,7 +28,6 @@ #include #include -#include #include #include #include @@ -102,6 +101,12 @@ class Process { public: static const unsigned int MAX_SOCKETS_ACCEPTING_HTTP_REQUESTS = 3; + /** + * Time at which the Spawner that created this process was created. + * Microseconds resolution. + */ + unsigned long long spawnerCreationTime; + private: /************************************************************* * Read-only fields, set once during initialization and never @@ -141,12 +146,6 @@ class Process { */ StaticString codeRevision; - /** - * Time at which the Spawner that created this process was created. - * Microseconds resolution. - */ - unsigned long long spawnerCreationTime; - /** Time at which we started spawning this process. Microseconds resolution. */ unsigned long long spawnStartTime; @@ -451,9 +450,9 @@ class Process { ProcessMetrics metrics; Process(const BasicGroupInfo *groupInfo, const unsigned int gen, const Json::Value &args) - : info(this, groupInfo, args), + : spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), + info(this, groupInfo, args), socketsAcceptingHttpRequestsCount(0), - spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), spawnStartTime(getJsonUint64Field(args, "spawn_start_time")), spawnEndTime(SystemTime::getUsec()), type(args["type"] == "dummy" ? SpawningKit::Result::DUMMY : SpawningKit::Result::UNKNOWN), @@ -477,9 +476,9 @@ class Process { Process(const BasicGroupInfo *groupInfo, const unsigned int gen, const SpawningKit::Result &skResult, const Json::Value &args) - : info(this, groupInfo, skResult), + : spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), + info(this, groupInfo, skResult), socketsAcceptingHttpRequestsCount(0), - spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), spawnStartTime(skResult.spawnStartTime), spawnEndTime(skResult.spawnEndTime), type(skResult.type), From 6b739f36bda65b11e93800735139227fe7979e6c Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Sun, 29 Sep 2024 22:31:37 -0600 Subject: [PATCH 09/20] update changelog [ci skip] [ci:skip] --- CHANGELOG | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 3e1ee91260..f322903a7f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Release 6.0.24 (Not yet released) ------------- - * + * Update the order Passenger routes requests to app processes. Processes are now chosen based on being in the latest generation (Enterprise), then by newest process, then by oldest, then by +busyness. Closes GH-2551. Release 6.0.23 From fd217749bbdd2c77b694c4ccc0e8bc4aba4da6d0 Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Thu, 3 Oct 2024 15:51:20 +0200 Subject: [PATCH 10/20] Fix indentation --- .../Core/ApplicationPool/Group/ProcessListManagement.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 0c0544d52f..5d5e304bfa 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -66,11 +66,11 @@ Group::findBestProcessPreferringStickySessionId(unsigned int id) const { Process *process = (*it).get(); if (process->getStickySessionId() == id) { return process; - } else if (bestProcess == nullptr || - process->generation > bestProcess->generation || + } else if (bestProcess == nullptr || + process->generation > bestProcess->generation || (process->generation == bestProcess->generation && process->spawnerCreationTime < bestProcess->spawnerCreationTime) || (process->generation == bestProcess->generation && process->spawnerCreationTime == bestProcess->spawnerCreationTime && process->busyness() < bestProcess->busyness()) - ) { + ) { bestProcess = process; } } @@ -93,7 +93,7 @@ Group::findBestProcess(const ProcessList &processes) const { process->generation > bestProcess->generation || (process->generation == bestProcess->generation && process->spawnerCreationTime < bestProcess->spawnerCreationTime) || (process->generation == bestProcess->generation && process->spawnerCreationTime == bestProcess->spawnerCreationTime && process->busyness() < bestProcess->busyness()) - ) { + ) { bestProcess = process; } } From c0d99a6ef4081e8a96cd4d78e7a8ca382c48f05f Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Thu, 3 Oct 2024 16:42:34 +0200 Subject: [PATCH 11/20] Remove the new test in ProcessTest.cpp because it's pointless --- test/cxx/Core/ApplicationPool/ProcessTest.cpp | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/test/cxx/Core/ApplicationPool/ProcessTest.cpp b/test/cxx/Core/ApplicationPool/ProcessTest.cpp index e90f2fb1a0..a3d508cd06 100644 --- a/test/cxx/Core/ApplicationPool/ProcessTest.cpp +++ b/test/cxx/Core/ApplicationPool/ProcessTest.cpp @@ -14,7 +14,6 @@ namespace tut { SpawningKit::Context skContext; Context context; BasicGroupInfo groupInfo; - unsigned int generation; vector sockets; Pipe stdinFd, stdoutAndErrFd; FileDescriptor server1, server2, server3; @@ -35,8 +34,6 @@ namespace tut { groupInfo.group = NULL; groupInfo.name = "test"; - generation = 0; - struct sockaddr_in addr; socklen_t len = sizeof(addr); SpawningKit::Result::Socket socket; @@ -122,9 +119,8 @@ namespace tut { args["spawner_creation_time"] = 0; Process *p = context.processObjectPool.malloc(); - p = new (p) Process(&groupInfo, generation, result, args); + p = new (p) Process(&groupInfo, 0, result, args); ProcessPtr process(p, false); - process->shutdownNotRequired(); return process; } @@ -243,15 +239,4 @@ namespace tut { && contents.find("stdout and err 4\n") != string::npos; ); } - - TEST_METHOD(6) { - set_test_name("Test generation is inherited from pool at construction time"); - // this test is pointless b/c the process is made at line 123 of this file, not in the regular codebase - ProcessPtr process1 = createProcess(); - ensure_equals(process1->generation, generation); - generation++; - ProcessPtr process2 = createProcess(); - ensure_equals(process2->generation, generation); - ensure(process1->generation != process2->generation); - } } From f8481e1dc254074de85a73e085d5047eac08083a Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Thu, 3 Oct 2024 16:44:45 +0200 Subject: [PATCH 12/20] Fix typo, set test name --- src/agent/Core/ApplicationPool/Process.h | 2 +- test/cxx/Core/ApplicationPool/PoolTest.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Process.h b/src/agent/Core/ApplicationPool/Process.h index 611a384fd5..0685e367a0 100644 --- a/src/agent/Core/ApplicationPool/Process.h +++ b/src/agent/Core/ApplicationPool/Process.h @@ -383,7 +383,7 @@ class Process { /** Last time when a session was opened for this Process. */ unsigned long long lastUsed; - /** Which gereration of app processes this one belongs to, + /** Which generation of app processes this one belongs to, inherited from the app group, incremented when a restart is initiated*/ const unsigned int generation; diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index 3bb0e4ce44..9a86b3ff86 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -632,7 +632,7 @@ namespace tut { } TEST_METHOD(15) { - // Test that the process generation increments when the group restarts + set_test_name("Process generation increments when the group restarts"); Options options = createOptions(); // Spawn a process and opens a session with it. From 6ab1b52d6d9c8c4885dda8c8932626c5ed4a3902 Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Thu, 3 Oct 2024 16:55:09 +0200 Subject: [PATCH 13/20] Use spawnStartTime instead of spawnerCreationTime --- .../Group/ProcessListManagement.cpp | 10 ++++----- src/agent/Core/ApplicationPool/Process.h | 22 ++++++++++--------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 5d5e304bfa..8020020417 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -68,8 +68,8 @@ Group::findBestProcessPreferringStickySessionId(unsigned int id) const { return process; } else if (bestProcess == nullptr || process->generation > bestProcess->generation || - (process->generation == bestProcess->generation && process->spawnerCreationTime < bestProcess->spawnerCreationTime) || - (process->generation == bestProcess->generation && process->spawnerCreationTime == bestProcess->spawnerCreationTime && process->busyness() < bestProcess->busyness()) + (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) || + (process->generation == bestProcess->generation && process->spawnStartTime == bestProcess->spawnStartTime && process->busyness() < bestProcess->busyness()) ) { bestProcess = process; } @@ -91,8 +91,8 @@ Group::findBestProcess(const ProcessList &processes) const { if (bestProcess == nullptr || process->generation > bestProcess->generation || - (process->generation == bestProcess->generation && process->spawnerCreationTime < bestProcess->spawnerCreationTime) || - (process->generation == bestProcess->generation && process->spawnerCreationTime == bestProcess->spawnerCreationTime && process->busyness() < bestProcess->busyness()) + (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) || + (process->generation == bestProcess->generation && process->spawnStartTime == bestProcess->spawnStartTime && process->busyness() < bestProcess->busyness()) ) { bestProcess = process; } @@ -119,7 +119,7 @@ Group::findBestEnabledProcess() const { for (unsigned int i = 0; i < size; i++) { Process *process = enabledProcesses.at(i).get(); unsigned int gen = process->generation; - unsigned long long startTime = process->spawnerCreationTime; + unsigned long long startTime = process->spawnStartTime; int busyness = enabledProcessBusynessLevels[i]; if (bestProcess == nullptr || gen > bestProcess->generation || diff --git a/src/agent/Core/ApplicationPool/Process.h b/src/agent/Core/ApplicationPool/Process.h index 0685e367a0..7275402d6d 100644 --- a/src/agent/Core/ApplicationPool/Process.h +++ b/src/agent/Core/ApplicationPool/Process.h @@ -99,13 +99,9 @@ typedef boost::container::vector ProcessList; */ class Process { public: - static const unsigned int MAX_SOCKETS_ACCEPTING_HTTP_REQUESTS = 3; + friend class Group; - /** - * Time at which the Spawner that created this process was created. - * Microseconds resolution. - */ - unsigned long long spawnerCreationTime; + static const unsigned int MAX_SOCKETS_ACCEPTING_HTTP_REQUESTS = 3; private: /************************************************************* @@ -146,6 +142,12 @@ class Process { */ StaticString codeRevision; + /** + * Time at which the Spawner that created this process was created. + * Microseconds resolution. + */ + unsigned long long spawnerCreationTime; + /** Time at which we started spawning this process. Microseconds resolution. */ unsigned long long spawnStartTime; @@ -450,9 +452,9 @@ class Process { ProcessMetrics metrics; Process(const BasicGroupInfo *groupInfo, const unsigned int gen, const Json::Value &args) - : spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), - info(this, groupInfo, args), + : info(this, groupInfo, args), socketsAcceptingHttpRequestsCount(0), + spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), spawnStartTime(getJsonUint64Field(args, "spawn_start_time")), spawnEndTime(SystemTime::getUsec()), type(args["type"] == "dummy" ? SpawningKit::Result::DUMMY : SpawningKit::Result::UNKNOWN), @@ -476,9 +478,9 @@ class Process { Process(const BasicGroupInfo *groupInfo, const unsigned int gen, const SpawningKit::Result &skResult, const Json::Value &args) - : spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), - info(this, groupInfo, skResult), + : info(this, groupInfo, skResult), socketsAcceptingHttpRequestsCount(0), + spawnerCreationTime(getJsonUint64Field(args, "spawner_creation_time")), spawnStartTime(skResult.spawnStartTime), spawnEndTime(skResult.spawnEndTime), type(skResult.type), From cec763864668a8ca2d9987126120e92b176c8e86 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 14 Oct 2024 13:13:38 -0600 Subject: [PATCH 14/20] remove unused headers --- src/agent/Core/ApplicationPool/Pool.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Pool.h b/src/agent/Core/ApplicationPool/Pool.h index 16ee56f928..e2c727db7f 100644 --- a/src/agent/Core/ApplicationPool/Pool.h +++ b/src/agent/Core/ApplicationPool/Pool.h @@ -28,10 +28,8 @@ #include #include -#include #include #include -#include #include #include #include From a45c981a935e1d70e83a413f20d3b76af2b774ae Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 14 Oct 2024 13:16:55 -0600 Subject: [PATCH 15/20] =?UTF-8?q?change=20test=20condition=20per=20hongli?= =?UTF-8?q?=E2=80=99s=20request?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index 9a86b3ff86..bdda8bc6b6 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -644,12 +644,15 @@ namespace tut { // Close the session so that the process is now idle. ProcessPtr process = currentSession->getProcess()->shared_from_this(); + pid_t pid = process->getPid(); currentSession.reset(); unsigned int gen1 = process->generation; ensure(pool->restartGroupByName(options.appRoot)); EVENTUALLY(5, - result = pool->getProcessCount() == 1; + LockGuard l(pool->syncher); + vector processes = pool->getProcesses(false); + processes.size() > 0 && processes[0]->getPid() != pid; ); pool->asyncGet(options, callback); EVENTUALLY(5, From 8cff85c2bd2fba8b2e6afe0409f63c1e5a66caf8 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 14 Oct 2024 13:25:05 -0600 Subject: [PATCH 16/20] =?UTF-8?q?don=E2=80=99t=20select=20a=20totally=20bu?= =?UTF-8?q?sy=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../ApplicationPool/Group/ProcessListManagement.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 3758d198b0..6be6e7bee3 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -72,11 +72,11 @@ Group::findBestProcessPreferringStickySessionId(unsigned int id) const { Process *process = (*it).get(); if (process->getStickySessionId() == id) { return process; - } else if (bestProcess == nullptr || + } else if (!process->isTotallyBusy() && (bestProcess == nullptr || process->generation > bestProcess->generation || (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) || (process->generation == bestProcess->generation && process->spawnStartTime == bestProcess->spawnStartTime && process->busyness() < bestProcess->busyness()) - ) { + )) { bestProcess = process; } } @@ -95,11 +95,11 @@ Group::findBestProcess(const ProcessList &processes) const { for (it = processes.begin(); it != end; it++) { Process *process = (*it).get(); - if (bestProcess == nullptr || + if (!process->isTotallyBusy() && (bestProcess == nullptr || process->generation > bestProcess->generation || (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) || (process->generation == bestProcess->generation && process->spawnStartTime == bestProcess->spawnStartTime && process->busyness() < bestProcess->busyness()) - ) { + )) { bestProcess = process; } } @@ -127,11 +127,11 @@ Group::findBestEnabledProcess() const { unsigned int gen = process->generation; unsigned long long startTime = process->spawnStartTime; int busyness = enabledProcessBusynessLevels[i]; - if (bestProcess == nullptr || + if (!process->isTotallyBusy() && (bestProcess == nullptr || gen > bestProcess->generation || (gen == bestProcessGen && startTime < bestProcessStartTime) || (gen == bestProcessGen && startTime == bestProcessStartTime && busyness < bestProcessBusyness) - ) { + )) { bestProcess = process; bestProcessGen = gen; bestProcessBusyness = busyness; From b40c7d2d17292baa879a9a6be4213ad45f432a1c Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 14 Oct 2024 13:59:47 -0600 Subject: [PATCH 17/20] fallback to best possible process in case all are totally busy --- .../Group/ProcessListManagement.cpp | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index 6be6e7bee3..c92c587205 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -66,12 +66,21 @@ Group::findProcessWithStickySessionId(unsigned int id) const { Process * Group::findBestProcessPreferringStickySessionId(unsigned int id) const { Process *bestProcess = nullptr; + Process *fallbackProcess = nullptr; ProcessList::const_iterator it; ProcessList::const_iterator end = enabledProcesses.end(); for (it = enabledProcesses.begin(); it != end; it++) { Process *process = (*it).get(); if (process->getStickySessionId() == id) { return process; + } else if (process->isTotallyBusy() && bestProcess == nullptr) { + if (fallbackProcess == nullptr || + process->generation > fallbackProcess->generation || + (process->generation == fallbackProcess->generation && process->spawnStartTime < fallbackProcess->spawnStartTime) || + (process->generation == fallbackProcess->generation && process->spawnStartTime == fallbackProcess->spawnStartTime && process->busyness() < fallbackProcess->busyness()) + ) { + fallbackProcess = process; + } } else if (!process->isTotallyBusy() && (bestProcess == nullptr || process->generation > bestProcess->generation || (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) || @@ -80,6 +89,9 @@ Group::findBestProcessPreferringStickySessionId(unsigned int id) const { bestProcess = process; } } + if (bestProcess == nullptr) { + return fallbackProcess; + } return bestProcess; } @@ -90,12 +102,21 @@ Group::findBestProcess(const ProcessList &processes) const { } Process *bestProcess = nullptr; + Process *fallbackProcess = nullptr; ProcessList::const_iterator it; ProcessList::const_iterator end = processes.end(); for (it = processes.begin(); it != end; it++) { Process *process = (*it).get(); - if (!process->isTotallyBusy() && (bestProcess == nullptr || + if (process->isTotallyBusy() && bestProcess == nullptr) { + if (fallbackProcess == nullptr || + process->generation > fallbackProcess->generation || + (process->generation == fallbackProcess->generation && process->spawnStartTime < fallbackProcess->spawnStartTime) || + (process->generation == fallbackProcess->generation && process->spawnStartTime == fallbackProcess->spawnStartTime && process->busyness() < fallbackProcess->busyness()) + ) { + fallbackProcess = process; + } + } else if (!process->isTotallyBusy() && (bestProcess == nullptr || process->generation > bestProcess->generation || (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) || (process->generation == bestProcess->generation && process->spawnStartTime == bestProcess->spawnStartTime && process->busyness() < bestProcess->busyness()) @@ -103,6 +124,9 @@ Group::findBestProcess(const ProcessList &processes) const { bestProcess = process; } } + if (bestProcess == nullptr) { + return fallbackProcess; + } return bestProcess; } @@ -119,6 +143,12 @@ Group::findBestEnabledProcess() const { unsigned long long bestProcessStartTime = 0; unsigned int bestProcessGen = 0; int bestProcessBusyness = 0; + + Process* fallbackProcess = nullptr; + unsigned long long fallbackProcessStartTime = 0; + unsigned int fallbackProcessGen = 0; + int fallbackProcessBusyness = 0; + unsigned int size = enabledProcessBusynessLevels.size(); const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; @@ -127,7 +157,19 @@ Group::findBestEnabledProcess() const { unsigned int gen = process->generation; unsigned long long startTime = process->spawnStartTime; int busyness = enabledProcessBusynessLevels[i]; - if (!process->isTotallyBusy() && (bestProcess == nullptr || + bool totallyBusy = process->isTotallyBusy(); + if (totallyBusy && bestProcess == nullptr) { + if (fallbackProcess == nullptr || + gen > fallbackProcess->generation || + (gen == fallbackProcessGen && startTime < fallbackProcessStartTime) || + (gen == fallbackProcessGen && startTime == fallbackProcessStartTime && busyness < fallbackProcessBusyness) + ) { + fallbackProcess = process; + fallbackProcessGen = gen; + fallbackProcessBusyness = busyness; + fallbackProcessStartTime = startTime; + } + } else if (!totallyBusy && (bestProcess == nullptr || gen > bestProcess->generation || (gen == bestProcessGen && startTime < bestProcessStartTime) || (gen == bestProcessGen && startTime == bestProcessStartTime && busyness < bestProcessBusyness) @@ -138,6 +180,9 @@ Group::findBestEnabledProcess() const { bestProcessStartTime = startTime; } } + if (bestProcess == nullptr) { + return fallbackProcess; + } return bestProcess; } From dec5f73dd6c31d32159a95ea5a39210f1d59d395 Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Thu, 17 Oct 2024 13:10:50 -0600 Subject: [PATCH 18/20] fix pool test for process generation incrementing --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index bdda8bc6b6..32c732f116 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -652,7 +652,7 @@ namespace tut { EVENTUALLY(5, LockGuard l(pool->syncher); vector processes = pool->getProcesses(false); - processes.size() > 0 && processes[0]->getPid() != pid; + result = (processes.size() > 0 && processes[0]->getPid() != pid); ); pool->asyncGet(options, callback); EVENTUALLY(5, From 6de82a7e59cd9da889631275534723429ffa487f Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Fri, 18 Oct 2024 10:59:31 -0600 Subject: [PATCH 19/20] try to make test 8 match new expectations of process selection --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 27 ++++++++++++++-------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index 32c732f116..5b7022a1bc 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -434,6 +434,7 @@ namespace tut { EVENTUALLY(5, result = pool->getProcessCount() == 2; ); + ProcessPtr first_spawned_process = pool->getProcesses(false)[0]; // asyncGet() selects some process. pool->asyncGet(options, callback); @@ -442,31 +443,39 @@ namespace tut { ProcessPtr process1 = currentSession->getProcess()->shared_from_this(); currentSession.reset(); - // The first process now has 1 session, so next asyncGet() should - // select the other process. + // Next asyncGet() should select the process with the lowest spawnTime. pool->asyncGet(options, callback); ensure_equals("(2)", number, 2); SessionPtr session2 = currentSession; ProcessPtr process2 = currentSession->getProcess()->shared_from_this(); currentSession.reset(); - ensure("(3)", process1 != process2); + ensure_equals("(3)", process2, first_spawned_process); - // Both processes now have an equal number of sessions. Next asyncGet() - // can select either. + // Now that one process is totally busy, next asyncGet() should + // select the process that is not totally busy. pool->asyncGet(options, callback); ensure_equals("(4)", number, 3); SessionPtr session3 = currentSession; ProcessPtr process3 = currentSession->getProcess()->shared_from_this(); currentSession.reset(); + ensure("(5)", process3 != first_spawned_process); - // One process now has the lowest number of sessions. Next - // asyncGet() should select that one. + // Next asyncGet() should select the process that is not totally busy again. pool->asyncGet(options, callback); - ensure_equals("(5)", number, 4); + ensure_equals("(6)", number, 4); SessionPtr session4 = currentSession; ProcessPtr process4 = currentSession->getProcess()->shared_from_this(); currentSession.reset(); - ensure("(6)", process3 != process4); + ensure_equals("(7)", process3, process4); + + // Now that both processes are totally busy, next asyncGet() + // should select the process that has the lowest spawnTime. + pool->asyncGet(options, callback); + ensure_equals("(8)", number, 5); + SessionPtr session5 = currentSession; + ProcessPtr process5 = currentSession->getProcess()->shared_from_this(); + currentSession.reset(); + ensure_equals("(9)", process5, first_spawned_process); } TEST_METHOD(9) { From 3b0605ab2aa5807947ab68fe646092f7407076ff Mon Sep 17 00:00:00 2001 From: Camden Narzt Date: Mon, 21 Oct 2024 06:53:50 -0600 Subject: [PATCH 20/20] =?UTF-8?q?test=20doesn=E2=80=99t=20process=20reques?= =?UTF-8?q?ts=20on=20totallybusy=20processes=20obviously?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/cxx/Core/ApplicationPool/PoolTest.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index 5b7022a1bc..459c60379f 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -467,15 +467,6 @@ namespace tut { ProcessPtr process4 = currentSession->getProcess()->shared_from_this(); currentSession.reset(); ensure_equals("(7)", process3, process4); - - // Now that both processes are totally busy, next asyncGet() - // should select the process that has the lowest spawnTime. - pool->asyncGet(options, callback); - ensure_equals("(8)", number, 5); - SessionPtr session5 = currentSession; - ProcessPtr process5 = currentSession->getProcess()->shared_from_this(); - currentSession.reset(); - ensure_equals("(9)", process5, first_spawned_process); } TEST_METHOD(9) {