diff --git a/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java b/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java index c146071..f24d814 100644 --- a/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java +++ b/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java @@ -400,8 +400,9 @@ void heartbeat() { private void testConnection() { PooledConnection conn = null; try { - // Get a connection from the pool and test it - conn = getPooledConnection(); + // Get a connection from the pool and test it (heartbeat obtain is + // excluded from the pool usage metrics - it is not application use) + conn = getPooledConnection(true); heartbeatPoolExhaustedCount = 0; if (testConnection(conn)) { notifyDataSourceIsUp(); @@ -596,7 +597,7 @@ void removeClosedConnection(PooledConnection pooledConnection) { * must be removed and closed fully. */ private void returnTheConnection(PooledConnection pooledConnection, boolean forceClose) { - if (poolListener != null && !forceClose) { + if (poolListener != null && !forceClose && !pooledConnection.heartbeat()) { poolListener.onBeforeReturnConnection(pooledConnection); } queue.returnPooledConnection(pooledConnection, forceClose); @@ -664,7 +665,16 @@ public Connection getConnection() throws SQLException { * will go into a wait if the pool has hit its maximum size. */ private PooledConnection getPooledConnection() throws SQLException { - PooledConnection c = queue.obtainConnection(); + return getPooledConnection(false); + } + + private PooledConnection getPooledConnection(boolean heartbeat) throws SQLException { + PooledConnection c = queue.obtainConnection(heartbeat); + c.setHeartbeat(heartbeat); + if (heartbeat) { + // not application use so skip poolListener etc + return c; + } if (captureStackTrace) { c.setStackTrace(Thread.currentThread().getStackTrace()); } diff --git a/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnection.java b/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnection.java index 097f7e2..050c98f 100644 --- a/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnection.java +++ b/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnection.java @@ -109,6 +109,7 @@ final class PooledConnection extends ConnectionDelegator { private boolean failoverToReadOnly; private boolean autoCommit; private boolean readOnly; + private boolean heartbeat; private int transactionIsolation; private int schemaState = SCHEMA_CATALOG_UNKNOWN; private int catalogState = SCHEMA_CATALOG_UNKNOWN; @@ -976,6 +977,21 @@ void setStackTrace(StackTraceElement[] stackTrace) { this.stackTrace = stackTrace; } + /** + * Mark this connection as borrowed for internal heartbeat validation + * (rather than application use). + */ + void setHeartbeat(boolean heartbeat) { + this.heartbeat = heartbeat; + } + + /** + * Return true if this connection was borrowed for internal heartbeat validation. + */ + boolean heartbeat() { + return heartbeat; + } + /** * Return the stackTrace as a String for logging purposes. */ diff --git a/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnectionQueue.java b/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnectionQueue.java index 6beb82b..3383f80 100644 --- a/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnectionQueue.java +++ b/ebean-datasource/src/main/java/io/ebean/datasource/pool/PooledConnectionQueue.java @@ -191,8 +191,12 @@ private boolean stale(PooledConnection c) { } PooledConnection obtainConnection() throws SQLException { + return obtainConnection(false); + } + + PooledConnection obtainConnection(boolean heartbeat) throws SQLException { try { - PooledConnection pc = _obtainConnection(); + PooledConnection pc = _obtainConnection(heartbeat); pc.resetForUse(); return pc; @@ -214,16 +218,17 @@ private int registerBusyConnection(PooledConnection connection) { return busySize; } - private PooledConnection _obtainConnection() throws InterruptedException, SQLException { + private PooledConnection _obtainConnection(boolean heartbeat) throws InterruptedException, SQLException { var start = System.nanoTime(); lock.lockInterruptibly(); try { if (doingShutdown) { throw new SQLException("Trying to access the Connection Pool when it is shutting down"); } - // this includes attempts that fail with InterruptedException - // or SQLException but that is ok as its only an indicator - hitCount++; + // exclude heartbeat from application metrics + if (!heartbeat) { + hitCount++; + } // are other threads already waiting? (they get priority) if (waitingThreads == 0) { PooledConnection connection = extractFromFreeList(); @@ -238,17 +243,23 @@ private PooledConnection _obtainConnection() throws InterruptedException, SQLExc try { // The pool is at maximum size. We are going to go into // a wait loop until connections are returned into the pool. - waitCount++; + if (!heartbeat) { + waitCount++; + } waitingThreads++; return _obtainConnectionWaitLoop(); } finally { waitingThreads--; - totalWaitNanos += (System.nanoTime() - start); + if (!heartbeat) { + totalWaitNanos += (System.nanoTime() - start); + } } } finally { - final var elapsed = System.nanoTime() - start; - totalAcquireNanos += elapsed; - maxAcquireNanos = Math.max(maxAcquireNanos, elapsed); + if (!heartbeat) { + final var elapsed = System.nanoTime() - start; + totalAcquireNanos += elapsed; + maxAcquireNanos = Math.max(maxAcquireNanos, elapsed); + } lock.unlock(); } } diff --git a/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolFullTest.java b/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolFullTest.java index 36e0c50..970d5d1 100644 --- a/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolFullTest.java +++ b/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolFullTest.java @@ -70,8 +70,11 @@ void testPoolFullWithHeartbeat() throws Exception { assertThat(down).isEqualTo(1); PoolStatus status = pool.status(true); - assertThat(status.waitCount()).isGreaterThan(0); - assertThat(status.totalWaitMicros()).isBetween(0L, 9_000_000L); + // heartbeat validation waits on the full pool are no longer counted as + // application use, and this single-threaded app never waits, so the wait + // metrics are now zero (previously the heartbeat contention inflated them) + assertThat(status.waitCount()).isEqualTo(0); + assertThat(status.totalWaitMicros()).isEqualTo(0); assertThat(status.totalAcquireMicros()).isBetween(0L, 20_000_000L); assertThat(status.maxAcquireMicros()).isBetween(0L, 3_000_000L); diff --git a/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolHeartbeatMetricsTest.java b/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolHeartbeatMetricsTest.java new file mode 100644 index 0000000..2f9adb8 --- /dev/null +++ b/ebean-datasource/src/test/java/io/ebean/datasource/pool/ConnectionPoolHeartbeatMetricsTest.java @@ -0,0 +1,73 @@ +package io.ebean.datasource.pool; + +import io.ebean.datasource.DataSourceConfig; +import io.ebean.datasource.DataSourcePoolListener; +import io.ebean.datasource.PoolStatus; +import org.junit.jupiter.api.Test; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.assertj.core.api.Assertions.assertThat; + +class ConnectionPoolHeartbeatMetricsTest { + + private final AtomicInteger borrowed = new AtomicInteger(); + private final AtomicInteger returned = new AtomicInteger(); + + private ConnectionPool createPool() { + DataSourceConfig config = new DataSourceConfig(); + config.setUrl("jdbc:h2:mem:heartbeatMetrics"); + config.setUsername("sa"); + config.setPassword(""); + config.setMinConnections(2); + config.setMaxConnections(4); + config.setListener(new DataSourcePoolListener() { + @Override + public void onAfterBorrowConnection(Connection connection) { + borrowed.incrementAndGet(); + } + + @Override + public void onBeforeReturnConnection(Connection connection) { + returned.incrementAndGet(); + } + }); + return new ConnectionPool("heartbeatMetrics", config); + } + + @Test + void heartbeat_doesNotAffectUsageMetricsOrListener() throws SQLException { + ConnectionPool pool = createPool(); + try { + // application use - establishes the baseline usage metrics + listener counts + try (Connection c1 = pool.getConnection(); Connection c2 = pool.getConnection()) { + c1.rollback(); + c2.rollback(); + } + + PoolStatus before = pool.status(false); + assertThat(before.hitCount()).isEqualTo(2); + assertThat(borrowed.get()).isEqualTo(2); + assertThat(returned.get()).isEqualTo(2); + + // heartbeat validation obtains/validates a connection multiple times + for (int i = 0; i < 5; i++) { + pool.heartbeat(); + } + + PoolStatus after = pool.status(false); + // heartbeat validation must NOT be counted as application use + assertThat(after.hitCount()).isEqualTo(before.hitCount()); + assertThat(after.waitCount()).isEqualTo(before.waitCount()); + assertThat(after.totalWaitMicros()).isEqualTo(before.totalWaitMicros()); + assertThat(after.maxAcquireMicros()).isEqualTo(before.maxAcquireMicros()); + // and must NOT fire the borrow/return listener callbacks + assertThat(borrowed.get()).isEqualTo(2); + assertThat(returned.get()).isEqualTo(2); + } finally { + pool.shutdown(); + } + } +}