From edc5bbf0d9d4faf48fd9a8d479d5bc5de938c82d Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 6 Feb 2023 13:29:38 +0530 Subject: [PATCH 1/7] fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests. * For details on issue see - https://github.com/googleapis/java-spanner/issues/2206 --- .../com/google/cloud/spanner/it/ITClosedSessionTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java index aeb0256285b..227611a10de 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java @@ -251,7 +251,10 @@ public void testTransactionManager() throws InterruptedException { break; } } catch (AbortedException e) { - Thread.sleep(e.getRetryDelayInMillis()); + long retryDelayInMillis = e.getRetryDelayInMillis(); + if(retryDelayInMillis > 0) { + Thread.sleep(retryDelayInMillis); + } txn = manager.resetForRetry(); } } From 4cd497b05eab3e3b6b89b582bfafde80d42c1518 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Wed, 8 Feb 2023 15:27:18 +0530 Subject: [PATCH 2/7] Fixing lint issues. --- .../java/com/google/cloud/spanner/it/ITClosedSessionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java index 227611a10de..efbffcfa899 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java @@ -252,7 +252,7 @@ public void testTransactionManager() throws InterruptedException { } } catch (AbortedException e) { long retryDelayInMillis = e.getRetryDelayInMillis(); - if(retryDelayInMillis > 0) { + if (retryDelayInMillis > 0) { Thread.sleep(retryDelayInMillis); } txn = manager.resetForRetry(); From b035607bc65443a2439e9b6da70b9547522749d3 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Thu, 21 Mar 2024 15:49:29 +0530 Subject: [PATCH 3/7] chore: add session pool options for multiplexed session. --- .../cloud/spanner/SessionPoolOptions.java | 76 ++++++++++++++++++- .../cloud/spanner/SessionPoolOptionsTest.java | 58 ++++++++++++++ 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 2ebe77e1ba2..97d3ef5d584 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -69,6 +69,10 @@ public class SessionPoolOptions { /** Property for allowing mocking of session maintenance clock. */ private final Clock poolMaintainerClock; + private final Duration waitForMultiplexedSession; + private final boolean useMultiplexedSession; + private final Duration multiplexedSessionMaintenanceDuration; + private SessionPoolOptions(Builder builder) { // minSessions > maxSessions is only possible if the user has only set a value for maxSessions. // We allow that to prevent code that only sets a value for maxSessions to break if the @@ -93,6 +97,9 @@ private SessionPoolOptions(Builder builder) { this.randomizePositionQPSThreshold = builder.randomizePositionQPSThreshold; this.inactiveTransactionRemovalOptions = builder.inactiveTransactionRemovalOptions; this.poolMaintainerClock = builder.poolMaintainerClock; + this.useMultiplexedSession = builder.useMultiplexedSession; + this.multiplexedSessionMaintenanceDuration = builder.multiplexedSessionMaintenanceDuration; + this.waitForMultiplexedSession = builder.waitForMultiplexedSession; } @Override @@ -123,7 +130,11 @@ public boolean equals(Object o) { && Objects.equals(this.randomizePositionQPSThreshold, other.randomizePositionQPSThreshold) && Objects.equals( this.inactiveTransactionRemovalOptions, other.inactiveTransactionRemovalOptions) - && Objects.equals(this.poolMaintainerClock, other.poolMaintainerClock); + && Objects.equals(this.poolMaintainerClock, other.poolMaintainerClock) + && Objects.equals(this.useMultiplexedSession, other.useMultiplexedSession) + && Objects.equals( + this.multiplexedSessionMaintenanceDuration, other.multiplexedSessionMaintenanceDuration) + && Objects.equals(this.waitForMultiplexedSession, other.waitForMultiplexedSession); } @Override @@ -148,7 +159,10 @@ public int hashCode() { this.releaseToPosition, this.randomizePositionQPSThreshold, this.inactiveTransactionRemovalOptions, - this.poolMaintainerClock); + this.poolMaintainerClock, + this.useMultiplexedSession, + this.multiplexedSessionMaintenanceDuration, + this.waitForMultiplexedSession); } public Builder toBuilder() { @@ -271,6 +285,18 @@ long getRandomizePositionQPSThreshold() { return randomizePositionQPSThreshold; } + boolean getUseMultiplexedSession() { + return useMultiplexedSession; + } + + Duration getMultiplexedSessionMaintenanceDuration() { + return multiplexedSessionMaintenanceDuration; + } + + Duration getWaitForMultiplexedSession() { + return waitForMultiplexedSession; + } + public static Builder newBuilder() { return new Builder(); } @@ -467,6 +493,9 @@ public static class Builder { */ private long randomizePositionQPSThreshold = 0L; + private boolean useMultiplexedSession = false; + private Duration multiplexedSessionMaintenanceDuration = Duration.ofDays(7); + private Duration waitForMultiplexedSession = Duration.ofSeconds(10); private Clock poolMaintainerClock; private static Position getReleaseToPositionFromSystemProperty() { @@ -669,6 +698,49 @@ Builder setPoolMaintainerClock(Clock poolMaintainerClock) { return this; } + /** + * Sets whether the client should use multiplexed session or not. If set to true, the client + * optimises and runs multiple applicable requests concurrently on a single session. A single + * multiplexed session is sufficient to handle all concurrent traffic, so we don't require to + * provision any additional sessions. + * + *

When set to false, the client uses the regular session cached in the session pool for + * running 1 concurrent transaction per session. We require to provision sufficient sessions by + * making use of {@link SessionPoolOptions#minSessions} and {@link + * SessionPoolOptions#maxSessions} based on the traffic load. Failing to do so will result in + * higher latencies. + * + * @param useMultiplexedSession + */ + Builder setUseMultiplexedSession(boolean useMultiplexedSession) { + this.useMultiplexedSession = useMultiplexedSession; + return this; + } + + @VisibleForTesting + Builder setMultiplexedSessionMaintenanceDuration( + Duration multiplexedSessionMaintenanceDuration) { + this.multiplexedSessionMaintenanceDuration = multiplexedSessionMaintenanceDuration; + return this; + } + + /** + * This option is only useful when {@link SessionPoolOptions#useMultiplexedSession} is set to + * true. If greater than zero, we wait for the said duration at time of client initialization to + * ensure that the multiplexed session is initialised. The default value for this is 10s. + * + *

If this is set to null, the client does not wait for session is initialised which means + * that the first few requests would see more latency since they will need to wait till the + * multiplexed session is initialised. + * + * @param waitForMultiplexedSession + * @return + */ + Builder setWaitForMultiplexedSession(Duration waitForMultiplexedSession) { + this.waitForMultiplexedSession = waitForMultiplexedSession; + return this; + } + /** * Sets whether the client should automatically execute a background query to detect the dialect * that is used by the database or not. Set this option to true if you do not know what the diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java index 22d10d92a87..9e029e931d6 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java @@ -246,4 +246,62 @@ public void testRandomizePositionQPSThreshold() { IllegalArgumentException.class, () -> SessionPoolOptions.newBuilder().setRandomizePositionQPSThreshold(-1L)); } + + @Test + public void testUseMultiplexedSession() { + assertEquals(false, SessionPoolOptions.newBuilder().build().getUseMultiplexedSession()); + assertEquals( + true, + SessionPoolOptions.newBuilder() + .setUseMultiplexedSession(true) + .build() + .getUseMultiplexedSession()); + assertEquals( + false, + SessionPoolOptions.newBuilder() + .setUseMultiplexedSession(true) + .setUseMultiplexedSession(false) + .build() + .getUseMultiplexedSession()); + } + + @Test + public void testMultiplexedSessionMaintenanceDuration() { + assertEquals( + Duration.ofDays(7), + SessionPoolOptions.newBuilder().build().getMultiplexedSessionMaintenanceDuration()); + assertEquals( + Duration.ofDays(2), + SessionPoolOptions.newBuilder() + .setMultiplexedSessionMaintenanceDuration(Duration.ofDays(2)) + .build() + .getMultiplexedSessionMaintenanceDuration()); + assertEquals( + Duration.ofDays(10), + SessionPoolOptions.newBuilder() + .setMultiplexedSessionMaintenanceDuration(Duration.ofDays(2)) + .setMultiplexedSessionMaintenanceDuration(Duration.ofDays(10)) + .build() + .getMultiplexedSessionMaintenanceDuration()); + } + + @Test + public void testWaitForMultiplexedSession() { + assertEquals( + Duration.ofSeconds(10), + SessionPoolOptions.newBuilder().build().getWaitForMultiplexedSession()); + assertEquals( + Duration.ofSeconds(20), + SessionPoolOptions.newBuilder() + .setWaitForMultiplexedSession(Duration.ofSeconds(20)) + .build() + .getWaitForMultiplexedSession()); + assertEquals( + Duration.ofSeconds(10), + SessionPoolOptions.newBuilder() + .setWaitForMultiplexedSession(Duration.ofSeconds(2)) + .setWaitForMultiplexedSession(Duration.ofSeconds(10)) + .build() + .getWaitForMultiplexedSession()); + } } From e59cb3ae4d5cf48753db01fff185ba7d26cac12a Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Thu, 21 Mar 2024 18:05:57 +0530 Subject: [PATCH 4/7] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Olav Løite --- .../main/java/com/google/cloud/spanner/SessionPoolOptions.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 97d3ef5d584..f0f8b540785 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -701,8 +701,7 @@ Builder setPoolMaintainerClock(Clock poolMaintainerClock) { /** * Sets whether the client should use multiplexed session or not. If set to true, the client * optimises and runs multiple applicable requests concurrently on a single session. A single - * multiplexed session is sufficient to handle all concurrent traffic, so we don't require to - * provision any additional sessions. + * multiplexed session is sufficient to handle all concurrent traffic. * *

When set to false, the client uses the regular session cached in the session pool for * running 1 concurrent transaction per session. We require to provision sufficient sessions by From 61ae225cb5c75e52a0aa641cd0b58ed9e790971b Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Thu, 21 Mar 2024 18:06:28 +0530 Subject: [PATCH 5/7] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Olav Løite --- .../google/cloud/spanner/SessionPoolOptions.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index f0f8b540785..b5da313db17 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -724,13 +724,14 @@ Builder setMultiplexedSessionMaintenanceDuration( } /** - * This option is only useful when {@link SessionPoolOptions#useMultiplexedSession} is set to - * true. If greater than zero, we wait for the said duration at time of client initialization to - * ensure that the multiplexed session is initialised. The default value for this is 10s. + * This option is only used when {@link SessionPoolOptions#useMultiplexedSession} is set to + * true. If greater than zero, calls to {@link Spanner#getDatabaseClient(DatabaseId)} will block + * for up to the given duration while waiting for the multiplexed session to be created. + * The default value for this is 10 seconds. * - *

If this is set to null, the client does not wait for session is initialised which means - * that the first few requests would see more latency since they will need to wait till the - * multiplexed session is initialised. + *

If this is set to null or zero, the client does not wait for the session to be created, which means + * that the first read requests could see more latency, as they will need to wait until the + * multiplexed session has been created. * * @param waitForMultiplexedSession * @return From f2f341e7b5c8f7c5cf1446bf6b71186eee0b99b4 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Thu, 21 Mar 2024 18:20:03 +0530 Subject: [PATCH 6/7] fix: comments. --- .../java/com/google/cloud/spanner/SessionPoolOptions.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index b5da313db17..87d42e2d946 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -708,8 +708,6 @@ Builder setPoolMaintainerClock(Clock poolMaintainerClock) { * making use of {@link SessionPoolOptions#minSessions} and {@link * SessionPoolOptions#maxSessions} based on the traffic load. Failing to do so will result in * higher latencies. - * - * @param useMultiplexedSession */ Builder setUseMultiplexedSession(boolean useMultiplexedSession) { this.useMultiplexedSession = useMultiplexedSession; @@ -733,8 +731,8 @@ Builder setMultiplexedSessionMaintenanceDuration( * that the first read requests could see more latency, as they will need to wait until the * multiplexed session has been created. * - * @param waitForMultiplexedSession - * @return + *

Note that we would need to use the option {@link SessionPoolOptions#waitForMinSessions} if + * we want a similar blocking behavior for the other sessions within the session pool. */ Builder setWaitForMultiplexedSession(Duration waitForMultiplexedSession) { this.waitForMultiplexedSession = waitForMultiplexedSession; From c5652b8d26f444844a9d71f00eb4a85d32ebc970 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Thu, 21 Mar 2024 19:39:09 +0530 Subject: [PATCH 7/7] chore: lint fix. --- .../com/google/cloud/spanner/SessionPoolOptions.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 87d42e2d946..92b1baacc03 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -724,12 +724,12 @@ Builder setMultiplexedSessionMaintenanceDuration( /** * This option is only used when {@link SessionPoolOptions#useMultiplexedSession} is set to * true. If greater than zero, calls to {@link Spanner#getDatabaseClient(DatabaseId)} will block - * for up to the given duration while waiting for the multiplexed session to be created. - * The default value for this is 10 seconds. + * for up to the given duration while waiting for the multiplexed session to be created. The + * default value for this is 10 seconds. * - *

If this is set to null or zero, the client does not wait for the session to be created, which means - * that the first read requests could see more latency, as they will need to wait until the - * multiplexed session has been created. + *

If this is set to null or zero, the client does not wait for the session to be created, + * which means that the first read requests could see more latency, as they will need to wait + * until the multiplexed session has been created. * *

Note that we would need to use the option {@link SessionPoolOptions#waitForMinSessions} if * we want a similar blocking behavior for the other sessions within the session pool.