SERVER-42930 Pools for hosts can outlive their groups

Previously, SpecificPools always were shutdown if their Controller told
them they _could_ shutdown. This commit changes the logic to allow
SpecificPools to reject their Controller's suggestions if the
SpecificPool no longer thinks it is "expired".

1 files changed, 24 insertions, 4 deletions

diff --git a/src/mongo/executor/connection_pool.cpp b/src/mongo/executor/connection_pool.cpp
index d584c80a087..0cc26800c65 100644
--- a/src/mongo/executor/connection_pool.cpp
+++ b/src/mongo/executor/connection_pool.cpp
@@ -43,6 +43,7 @@
 #include "mongo/executor/remote_command_request.h"
 #include "mongo/logv2/log.h"
 #include "mongo/util/assert_util.h"
+#include "mongo/util/debug_util.h"
 #include "mongo/util/destructor_guard.h"
 #include "mongo/util/hierarchical_acquisition.h"
 #include "mongo/util/log.h"
@@ -76,6 +77,10 @@ void emplaceOrInvariant(Map&& map, Args&&... args) noexcept {
     invariant(ret.second, "Element already existed in map/set");
 }
 
+bool shouldInvariantOnPoolCorrectness() noexcept {
+    return kDebugBuild;
+}
+
 }  // namespace
 
 namespace executor {
@@ -597,8 +602,10 @@ ConnectionPool::SpecificPool::SpecificPool(std::shared_ptr<ConnectionPool> paren
 ConnectionPool::SpecificPool::~SpecificPool() {
     DESTRUCTOR_GUARD(_eventTimer->cancelTimeout();)
 
-    invariant(_requests.empty());
-    invariant(_checkedOutPool.empty());
+    if (shouldInvariantOnPoolCorrectness()) {
+        invariant(_requests.empty());
+        invariant(_checkedOutPool.empty());
+    }
 }
 
 size_t ConnectionPool::SpecificPool::inUseConnections() const {
@@ -1146,14 +1153,27 @@ void ConnectionPool::SpecificPool::updateController() {
             }
 
             auto& pool = it->second;
+            if (!pool->_health.isExpired) {
+                // Just because a HostGroup "canShutdown" doesn't mean that a SpecificPool should
+                // shutdown. For example, it is always inappropriate to shutdown a SpecificPool with
+                // connections in use or requests outstanding unless its parent ConnectionPool is
+                // also shutting down.
+                LOGV2_WARNING(4293001,
+                              "Controller requested shutdown but connections still in use. "
+                              "Connection pool will stay active.",
+                              "hostAndPort"_attr = pool->_hostAndPort);
+                continue;
+            }
 
             // At the moment, controllers will never mark for shutdown a pool with active
             // connections or pending requests. isExpired is never true if these invariants are
             // false. That's not to say that it's a terrible idea, but if this happens then we
             // should review what it means to be expired.
 
-            invariant(pool->_checkedOutPool.empty());
-            invariant(pool->_requests.empty());
+            if (shouldInvariantOnPoolCorrectness()) {
+                invariant(pool->_checkedOutPool.empty());
+                invariant(pool->_requests.empty());
+            }
 
             pool->triggerShutdown(Status(ErrorCodes::ShutdownInProgress,
                                          str::stream() << "Pool for " << host << " has expired."));