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".

(cherry picked from commit 2d8f351c2640f402027a2fc0c51628db8d4f85b1)

1 files changed, 23 insertions, 4 deletions

diff --git a/src/mongo/executor/connection_pool.cpp b/src/mongo/executor/connection_pool.cpp
index 00d481b217a..f3f2a61bbb8 100644
--- a/src/mongo/executor/connection_pool.cpp
+++ b/src/mongo/executor/connection_pool.cpp
@@ -39,6 +39,7 @@
 #include "mongo/executor/connection_pool_stats.h"
 #include "mongo/executor/remote_command_request.h"
 #include "mongo/util/assert_util.h"
+#include "mongo/util/debug_util.h"
 #include "mongo/util/destructor_guard.h"
 #include "mongo/util/log.h"
 #include "mongo/util/lru_cache.h"
@@ -71,6 +72,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 {
@@ -585,8 +590,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 {
@@ -1075,14 +1082,26 @@ 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.
+                warning() << "Controller requested shutdown but connections still in use. "
+                          << "Connection pool will stay active. "
+                          << "host: " << 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."));