diff --git a/CHANGELOG b/CHANGELOG index 8320380f3c..a517b8d252 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +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. * Fix a regression from 6.0.10 where running `passenger-config system-properties` would throw an error. Closes GH-2565. * [Enterprise] Fix a memory corruption-related crash that could occur during rolling restarting. * [Ubuntu] Add packages for Ubuntu 24.10 "oracular". diff --git a/src/agent/Core/ApplicationPool/Group.h b/src/agent/Core/ApplicationPool/Group.h index fba45f8091..5a33e55106 100644 --- a/src/agent/Core/ApplicationPool/Group.h +++ b/src/agent/Core/ApplicationPool/Group.h @@ -92,8 +92,18 @@ class Group: public boost::enable_shared_from_this { }; struct RouteResult { - Process *process; - bool finished; + /** The Process to route the request to, or nullptr if no process can be routed to. */ + Process * const process; + /** + * If `process` is nullptr, then `finished` indicates whether another `Group::route()` + * call on a different request *could* succeed, meaning that the caller should continue + * calling `Group::route()` if there are more queued requests that need to be processed. + * + * Usually `finished` is false because all processes are totally busy. But in some cases, + * for example when using sticky sessions, it could be true because other requests can + * potentially be routed to other processes. + */ + const bool finished; RouteResult(Process *p, bool _finished = false) : process(p), @@ -223,9 +233,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/InternalUtils.cpp b/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp index 75e9b8b040..23d91a3772 100644 --- a/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp +++ b/src/agent/Core/ApplicationPool/Group/InternalUtils.cpp @@ -184,7 +184,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); @@ -221,7 +221,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/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp index f387d13c70..5c17841a59 100644 --- a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp @@ -63,72 +63,75 @@ Group::findProcessWithStickySessionId(unsigned int id) const { return NULL; } +/** + * Return the process with the given sticky session ID if it exists. + * If not, then find the "best" enabled process to route a request to, + * according to the same criteria documented for findBestProcess(). + * + * - If the process with the given sticky session ID exists, then always + * returns that process. Meaning that this process could be `!canBeRoutedTo()`. + * - If there is no process that can be routed to, then returns nullptr. + */ Process * -Group::findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const { - int leastBusyProcessIndex = -1; - int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(); - 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) { - leastBusyProcessIndex = i; - lowestBusyness = enabledProcessBusynessLevels[i]; + } 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; } } - if (leastBusyProcessIndex == -1) { - return NULL; - } else { - return enabledProcesses[leastBusyProcessIndex].get(); - } + return bestProcess; } +/** + * Given a ProcessList, find the "best" process to route a request to. + * At the moment, "best" is defined as the process with the highest generation, + * lowest start time, and lowest busyness, in that order of priority. + * + * If there is no process that can be routed to, then returns nullptr. + * + * @post result != nullptr || result.canBeRoutedTo() + */ Process * -Group::findProcessWithLowestBusyness(const ProcessList &processes) const { +Group::findBestProcess(const ProcessList &processes) const { if (processes.empty()) { - return NULL; + return nullptr; } - int lowestBusyness = -1; - 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) { - lowestBusyness = busyness; - leastBusyProcess = process; - } - } - return leastBusyProcess; -} - -/** - * Cache-optimized version of findProcessWithLowestBusyness() for the common case. - */ -Process * -Group::findEnabledProcessWithLowestBusyness() const { - if (enabledProcesses.empty()) { - return NULL; - } - int leastBusyProcessIndex = -1; - int lowestBusyness = 0; - unsigned int i, size = enabledProcessBusynessLevels.size(); - const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0]; + for (it = processes.begin(); it != end; it++) { + Process *process = it->get(); - for (i = 0; i < size; i++) { - if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness) { - leastBusyProcessIndex = i; - lowestBusyness = enabledProcessBusynessLevels[i]; + 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; } } - 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 be41f6ceee..0a7af88e0e 100644 --- a/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp +++ b/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp @@ -30,6 +30,7 @@ #include #endif #include +#include /************************************************************************* * @@ -64,16 +65,17 @@ Group::RouteResult Group::route(const Options &options) const { if (OXT_LIKELY(enabledCount > 0)) { if (options.stickySessionId == 0) { - Process *process = findEnabledProcessWithLowestBusyness(); - if (process->canBeRoutedTo()) { + Process *process = findBestProcess(enabledProcesses); + if (process != nullptr) { + assert(process->canBeRoutedTo()); return RouteResult(process); } else { return RouteResult(NULL, true); } } else { - Process *process = findProcessWithStickySessionIdOrLowestBusyness( + Process *process = findBestProcessPreferringStickySessionId( options.stickySessionId); - if (process != NULL) { + if (process != nullptr) { if (process->canBeRoutedTo()) { return RouteResult(process); } else { @@ -84,8 +86,9 @@ Group::route(const Options &options) const { } } } else { - Process *process = findProcessWithLowestBusyness(disablingProcesses); - if (process->canBeRoutedTo()) { + Process *process = findBestProcess(disablingProcesses); + if (process != nullptr) { + assert(process->canBeRoutedTo()); return RouteResult(process); } else { return RouteResult(NULL, true); @@ -310,9 +313,8 @@ Group::get(const Options &newOptions, const GetCallback &callback, assert(m_spawning || restarting() || poolAtFullCapacity()); if (disablingCount > 0 && !restarting()) { - Process *process = findProcessWithLowestBusyness(disablingProcesses); - assert(process != NULL); - if (!process->isTotallyBusy()) { + Process *process = findBestProcess(disablingProcesses); + if (process != nullptr && !process->isTotallyBusy()) { return newSession(process, newOptions.currentTime); } } 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 diff --git a/src/agent/Core/ApplicationPool/Process.h b/src/agent/Core/ApplicationPool/Process.h index c716b13f41..d6ecb812d1 100644 --- a/src/agent/Core/ApplicationPool/Process.h +++ b/src/agent/Core/ApplicationPool/Process.h @@ -53,6 +53,10 @@ #include #include +namespace tut { + template class test_object; +} + namespace Passenger { namespace ApplicationPool2 { @@ -99,6 +103,9 @@ typedef boost::container::vector ProcessList; */ class Process { public: + friend class Group; + template friend class tut::test_object; + static const unsigned int MAX_SOCKETS_ACCEPTING_HTTP_REQUESTS = 3; private: @@ -388,6 +395,10 @@ class Process { /** Last time when a session was opened for this Process. */ unsigned long long lastUsed; + /** Which generation 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 */ @@ -450,8 +461,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")), @@ -462,6 +472,7 @@ class Process { refcount(1), index(-1), lastUsed(spawnEndTime), + generation(gen), sessions(0), processed(0), lifeStatus(ALIVE), @@ -475,7 +486,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), @@ -487,6 +498,7 @@ class Process { refcount(1), index(-1), lastUsed(spawnEndTime), + generation(gen), sessions(0), processed(0), lifeStatus(ALIVE), diff --git a/test/cxx/Core/ApplicationPool/PoolTest.cpp b/test/cxx/Core/ApplicationPool/PoolTest.cpp index fe8ba18608..6a39853c46 100644 --- a/test/cxx/Core/ApplicationPool/PoolTest.cpp +++ b/test/cxx/Core/ApplicationPool/PoolTest.cpp @@ -207,11 +207,12 @@ namespace tut { // Ensure that n processes exist. Options ensureMinProcesses(unsigned int n) { + int prevNumber = number; Options options = createOptions(); options.minProcesses = n; pool->asyncGet(options, callback); EVENTUALLY(5, - result = number == 1; + result = number == prevNumber + 1; ); EVENTUALLY(5, result = pool->getProcessCount() == n; @@ -261,7 +262,7 @@ namespace tut { TEST_METHOD(3) { // If one matching process already exists and it's not at full - // capacity then asyncGet() will immediately use it. + // utilization, then asyncGet() will immediately use it. Options options = createOptions(); // Spawn a process and opens a session with it. @@ -283,13 +284,13 @@ namespace tut { } TEST_METHOD(4) { - // If one matching process already exists but it's at full capacity, + // If one matching process already exists but it's at full utilization, // and the limits prevent spawning of a new process, // then asyncGet() will put the get action on the group's wait - // queue. When the process is no longer at full capacity it will + // queue. When the process is no longer at full utilization it will // process the request. - // Spawn a process and verify that it's at full capacity. + // Spawn a process and verify that it's at full utilization. // Keep its session open. Options options = createOptions(); options.appGroupName = "test"; @@ -379,104 +380,278 @@ namespace tut { } TEST_METHOD(7) { - // If multiple matching processes exist, and one of them is idle, - // then asyncGet() will use that. + set_test_name("If multiple matching processes exist, none of them at full utilization," + " then asyncGet() routes to the process with the highest generation number"); + + // Spawn a process with generation 0. + ensureMinProcesses(1); + ProcessPtr process1 = pool->getProcesses()[0]; + ensure_equals("(1)", process1->generation, 0u); + + // Spawn another process with generation 1. + { + LockGuard l(pool->syncher); + process1->getGroup()->restartsInitiated = 1; + } + ensureMinProcesses(2); + ProcessPtr process2 = pool->getProcesses()[1]; + ensure_equals("(2)", process2->generation, 1u); - // Spawn 3 processes and keep a session open with 1 of them. + // asyncGet() should select the process with the highest generation. Options options = createOptions(); - options.minProcesses = 3; pool->asyncGet(options, callback); EVENTUALLY(5, - result = number == 1; - ); - EVENTUALLY(5, - result = pool->getProcessCount() == 3; + result = number == 3; ); - SessionPtr session1 = currentSession; - ProcessPtr process1 = currentSession->getProcess()->shared_from_this(); + ProcessPtr process3 = currentSession->getProcess()->shared_from_this(); + ensure_equals("(3)", process3, process2); currentSession.reset(); + } - // Now open another session. It should complete immediately - // and should not use the first process. - ScopedLock l(pool->syncher); - pool->asyncGet(options, callback, false); - ensure_equals("asyncGet() completed immediately", number, 2); - SessionPtr session2 = currentSession; - ProcessPtr process2 = currentSession->getProcess()->shared_from_this(); - l.unlock(); - currentSession.reset(); - ensure(process2 != process1); + TEST_METHOD(8) { + set_test_name("If multiple matching processes exist, none of them at full utilization," + " all of the same generation, then asyncGet() routes to the process with" + " the earliest start time"); - // Now open yet another session. It should also complete immediately - // and should not use the first or the second process. - l.lock(); - pool->asyncGet(options, callback, false); - ensure_equals("asyncGet() completed immediately", number, 3); - SessionPtr session3 = currentSession; + // Spawn two processes with the same generation number. + SystemTime::forceAll(1000000); + ensureMinProcesses(1); + SystemTime::forceAll(2000000); + ensureMinProcesses(2); + SystemTime::releaseAll(); + ProcessPtr process1 = pool->getProcesses()[0]; + ProcessPtr process2 = pool->getProcesses()[1]; + ensure_gt("(1)", process2->spawnStartTime, process1->spawnStartTime); + + // asyncGet() should select the oldest process. + Options options = createOptions(); + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == 3; + ); ProcessPtr process3 = currentSession->getProcess()->shared_from_this(); - l.unlock(); + ensure_equals("(2)", process3, process1); currentSession.reset(); - ensure(process3 != process1); - ensure(process3 != process2); } - TEST_METHOD(8) { - // If multiple matching processes exist, then asyncGet() will use - // the one with the smallest utilization number. + TEST_METHOD(9) { + set_test_name("If multiple matching processes exist, none of them at full utilization," + " all of the same generation and start time, then asyncGet() routes to" + " the process with the lowest busyness"); - // Spawn 2 processes, each with a concurrency of 2. - skDebugSupport.dummyConcurrency = 2; + // Spawn three processes with the same generation number and start time, + // all with a concurrency of 3. + SystemTime::forceAll(1000000); + skDebugSupport.dummyConcurrency = 3; + ensureMinProcesses(3); + SystemTime::releaseAll(); + ProcessPtr process1 = pool->getProcesses()[0]; + ProcessPtr process2 = pool->getProcesses()[1]; + ProcessPtr process3 = pool->getProcesses()[2]; + ensure_equals("(1)", process2->spawnStartTime, process1->spawnStartTime); + ensure_equals("(2)", process3->spawnStartTime, process1->spawnStartTime); + + // Create the following situation: + // process1: sessions=2 + // process2: sessions=1 + // process3: sessions=2 + + // First open 9 sessions. Options options = createOptions(); - options.minProcesses = 2; - pool->setMax(2); - GroupPtr group = pool->findOrCreateGroup(options); + vector sessions; + int prevNumber = number; + for (unsigned int i = 0; i < 9; i++) { + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == static_cast(prevNumber + i + 1); + ); + sessions.push_back(currentSession); + currentSession.reset(); + } + + // Check various sessions belong to the expected processes. + ensure_equals("(3)", sessions[0]->getProcess()->getPid(), process1->getPid()); + ensure_equals("(4)", sessions[1]->getProcess()->getPid(), process2->getPid()); + ensure_equals("(5)", sessions[2]->getProcess()->getPid(), process3->getPid()); + ensure_equals("(6)", sessions[4]->getProcess()->getPid(), process2->getPid()); + + // Close a bunch of sessions and we should arrive at the desired situation. + sessions[0].reset(); + sessions[1].reset(); + sessions[2].reset(); + sessions[4].reset(); { LockGuard l(pool->syncher); - group->spawn(); + ensure_equals("(7)", process1->sessions, 2); + ensure_equals("(8)", process2->sessions, 1); + ensure_equals("(9)", process3->sessions, 2); } - EVENTUALLY(5, - result = pool->getProcessCount() == 2; - ); - // asyncGet() selects some process. + // asyncGet() should select process 2 (least busyness). pool->asyncGet(options, callback); - ensure_equals("(1)", number, 1); - SessionPtr session1 = currentSession; - ProcessPtr process1 = currentSession->getProcess()->shared_from_this(); + prevNumber = number; + EVENTUALLY(5, + result = prevNumber + 1; + ); + ProcessPtr process = currentSession->getProcess()->shared_from_this(); + ensure_equals("(10)", process->getPid(), process2->getPid()); currentSession.reset(); + } - // The first process now has 1 session, so next asyncGet() should - // select the other process. - 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); + TEST_METHOD(10) { + set_test_name("If multiple matching processes exist," + " with generation 3 processes at full utilization," + " generation 2 and generation 1 processes not at full utilization," + " then asyncGet() routes to generation 2"); + + skDebugSupport.dummyConcurrency = 2; - // Both processes now have an equal number of sessions. Next asyncGet() - // can select either. + // Spawn a process with generation 0. + ensureMinProcesses(1); + ProcessPtr process1 = pool->getProcesses()[0]; + GroupPtr group = process1->getGroup()->shared_from_this(); + ensure_equals("(1)", process1->generation, 0u); + + // Spawn another process with generation 2. + { + LockGuard l(pool->syncher); + group->restartsInitiated = 2; + } + ensureMinProcesses(2); + ProcessPtr process2 = pool->getProcesses()[1]; + ensure_equals("(2)", process2->generation, 2u); + + // Spawn another process with generation 3. + { + LockGuard l(pool->syncher); + group->restartsInitiated = 3; + } + ensureMinProcesses(3); + ProcessPtr process3 = pool->getProcesses()[2]; + ensure_equals("(3)", process3->generation, 3u); + + // Create the following situation: + // process1: sessions=0 + // process2: sessions=1 + // process3: sessions=2 (full utilization) + + // First open 6 sessions. + Options options = createOptions(); + vector sessions; + int prevNumber = number; + for (unsigned int i = 0; i < 6; i++) { + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == static_cast(prevNumber + i + 1); + ); + sessions.push_back(currentSession); + currentSession.reset(); + } + + // Check various sessions belong to the expected processes. + ensure_equals("(4)", sessions[0]->getProcess()->getPid(), process3->getPid()); + ensure_equals("(5)", sessions[1]->getProcess()->getPid(), process3->getPid()); + ensure_equals("(6)", sessions[2]->getProcess()->getPid(), process2->getPid()); + ensure_equals("(7)", sessions[3]->getProcess()->getPid(), process2->getPid()); + ensure_equals("(8)", sessions[4]->getProcess()->getPid(), process1->getPid()); + ensure_equals("(9)", sessions[5]->getProcess()->getPid(), process1->getPid()); + + // Close a bunch of sessions and we should arrive at the desired situation. + sessions[3].reset(); + sessions[4].reset(); + sessions[5].reset(); + { + LockGuard l(pool->syncher); + ensure_equals("(10)", process1->sessions, 0); + ensure_equals("(11)", process2->sessions, 1); + ensure_equals("(12)", process3->sessions, 2); + } + + // asyncGet() should select process2 even though process1 has fewer sessions open and is older. pool->asyncGet(options, callback); - ensure_equals("(4)", number, 3); - SessionPtr session3 = currentSession; - ProcessPtr process3 = currentSession->getProcess()->shared_from_this(); + prevNumber = number; + EVENTUALLY(5, + result = number + 1; + ); + ProcessPtr process4 = currentSession->getProcess()->shared_from_this(); + ensure_equals("(13)", process4->getPid(), process2->getPid()); currentSession.reset(); + } - // One process now has the lowest number of sessions. Next - // asyncGet() should select that one. + TEST_METHOD(11) { + set_test_name("If multiple matching processes exist, all of them of the same generation," + " with the lowest-start-time processes being at full utilization" + " and higher-start-time processes not at full utilization," + " then asyncGet() routes to a lowest-start-time process that's not at full utilization"); + + skDebugSupport.dummyConcurrency = 2; + + SystemTime::forceAll(1000000); + ensureMinProcesses(1); + SystemTime::forceAll(2000000); + ensureMinProcesses(2); + SystemTime::forceAll(3000000); + ensureMinProcesses(3); + SystemTime::releaseAll(); + ProcessPtr process1 = pool->getProcesses()[0]; + ProcessPtr process2 = pool->getProcesses()[1]; + ProcessPtr process3 = pool->getProcesses()[2]; + ensure_gt("(1)", process2->spawnStartTime, process1->spawnStartTime); + ensure_gt("(2)", process3->spawnStartTime, process2->spawnStartTime); + + // Create the following situation: + // process1: sessions=2 (full utilization) + // process2: sessions=1 + // process3: sessions=0 + + // First open 6 sessions. + Options options = createOptions(); + vector sessions; + int prevNumber = number; + for (unsigned int i = 0; i < 6; i++) { + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == static_cast(prevNumber + i + 1); + ); + sessions.push_back(currentSession); + currentSession.reset(); + } + + // Check various sessions belong to the expected processes. + ensure_equals("(4)", sessions[0]->getProcess()->getPid(), process1->getPid()); + ensure_equals("(5)", sessions[1]->getProcess()->getPid(), process1->getPid()); + ensure_equals("(6)", sessions[2]->getProcess()->getPid(), process2->getPid()); + ensure_equals("(7)", sessions[3]->getProcess()->getPid(), process2->getPid()); + ensure_equals("(8)", sessions[4]->getProcess()->getPid(), process3->getPid()); + ensure_equals("(9)", sessions[5]->getProcess()->getPid(), process3->getPid()); + + // Close a bunch of sessions and we should arrive at the desired situation. + sessions[3].reset(); + sessions[4].reset(); + sessions[5].reset(); + { + LockGuard l(pool->syncher); + ensure_equals("(10)", process1->sessions, 2); + ensure_equals("(11)", process2->sessions, 1); + ensure_equals("(12)", process3->sessions, 0); + } + + // asyncGet() should select process2 (oldest possible process) even though process3 has fewer sessions open. pool->asyncGet(options, callback); - ensure_equals("(5)", number, 4); - SessionPtr session4 = currentSession; + prevNumber = number; + EVENTUALLY(5, + result = number + 1; + ); ProcessPtr process4 = currentSession->getProcess()->shared_from_this(); + ensure_equals("(13)", process4->getPid(), process2->getPid()); currentSession.reset(); - ensure("(6)", process3 != process4); } - TEST_METHOD(9) { - // If multiple matching processes exist, and all of them are at full capacity, + TEST_METHOD(13) { + // If multiple matching processes exist, and all of them are at full utilization, // and no more processes may be spawned, // then asyncGet() will put the action on the group's wait queue. - // The process that first becomes not at full capacity will process the action. + // The process that first becomes not at full utilization will process the action. // Spawn 2 processes and open 4 sessions. Options options = createOptions(); @@ -517,7 +692,7 @@ namespace tut { ensure(pool->atFullCapacity()); } - TEST_METHOD(10) { + TEST_METHOD(14) { // If multiple matching processes exist, and all of them are at full utilization, // and a new process may be spawned, // then asyncGet() will put the action on the group's wait queue and spawn the @@ -566,7 +741,7 @@ namespace tut { ensure_equals(pool->getProcessCount(), 2u); } - TEST_METHOD(11) { + TEST_METHOD(15) { // Here we test the case where the newly spawned process is earlier. // Spawn 2 processes and open 4 sessions. @@ -604,14 +779,14 @@ namespace tut { ensure_equals(group->getWaitlist.size(), 0u); } - TEST_METHOD(12) { + TEST_METHOD(16) { // Test shutting down. ensureMinProcesses(2); ensure(pool->detachGroupByName("stub/rack")); ensure_equals(pool->getGroupCount(), 0u); } - TEST_METHOD(13) { + TEST_METHOD(17) { // Test shutting down while Group is restarting. initPoolDebugging(); debug->messages->send("Proceed with spawn loop iteration 1"); @@ -623,7 +798,7 @@ namespace tut { ensure_equals(pool->getGroupCount(), 0u); } - TEST_METHOD(14) { + TEST_METHOD(18) { // Test shutting down while Group is spawning. initPoolDebugging(); Options options = createOptions(); @@ -634,7 +809,41 @@ namespace tut { ensure_equals(pool->getGroupCount(), 0u); } - TEST_METHOD(17) { + TEST_METHOD(19) { + set_test_name("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(); + pid_t pid = process->getPid(); + currentSession.reset(); + unsigned int gen1 = process->generation; + + ensure("(1)", pool->restartGroupByName(options.appRoot)); + EVENTUALLY(5, + LockGuard l(pool->syncher); + vector processes = pool->getProcesses(false); + result = (processes.size() > 0 && processes[0]->getPid() != pid); + ); + pool->asyncGet(options, callback); + EVENTUALLY(5, + result = number == 2; + ); + + process = currentSession->getProcess()->shared_from_this(); + currentSession.reset(); + unsigned int gen2 = process->generation; + ensure_equals("(2)", gen2,gen1 + 1); + } + + TEST_METHOD(20) { // Test that restartGroupByName() spawns more processes to ensure // that minProcesses and other constraints are met. ensureMinProcesses(1); @@ -644,7 +853,7 @@ namespace tut { ); } - TEST_METHOD(18) { + TEST_METHOD(21) { // Test getting from an app for which minProcesses is set to 0, // and restart.txt already existed. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); @@ -673,7 +882,7 @@ namespace tut { /*********** Test asyncGet() behavior on multiple Groups ***********/ - TEST_METHOD(20) { + TEST_METHOD(30) { // If the pool is full, and one tries to asyncGet() from a nonexistant group, // then it will kill the oldest idle process and spawn a new process. Options options = createOptions(); @@ -709,7 +918,7 @@ namespace tut { ensure_equals(group1->getProcessCount(), 0u); } - TEST_METHOD(21) { + TEST_METHOD(31) { // If the pool is full, and one tries to asyncGet() from a nonexistant group, // and all existing processes are non-idle, then it will // kill the oldest process and spawn a new process. @@ -745,7 +954,7 @@ namespace tut { ensure_equals(group1->getProcessCount(), 0u); } - TEST_METHOD(22) { + TEST_METHOD(32) { // Suppose the pool is at full capacity, and one tries to asyncGet() from an // existant group that does not have any processes. It should kill a process // from another group, and the request should succeed. @@ -782,7 +991,7 @@ namespace tut { ensure_equals("(4)", pool->getProcessCount(), 1u); } - TEST_METHOD(23) { + TEST_METHOD(33) { // Suppose the pool is at full capacity, and one tries to asyncGet() from an // existant group that does not have any processes, and that happens to need // restarting. It should kill a process from another group and the request @@ -825,7 +1034,7 @@ namespace tut { ensure_equals("(4)", pool->getProcessCount(), 1u); } - TEST_METHOD(24) { + TEST_METHOD(34) { // Suppose the pool is at full capacity, with two groups: // - one that is spawning a process. // - one with no processes. @@ -885,7 +1094,7 @@ namespace tut { ); } - TEST_METHOD(25) { + TEST_METHOD(35) { // Suppose the pool is at full capacity, with two groups: // - one that is spawning a process, and has a queued request. // - one with no processes. @@ -954,7 +1163,7 @@ namespace tut { /*********** Test detachProcess() ***********/ - TEST_METHOD(30) { + TEST_METHOD(40) { // detachProcess() detaches the process from the group. The pool // will restore the minimum number of processes afterwards. Options options = createOptions(); @@ -983,7 +1192,7 @@ namespace tut { ); } - TEST_METHOD(31) { + TEST_METHOD(41) { // If the containing group had waiters on it, and detachProcess() // detaches the only process in the group, then a new process // is automatically spawned to handle the waiters. @@ -1019,7 +1228,7 @@ namespace tut { ); } - TEST_METHOD(32) { + TEST_METHOD(42) { // If the pool had waiters on it then detachProcess() will // automatically create the Groups that were requested // by the waiters. @@ -1062,7 +1271,7 @@ namespace tut { ); } - TEST_METHOD(33) { + TEST_METHOD(43) { // A Group does not become garbage collectable // after detaching all its processes. Options options = createOptions(); @@ -1080,7 +1289,7 @@ namespace tut { ensure(!group->garbageCollectable()); } - TEST_METHOD(34) { + TEST_METHOD(44) { // When detaching a process, it waits until all sessions have // finished before telling the process to shut down. Options options = createOptions(); @@ -1109,7 +1318,7 @@ namespace tut { ); } - TEST_METHOD(35) { + TEST_METHOD(45) { // When detaching a process, it waits until the OS processes // have exited before cleaning up the in-memory data structures. Options options = createOptions(); @@ -1146,7 +1355,7 @@ namespace tut { ); } - TEST_METHOD(36) { + TEST_METHOD(46) { // Detaching a process that is already being detached, works. Options options = createOptions(); options.appGroupName = "test"; @@ -1194,7 +1403,7 @@ namespace tut { /*********** Test disabling and enabling processes ***********/ - TEST_METHOD(40) { + TEST_METHOD(50) { // Disabling a process under idle conditions should succeed immediately. ensureMinProcesses(2); vector processes = pool->getProcesses(); @@ -1212,7 +1421,7 @@ namespace tut { processes[1]->enabled, Process::ENABLED); } - TEST_METHOD(41) { + TEST_METHOD(51) { // Disabling the sole process in a group, in case the pool settings allow // spawning another process, should trigger a new process spawn. ensureMinProcesses(1); @@ -1240,7 +1449,7 @@ namespace tut { ); } - TEST_METHOD(42) { + TEST_METHOD(52) { // Disabling the sole process in a group, in case pool settings don't allow // spawning another process, should fail. pool->setMax(1); @@ -1260,7 +1469,7 @@ namespace tut { ensure_equals("(3)", pool->getProcessCount(), 1u); } - TEST_METHOD(43) { + TEST_METHOD(53) { // If there are no enabled processes in the group, then disabling should // succeed after the new process has been spawned. initPoolDebugging(); @@ -1307,7 +1516,7 @@ namespace tut { } } - TEST_METHOD(44) { + TEST_METHOD(54) { // Suppose that a previous disable command triggered a new process spawn, // and the spawn fails. Then any disabling processes should become enabled // again, and the callbacks for the previous disable commands should be called. @@ -1372,7 +1581,7 @@ namespace tut { // TODO: A disabling process becomes disabled as soon as it's done with // all its request. - TEST_METHOD(50) { + TEST_METHOD(60) { // Disabling a process that's already being disabled should result in the // callback being called after disabling is done. ensureMinProcesses(2); @@ -1396,7 +1605,7 @@ namespace tut { // TODO: Enabling a process that's disabling succeeds immediately. The disable // callbacks will be called with DR_CANCELED. - TEST_METHOD(51) { + TEST_METHOD(61) { // If the number of processes is already at maximum, then disabling // a process will cause that process to be disabled, without spawning // a new process. @@ -1421,7 +1630,7 @@ namespace tut { /*********** Other tests ***********/ - TEST_METHOD(60) { + TEST_METHOD(70) { // The pool is considered to be at full capacity if and only // if all Groups are at full capacity. Options options = createOptions(); @@ -1446,7 +1655,7 @@ namespace tut { ensure(!pool->atFullCapacity()); } - TEST_METHOD(61) { + TEST_METHOD(71) { // If the pool is at full capacity, then increasing 'max' will cause // new processes to be spawned. Any queued get requests are processed // as those new processes become available or as existing processes @@ -1469,7 +1678,7 @@ namespace tut { ensure_equals(pool->getProcessCount(), 3u); } - TEST_METHOD(62) { + TEST_METHOD(72) { // Each spawned process has a GUPID, which can be looked up // through findProcessByGupid(). Options options = createOptions(); @@ -1483,13 +1692,13 @@ namespace tut { pool->findProcessByGupid(gupid).get()); } - TEST_METHOD(63) { + TEST_METHOD(73) { // findProcessByGupid() returns a NULL pointer if there is // no matching process. ensure(pool->findProcessByGupid("none") == NULL); } - TEST_METHOD(64) { + TEST_METHOD(74) { // Test process idle cleaning. Options options = createOptions(); pool->setMaxIdleTime(50000); @@ -1515,7 +1724,7 @@ namespace tut { ); } - TEST_METHOD(65) { + TEST_METHOD(75) { // Test spawner idle cleaning. Options options = createOptions(); options.appGroupName = "test1"; @@ -1541,7 +1750,7 @@ namespace tut { ); } - TEST_METHOD(66) { + TEST_METHOD(76) { // It should restart the app if restart.txt is created or updated. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); Options options = createOptions(); @@ -1578,7 +1787,7 @@ namespace tut { ensure_equals(sendRequest(options, "/"), "restarted 2"); } - TEST_METHOD(67) { + TEST_METHOD(77) { // Test spawn exceptions. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); Options options = createOptions(); @@ -1607,7 +1816,7 @@ namespace tut { ensure(e->getProblemDescriptionHTML().find("Something went wrong!") != string::npos); } - TEST_METHOD(68) { + TEST_METHOD(78) { // If a process fails to spawn, then it stops trying to spawn minProcesses processes. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); Options options = createOptions(); @@ -1655,7 +1864,7 @@ namespace tut { ); } - TEST_METHOD(69) { + TEST_METHOD(79) { // It removes the process from the pool if session->initiate() fails. Options options = createOptions(); options.appRoot = "stub/wsgi"; @@ -1685,7 +1894,7 @@ namespace tut { ensure_equals(pool->getProcessCount(), 0u); } - TEST_METHOD(70) { + TEST_METHOD(80) { // When a process has become idle, and there are waiters on the pool, // consider detaching it in order to satisfy a waiter. Options options1 = createOptions(); @@ -1715,7 +1924,7 @@ namespace tut { ensure_equals(group2->enabledCount, 1); } - TEST_METHOD(71) { + TEST_METHOD(81) { // A process is detached after processing maxRequests sessions. Options options = createOptions(); options.minProcesses = 0; @@ -1739,7 +1948,7 @@ namespace tut { ); } - TEST_METHOD(72) { + TEST_METHOD(82) { // If we restart while spawning is in progress, and the restart // finishes before the process is done spawning, then that // process will not be attached and the original spawn loop will @@ -1785,7 +1994,7 @@ namespace tut { ensure_equals("(4)", pool->getProcessCount(), 3u); } - TEST_METHOD(73) { + TEST_METHOD(83) { // If a get() request comes in while the restart is in progress, then // that get() request will be put into the get waiters list, which will // be processed after spawning is done. @@ -1819,7 +2028,7 @@ namespace tut { pool->getProcessCount(), 2u); } - TEST_METHOD(74) { + TEST_METHOD(84) { // If a process fails to spawn, it sends a SpawnException result to all get waiters. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); chmod("tmp.wsgi", 0777); @@ -1873,7 +2082,7 @@ namespace tut { ensure(sessions.empty()); } - TEST_METHOD(75) { + TEST_METHOD(85) { // If a process fails to spawn, the existing processes // are kept alive. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); @@ -1911,7 +2120,7 @@ namespace tut { } } - TEST_METHOD(76) { + TEST_METHOD(86) { // No more than maxOutOfBandWorkInstances process will be performing // out-of-band work at the same time. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); @@ -1958,7 +2167,7 @@ namespace tut { debug->debugger->recv("OOBW request finished"); } - TEST_METHOD(77) { + TEST_METHOD(87) { // If the getWaitlist already has maxRequestQueueSize items, // then an exception is returned. Options options = createOptions(); @@ -1993,7 +2202,7 @@ namespace tut { ); } - TEST_METHOD(78) { + TEST_METHOD(88) { // Test restarting while a previous restart was already being finalized. // The previous finalization should abort. Options options = createOptions(); @@ -2011,7 +2220,7 @@ namespace tut { debug->debugger->recv("Restarting aborted"); } - TEST_METHOD(79) { + TEST_METHOD(89) { // Test sticky sessions. // Spawn 2 processes and get their sticky session IDs and PIDs. @@ -2072,7 +2281,7 @@ namespace tut { /*********** Test previously discovered bugs ***********/ - TEST_METHOD(85) { + TEST_METHOD(95) { // Test detaching, then restarting. This should not violate any invariants. TempDirCopy dir("stub/wsgi", "tmp.wsgi"); Options options = createOptions(); diff --git a/test/cxx/Core/ApplicationPool/ProcessTest.cpp b/test/cxx/Core/ApplicationPool/ProcessTest.cpp index 6e6518677b..a3d508cd06 100644 --- a/test/cxx/Core/ApplicationPool/ProcessTest.cpp +++ b/test/cxx/Core/ApplicationPool/ProcessTest.cpp @@ -118,8 +118,9 @@ 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; } diff --git a/test/tut/tut.h b/test/tut/tut.h index 23ebcca85f..80a8cb1db9 100644 --- a/test/tut/tut.h +++ b/test/tut/tut.h @@ -815,6 +815,52 @@ void ensure_equals(const Q& actual, const T& expected) ensure_equals<>(0, actual, expected); } +/** + * Tests two objects for not being equal. + * Throws if equal. + * + * NB: T must have operator << defined somewhere, or + * client code will not compile at all! + */ +template +void ensure_not_equals(const char* msg, const Q& a, const T& b) +{ + if (a == b) + { + std::stringstream ss; + ss << (msg ? msg : "") + << (msg ? ":" : "") + << " expected not to be '" + << a + << "'"; + throw failure(ss.str().c_str()); + } +} + +/** + * Tests that A > B. + * Throws if false. + * + * NB: both T and Q must have operator << defined somewhere, or + * client code will not compile at all! + */ +template +void ensure_gt(const char* msg, const Q& a, const T& b) +{ + if (!(a > b)) + { + std::stringstream ss; + ss << (msg ? msg : "") + << (msg ? ":" : "") + << " expected '" + << a + << "' to be greater than '" + << b + << '\''; + throw failure(ss.str().c_str()); + } +} + /** * Tests two objects for being at most in given distance one from another. * Borders are excluded.