Fix race condition causing 'pool not open' errors (#5882)

Fixed a race condition where we would potentially attempt to use a Cassandra client pool that we had already closed.
palantir · Jan 27, 2022 · 4ebeb74 · 4ebeb74
1 parent a8ebaa6
commit 4ebeb74
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 5 deletions.
diff --git a/...sandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraClientPoolImpl.java b/...sandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraClientPoolImpl.java
@@ -39,6 +39,7 @@
 import com.palantir.logsafe.exceptions.SafeIllegalStateException;
 import com.palantir.logsafe.logger.SafeLogger;
 import com.palantir.logsafe.logger.SafeLoggerFactory;
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -50,6 +51,7 @@
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 import org.apache.cassandra.thrift.NotFoundException;
 import org.apache.cassandra.thrift.TokenRange;
 
@@ -318,10 +320,16 @@ private void setServersInPoolTo(Set<InetSocketAddress> desiredServers) {
         serversToRemove.forEach(cassandra::removePool);
 
         if (!(serversToAdd.isEmpty() && serversToRemove.isEmpty())) { // if we made any changes
+            // Log IP addresses along with hostnames
+            Set<InetAddress> addressesToAdd =
+                    serversToAdd.stream().map(InetSocketAddress::getAddress).collect(Collectors.toSet());
+            Set<InetAddress> addressesToRemove =
+                    serversToRemove.stream().map(InetSocketAddress::getAddress).collect(Collectors.toSet());
+
             log.info(
-                    "Servers to add and remove, inside the if block",
-                    SafeArg.of("serversToAdd", serversToAdd),
-                    SafeArg.of("serversToRemove", serversToRemove));
+                    "Added and removed servers from the client pool",
+                    SafeArg.of("serversAdded", addressesToAdd),
+                    SafeArg.of("serversRemoved", addressesToRemove));
             sanityCheckRingConsistency();
             cassandra.refreshTokenRangesAndGetServers();
         }

diff --git a/...assandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/pool/CassandraService.java b/...assandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/pool/CassandraService.java
@@ -399,15 +399,15 @@ public void addPool(InetSocketAddress server) {
 
     public void removePool(InetSocketAddress removedServerAddress) {
         blacklist.remove(removedServerAddress);
+        CassandraClientPoolingContainer removedContainer = currentPools.remove(removedServerAddress);
         try {
-            currentPools.get(removedServerAddress).shutdownPooling();
+            removedContainer.shutdownPooling();
         } catch (Exception e) {
             log.warn(
                     "While removing a host ({}) from the pool, we were unable to gently cleanup resources.",
                     SafeArg.of("removedServerAddress", CassandraLogHelper.host(removedServerAddress)),
                     e);
         }
-        currentPools.remove(removedServerAddress);
     }
 
     public void cacheInitialCassandraHosts() {

diff --git a/changelog/@unreleased/pr-5882.v2.yml b/changelog/@unreleased/pr-5882.v2.yml
@@ -0,0 +1,6 @@
+type: fix
+fix:
+  description: Fixed a race condition where we would potentially attempt to use a
+    Cassandra client pool that we had already closed.
+  links:
+  - https://github.com/palantir/atlasdb/pull/5882