From ab9f3c5ac743b7ccd84c7d9cd8829473f37ffee4 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 26 Dec 2024 16:15:42 +0530 Subject: [PATCH 1/2] fix: update max_in_use_session at 10 mins interval --- .../com/google/cloud/spanner/SessionPool.java | 12 ++++- .../google/cloud/spanner/SessionPoolTest.java | 48 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java index ffaf913580b..104e31dab21 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java @@ -2048,6 +2048,7 @@ boolean isClosed() { // Does various pool maintenance activities. void maintainPool() { + Instant currTime = clock.instant(); synchronized (lock) { if (SessionPool.this.isClosed()) { return; @@ -2059,8 +2060,14 @@ void maintainPool() { / (loopFrequency / 1000L); } this.prevNumSessionsAcquired = SessionPool.this.numSessionsAcquired; + + // Reset the start time for recording the maximum number of sessions in the pool + if (currTime.isAfter(SessionPool.this.lastResetTime.plus(Duration.ofMinutes(10)))) { + SessionPool.this.maxSessionsInUse = SessionPool.this.numSessionsInUse; + SessionPool.this.lastResetTime = currTime; + } } - Instant currTime = clock.instant(); + removeIdleSessions(currTime); // Now go over all the remaining sessions and see if they need to be kept alive explicitly. keepAliveSessions(currTime); @@ -2309,6 +2316,9 @@ enum Position { @GuardedBy("lock") private int maxSessionsInUse = 0; + @GuardedBy("lock") + private Instant lastResetTime = Clock.INSTANCE.instant(); + @GuardedBy("lock") private long numSessionsAcquired = 0; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java index 0389410064a..4249f05c17f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java @@ -2247,6 +2247,54 @@ public void testWaitOnMinSessionsThrowsExceptionWhenTimeoutIsReached() { pool.maybeWaitOnMinSessions(); } + @Test + public void reset_maxSessionsInUse() { + Clock clock = mock(Clock.class); + when(clock.instant()).thenReturn(Instant.now()); + options = + SessionPoolOptions.newBuilder() + .setMinSessions(1) + .setMaxSessions(3) + .setIncStep(1) + .setMaxIdleSessions(0) + .setPoolMaintainerClock(clock) + .build(); + setupForLongRunningTransactionsCleanup(options); + + pool = createPool(clock); + // Make sure pool has been initialized + pool.getSession().close(); + + // All 3 sessions used. 100% of pool utilised. + PooledSessionFuture readSession1 = pool.getSession(); + PooledSessionFuture readSession2 = pool.getSession(); + PooledSessionFuture readSession3 = pool.getSession(); + + // complete the async tasks + readSession1.get().setEligibleForLongRunning(false); + readSession2.get().setEligibleForLongRunning(false); + readSession3.get().setEligibleForLongRunning(true); + + assertEquals(3, pool.getMaxSessionsInUse()); + assertEquals(3, pool.getNumberOfSessionsInUse()); + + // Release 1 session + readSession1.get().close(); + + // Verify that numSessionsInUse reduces to 2 while maxSessionsInUse remain 3 + assertEquals(3, pool.getMaxSessionsInUse()); + assertEquals(2, pool.getNumberOfSessionsInUse()); + + // ensure that the lastResetTime for maxSessionsInUse > 10 minutes + when(clock.instant()).thenReturn(Instant.now().plus(11, ChronoUnit.MINUTES)); + + pool.poolMaintainer.maintainPool(); + + // Verify that maxSessionsInUse is reset to numSessionsInUse + assertEquals(2, pool.getMaxSessionsInUse()); + assertEquals(2, pool.getNumberOfSessionsInUse()); + } + private void mockKeepAlive(ReadContext context) { ResultSet resultSet = mock(ResultSet.class); when(resultSet.next()).thenReturn(true, false); From cb3495efa865ec68025b0d7851816010cb3c4b14 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 31 Dec 2024 12:24:14 +0530 Subject: [PATCH 2/2] review comments --- .../src/main/java/com/google/cloud/spanner/SessionPool.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java index 104e31dab21..e560ca71deb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java @@ -2048,7 +2048,7 @@ boolean isClosed() { // Does various pool maintenance activities. void maintainPool() { - Instant currTime = clock.instant(); + Instant currTime; synchronized (lock) { if (SessionPool.this.isClosed()) { return; @@ -2061,6 +2061,7 @@ void maintainPool() { } this.prevNumSessionsAcquired = SessionPool.this.numSessionsAcquired; + currTime = clock.instant(); // Reset the start time for recording the maximum number of sessions in the pool if (currTime.isAfter(SessionPool.this.lastResetTime.plus(Duration.ofMinutes(10)))) { SessionPool.this.maxSessionsInUse = SessionPool.this.numSessionsInUse;