From 366e1ac02c9881a659dec2888399257acf5cfcf1 Mon Sep 17 00:00:00 2001
From: rzpt <51346452+rzpt@users.noreply.github.com>
Date: Tue, 15 Mar 2022 10:23:35 -0700
Subject: [PATCH] use unique pool names (#5963)

Use a global counter to avoid conflicts in hikari pool name. If the old exception handling code was hit, an executor would be leaked due to a bug in hikari.
---
 .../db/pool/HikariCPConnectionManager.java    | 20 +++++--------------
 changelog/@unreleased/pr-5963.v2.yml          |  7 +++++++
 2 files changed, 12 insertions(+), 15 deletions(-)
 create mode 100644 changelog/@unreleased/pr-5963.v2.yml

diff --git a/atlasdb-dbkvs-hikari/src/main/java/com/palantir/nexus/db/pool/HikariCPConnectionManager.java b/atlasdb-dbkvs-hikari/src/main/java/com/palantir/nexus/db/pool/HikariCPConnectionManager.java
index f7cac8c7b33..709c2d9ebd7 100644
--- a/atlasdb-dbkvs-hikari/src/main/java/com/palantir/nexus/db/pool/HikariCPConnectionManager.java
+++ b/atlasdb-dbkvs-hikari/src/main/java/com/palantir/nexus/db/pool/HikariCPConnectionManager.java
@@ -35,8 +35,8 @@
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.TimeZone;
-import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 import javax.management.JMX;
 import javax.management.MBeanServer;
 import javax.management.MalformedObjectNameException;
@@ -53,6 +53,8 @@
 public class HikariCPConnectionManager extends BaseConnectionManager {
     private static final SafeLogger log = SafeLoggerFactory.get(HikariCPConnectionManager.class);
 
+    private static final AtomicLong uniquePoolId = new AtomicLong(0L);
+
     private final ConnectionConfig connConfig;
     private final HikariConfig hikariConfig;
 
@@ -86,6 +88,7 @@ private static class State {
     public HikariCPConnectionManager(ConnectionConfig connConfig) {
         this.connConfig = Preconditions.checkNotNull(connConfig, "ConnectionConfig must not be null");
         this.hikariConfig = connConfig.getHikariConfig();
+        hikariConfig.setPoolName(hikariConfig.getPoolName() + "-" + uniquePoolId.incrementAndGet());
     }
 
     @Override
@@ -300,20 +303,7 @@ private HikariDataSource getDataSourcePool() {
         HikariDataSource dataSourcePool;
 
         try {
-            try {
-                dataSourcePool = new HikariDataSource(hikariConfig);
-            } catch (IllegalArgumentException e) {
-                // allow multiple pools on same JVM (they need unique names / endpoints)
-                if (e.getMessage().contains("A metric named")) {
-                    String poolName = connConfig.getConnectionPoolName();
-
-                    hikariConfig.setPoolName(
-                            poolName + "-" + ThreadLocalRandom.current().nextInt());
-                    dataSourcePool = new HikariDataSource(hikariConfig);
-                } else {
-                    throw e;
-                }
-            }
+            dataSourcePool = new HikariDataSource(hikariConfig);
         } catch (PoolInitializationException e) {
             log.error(
                     "Failed to initialize hikari data source",
diff --git a/changelog/@unreleased/pr-5963.v2.yml b/changelog/@unreleased/pr-5963.v2.yml
new file mode 100644
index 00000000000..67f9da07e0a
--- /dev/null
+++ b/changelog/@unreleased/pr-5963.v2.yml
@@ -0,0 +1,7 @@
+type: fix
+fix:
+  description: Use a global counter to avoid conflicts in hikari pool name. If the
+    old exception handling code was hit, an executor would be leaked due to a bug
+    in hikari.
+  links:
+  - https://github.com/palantir/atlasdb/pull/5963