From 4b44b1abd1d9b187159c3d60bec37dbee2fdb1a4 Mon Sep 17 00:00:00 2001 From: sriharshach Date: Tue, 6 Sep 2022 05:21:16 +0000 Subject: [PATCH 01/16] increase default number of channels when grpc-gcp channel pool is enabled --- .../google/cloud/spanner/SpannerOptions.java | 5 ++ .../cloud/spanner/DefaultNumChannelsTest.java | 64 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index b62b2d4455..8a253d9a1f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -693,6 +693,7 @@ public static class Builder private CloseableExecutorProvider asyncExecutorProvider; private String compressorName; private String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST"); + private boolean userSpecifiedNumChannels = false; private Builder() { // Manually set retry and polling settings that work. @@ -811,6 +812,7 @@ public Builder setInterceptorProvider(GrpcInterceptorProvider interceptorProvide */ public Builder setNumChannels(int numChannels) { this.numChannels = numChannels; + this.userSpecifiedNumChannels = true; return this; } @@ -1108,6 +1110,9 @@ public Builder setHost(String host) { /** Enables gRPC-GCP extension with the default settings. */ public Builder enableGrpcGcpExtension() { this.grpcGcpExtensionEnabled = true; + if (!userSpecifiedNumChannels) { // If numChannels is not set through options + this.numChannels = 8; // default when grpc-gcp channel pool is enabled + } return this; } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java new file mode 100644 index 0000000000..cfe311a93d --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java @@ -0,0 +1,64 @@ +package com.google.cloud.spanner; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DefaultNumChannelsTest { + + // With GRPC GCP channel pool enabled + @Test + public void testDefaultNumChannelsWithGrpcGcpEnabled() { + SpannerOptions.Builder builder = SpannerOptions.newBuilder() + .setProjectId("test-project") + .enableGrpcGcpExtension(); + + SpannerOptions options = builder.build(); + assertThat(options.getNumChannels()).isEqualTo(8); + } + + @Test + public void testNumChannelsWithGrpcGcpEnabledLater() { + SpannerOptions.Builder builder = SpannerOptions.newBuilder() + .setProjectId("test-project") + .setNumChannels(5) + .enableGrpcGcpExtension(); + + SpannerOptions options = builder.build(); + assertThat(options.getNumChannels()).isEqualTo(5); + } + + @Test + public void testNumChannelsWithGrpcGcpEnabled() { + SpannerOptions.Builder builder = SpannerOptions.newBuilder() + .setProjectId("test-project") + .enableGrpcGcpExtension() + .setNumChannels(5); + + SpannerOptions options = builder.build(); + assertThat(options.getNumChannels()).isEqualTo(5); + } + + // Without enabling GRPC GCP channel pool + @Test + public void testDefaultNumChannelsWithOutGrpcGcpEnabled() { + SpannerOptions.Builder builder = SpannerOptions.newBuilder() + .setProjectId("test-project"); + + SpannerOptions options = builder.build(); + assertThat(options.getNumChannels()).isEqualTo(4); + } + + @Test + public void testNumChannelsWithOutGrpcGcpEnabled() { + SpannerOptions.Builder builder = SpannerOptions.newBuilder() + .setProjectId("test-project") + .setNumChannels(7); + + SpannerOptions options = builder.build(); + assertThat(options.getNumChannels()).isEqualTo(7); + } +} From 860462ad2a5ffc5101e73d48f427a7e8c12f5150 Mon Sep 17 00:00:00 2001 From: sriharshach Date: Tue, 6 Sep 2022 06:16:08 +0000 Subject: [PATCH 02/16] add java file header --- .../cloud/spanner/DefaultNumChannelsTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java index cfe311a93d..cd7053aeda 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java @@ -1,3 +1,19 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.google.cloud.spanner; import static com.google.common.truth.Truth.assertThat; From f2e2d8fa8009d29c3d40c8d105ad51daaba3d70c Mon Sep 17 00:00:00 2001 From: sriharshach Date: Tue, 6 Sep 2022 09:47:37 +0000 Subject: [PATCH 03/16] increase default number of channels when grpc-gcp channel pool is enabled --- .../google/cloud/spanner/SpannerOptions.java | 25 +++--- .../cloud/spanner/DefaultNumChannelsTest.java | 80 ------------------- .../cloud/spanner/SpannerOptionsTest.java | 33 ++++++++ 3 files changed, 49 insertions(+), 89 deletions(-) delete mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 5e2fb08ab4..c982d0804b 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -89,6 +89,8 @@ public class SpannerOptions extends ServiceOptions { "https://www.googleapis.com/auth/spanner.admin", "https://www.googleapis.com/auth/spanner.data"); private static final int MAX_CHANNELS = 256; + private static final int DEFAULT_NUM_CHANNELS = 4; + private static final int GRPC_GCP_ENABLED_DEFAULT_NUM_CHANNELS = 8; private final TransportChannelProvider channelProvider; @SuppressWarnings("rawtypes") @@ -669,8 +671,10 @@ public static class Builder private GrpcInterceptorProvider interceptorProvider; - /** By default, we create 4 channels per {@link SpannerOptions} */ - private int numChannels = 4; + /** + * By default, we create 4 channels per {@link SpannerOptions} + */ + private Integer numChannels; private String transportChannelExecutorThreadNameFormat = "Cloud-Spanner-TransportChannel-%d"; @@ -696,7 +700,6 @@ public static class Builder private CloseableExecutorProvider asyncExecutorProvider; private String compressorName; private String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST"); - private boolean userSpecifiedNumChannels = false; private Builder() { // Manually set retry and polling settings that work. @@ -816,7 +819,6 @@ public Builder setInterceptorProvider(GrpcInterceptorProvider interceptorProvide */ public Builder setNumChannels(int numChannels) { this.numChannels = numChannels; - this.userSpecifiedNumChannels = true; return this; } @@ -1124,11 +1126,7 @@ public Builder setHost(String host) { /** Enables gRPC-GCP extension with the default settings. */ public Builder enableGrpcGcpExtension() { - this.grpcGcpExtensionEnabled = true; - if (!userSpecifiedNumChannels) { // If numChannels is not set through options - this.numChannels = 8; // default when grpc-gcp channel pool is enabled - } - return this; + return this.enableGrpcGcpExtension(null); } /** @@ -1138,6 +1136,10 @@ public Builder enableGrpcGcpExtension() { public Builder enableGrpcGcpExtension(GcpManagedChannelOptions options) { this.grpcGcpExtensionEnabled = true; this.grpcGcpOptions = options; + // By default, set number of channels to 8 if grpc-gcp extension is enabled + if (this.numChannels == null) { + this.numChannels = GRPC_GCP_ENABLED_DEFAULT_NUM_CHANNELS; + } return this; } @@ -1171,6 +1173,11 @@ public SpannerOptions build() { // As we are using plain text, we should never send any credentials. this.setCredentials(NoCredentials.getInstance()); } + // Set the number of channels if not set + if (this.numChannels == null) { + this.numChannels = DEFAULT_NUM_CHANNELS; + } + return new SpannerOptions(this); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java deleted file mode 100644 index cd7053aeda..0000000000 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultNumChannelsTest.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.cloud.spanner; - -import static com.google.common.truth.Truth.assertThat; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class DefaultNumChannelsTest { - - // With GRPC GCP channel pool enabled - @Test - public void testDefaultNumChannelsWithGrpcGcpEnabled() { - SpannerOptions.Builder builder = SpannerOptions.newBuilder() - .setProjectId("test-project") - .enableGrpcGcpExtension(); - - SpannerOptions options = builder.build(); - assertThat(options.getNumChannels()).isEqualTo(8); - } - - @Test - public void testNumChannelsWithGrpcGcpEnabledLater() { - SpannerOptions.Builder builder = SpannerOptions.newBuilder() - .setProjectId("test-project") - .setNumChannels(5) - .enableGrpcGcpExtension(); - - SpannerOptions options = builder.build(); - assertThat(options.getNumChannels()).isEqualTo(5); - } - - @Test - public void testNumChannelsWithGrpcGcpEnabled() { - SpannerOptions.Builder builder = SpannerOptions.newBuilder() - .setProjectId("test-project") - .enableGrpcGcpExtension() - .setNumChannels(5); - - SpannerOptions options = builder.build(); - assertThat(options.getNumChannels()).isEqualTo(5); - } - - // Without enabling GRPC GCP channel pool - @Test - public void testDefaultNumChannelsWithOutGrpcGcpEnabled() { - SpannerOptions.Builder builder = SpannerOptions.newBuilder() - .setProjectId("test-project"); - - SpannerOptions options = builder.build(); - assertThat(options.getNumChannels()).isEqualTo(4); - } - - @Test - public void testNumChannelsWithOutGrpcGcpEnabled() { - SpannerOptions.Builder builder = SpannerOptions.newBuilder() - .setProjectId("test-project") - .setNumChannels(7); - - SpannerOptions options = builder.build(); - assertThat(options.getNumChannels()).isEqualTo(7); - } -} diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 7d5c8a553a..2ddf9be063 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -918,4 +918,37 @@ public void testCustomAsyncExecutorProvider() { .build(); assertSame(service, options.getAsyncExecutorProvider().getExecutor()); } + + @Test + public void testDefaultNumChannelsWithGrpcGcpExtensionEnabled() { + SpannerOptions.Builder builder = + SpannerOptions.newBuilder().setProjectId("test-project").enableGrpcGcpExtension(); + + SpannerOptions options = builder.build(); + assertEquals(options.getNumChannels(), 8); + } + + @Test + public void testNumChannelsWithGrpcGcpExtensionEnabled() { + // Set number of channels before enabling grpc-gcp channel pool + int numChannels = 5; + SpannerOptions.Builder builder1 = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setNumChannels(numChannels) + .enableGrpcGcpExtension(); + + SpannerOptions options1 = builder1.build(); + assertEquals(options1.getNumChannels(), numChannels); + + // Set number of channels after enabling grpc-gcp channel pool + SpannerOptions.Builder builder2 = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .enableGrpcGcpExtension() + .setNumChannels(numChannels); + + SpannerOptions options2 = builder2.build(); + assertEquals(options2.getNumChannels(), numChannels); + } } From 15e3155962237f1ff77d5bc11b41dcc8ca1f5ac7 Mon Sep 17 00:00:00 2001 From: sriharshach Date: Tue, 6 Sep 2022 09:55:19 +0000 Subject: [PATCH 04/16] fix lint issue --- .../com/google/cloud/spanner/SpannerOptions.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index c982d0804b..bcb1f610cc 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -74,6 +74,7 @@ /** Options for the Cloud Spanner service. */ public class SpannerOptions extends ServiceOptions { + private static final long serialVersionUID = 2789571558532701170L; private static SpannerEnvironment environment = SpannerEnvironmentImpl.INSTANCE; @@ -135,6 +136,7 @@ public class SpannerOptions extends ServiceOptions { * {@link SpannerOptions}. */ public interface CallCredentialsProvider { + /** Return the {@link CallCredentials} to use for a gRPC call. */ CallCredentials getCallCredentials(); } @@ -189,6 +191,7 @@ public interface CallCredentialsProvider { * } */ public interface CallContextConfigurator { + /** * Configure a {@link ApiCallContext} for a specific RPC call. * @@ -309,6 +312,7 @@ static SpannerMethod valueOf(ReqT request, MethodDescriptor */ public static class SpannerCallContextTimeoutConfigurator implements CallContextConfigurator { + private Duration commitTimeout; private Duration rollbackTimeout; @@ -455,6 +459,7 @@ public SpannerCallContextTimeoutConfigurator withPartitionReadTimeout( /** Default implementation of {@code SpannerFactory}. */ private static class DefaultSpannerFactory implements SpannerFactory { + private static final DefaultSpannerFactory INSTANCE = new DefaultSpannerFactory(); @Override @@ -465,6 +470,7 @@ public Spanner create(SpannerOptions serviceOptions) { /** Default implementation of {@code SpannerRpcFactory}. */ private static class DefaultSpannerRpcFactory implements SpannerRpcFactory { + private static final DefaultSpannerRpcFactory INSTANCE = new DefaultSpannerRpcFactory(); @Override @@ -477,6 +483,7 @@ public ServiceRpc create(SpannerOptions options) { /** {@link ExecutorProvider} that is used for {@link AsyncResultSet}. */ public interface CloseableExecutorProvider extends ExecutorProvider, AutoCloseable { + /** Overridden to suppress the throws declaration of the super interface. */ @Override void close(); @@ -487,6 +494,7 @@ public interface CloseableExecutorProvider extends ExecutorProvider, AutoCloseab * ScheduledExecutorService}. */ public static class FixedCloseableExecutorProvider implements CloseableExecutorProvider { + private final ScheduledExecutorService executor; private FixedCloseableExecutorProvider(ScheduledExecutorService executor) { @@ -604,6 +612,7 @@ private SpannerOptions(Builder builder) { * variables. */ public interface SpannerEnvironment { + /** * The optimizer version to use. Must return an empty string to indicate that no value has been * set. @@ -626,6 +635,7 @@ default String getOptimizerStatisticsPackage() { * variables. */ private static class SpannerEnvironmentImpl implements SpannerEnvironment { + private static final SpannerEnvironmentImpl INSTANCE = new SpannerEnvironmentImpl(); private static final String SPANNER_OPTIMIZER_VERSION_ENV_VAR = "SPANNER_OPTIMIZER_VERSION"; private static final String SPANNER_OPTIMIZER_STATISTICS_PACKAGE_ENV_VAR = @@ -648,6 +658,7 @@ public String getOptimizerStatisticsPackage() { /** Builder for {@link SpannerOptions} instances. */ public static class Builder extends ServiceOptions.Builder { + static final int DEFAULT_PREFETCH_CHUNKS = 4; static final QueryOptions DEFAULT_QUERY_OPTIONS = QueryOptions.getDefaultInstance(); static final RetrySettings DEFAULT_ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS = @@ -1175,6 +1186,7 @@ public SpannerOptions build() { } // Set the number of channels if not set if (this.numChannels == null) { + /** By default, we create 4 channels per {@link SpannerOptions} */ this.numChannels = DEFAULT_NUM_CHANNELS; } From 7a3ed3c89e2f1f9e414ce99edbf1c179432225dc Mon Sep 17 00:00:00 2001 From: sriharshach Date: Tue, 6 Sep 2022 10:05:06 +0000 Subject: [PATCH 05/16] fix lint issue --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index bcb1f610cc..590f2e7874 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -682,9 +682,7 @@ public static class Builder private GrpcInterceptorProvider interceptorProvider; - /** - * By default, we create 4 channels per {@link SpannerOptions} - */ + /** By default, we create 4 channels per {@link SpannerOptions} */ private Integer numChannels; private String transportChannelExecutorThreadNameFormat = "Cloud-Spanner-TransportChannel-%d"; From 1449429920835cfac3aaa23aaf1cf12674e8832d Mon Sep 17 00:00:00 2001 From: sriharshach Date: Wed, 7 Sep 2022 11:15:16 +0000 Subject: [PATCH 06/16] code fixes --- .../google/cloud/spanner/SpannerOptions.java | 14 +++++------ .../cloud/spanner/SpannerOptionsTest.java | 25 ++++++++++--------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 590f2e7874..abb33f899e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -90,8 +90,8 @@ public class SpannerOptions extends ServiceOptions { "https://www.googleapis.com/auth/spanner.admin", "https://www.googleapis.com/auth/spanner.data"); private static final int MAX_CHANNELS = 256; - private static final int DEFAULT_NUM_CHANNELS = 4; - private static final int GRPC_GCP_ENABLED_DEFAULT_NUM_CHANNELS = 8; + private static final int DEFAULT_CHANNELS = 4; + @VisibleForTesting static final int GRPC_GCP_ENABLED_DEFAULT_CHANNELS = 8; private final TransportChannelProvider channelProvider; @SuppressWarnings("rawtypes") @@ -682,7 +682,6 @@ public static class Builder private GrpcInterceptorProvider interceptorProvider; - /** By default, we create 4 channels per {@link SpannerOptions} */ private Integer numChannels; private String transportChannelExecutorThreadNameFormat = "Cloud-Spanner-TransportChannel-%d"; @@ -1145,9 +1144,10 @@ public Builder enableGrpcGcpExtension() { public Builder enableGrpcGcpExtension(GcpManagedChannelOptions options) { this.grpcGcpExtensionEnabled = true; this.grpcGcpOptions = options; - // By default, set number of channels to 8 if grpc-gcp extension is enabled + // By default, set number of channels to GRPC_GCP_ENABLED_DEFAULT_CHANNELS if gRPC-GCP + // extension is enabled if (this.numChannels == null) { - this.numChannels = GRPC_GCP_ENABLED_DEFAULT_NUM_CHANNELS; + this.numChannels = GRPC_GCP_ENABLED_DEFAULT_CHANNELS; } return this; } @@ -1184,8 +1184,8 @@ public SpannerOptions build() { } // Set the number of channels if not set if (this.numChannels == null) { - /** By default, we create 4 channels per {@link SpannerOptions} */ - this.numChannels = DEFAULT_NUM_CHANNELS; + // By default, we create 4 channels per {@link SpannerOptions} + this.numChannels = DEFAULT_CHANNELS; } return new SpannerOptions(this); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 2ddf9be063..ff7618b339 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -921,34 +921,35 @@ public void testCustomAsyncExecutorProvider() { @Test public void testDefaultNumChannelsWithGrpcGcpExtensionEnabled() { - SpannerOptions.Builder builder = - SpannerOptions.newBuilder().setProjectId("test-project").enableGrpcGcpExtension(); + SpannerOptions options = + SpannerOptions.newBuilder().setProjectId("test-project").enableGrpcGcpExtension().build(); - SpannerOptions options = builder.build(); - assertEquals(options.getNumChannels(), 8); + assertEquals(options.getNumChannels(), SpannerOptions.GRPC_GCP_ENABLED_DEFAULT_CHANNELS); } @Test public void testNumChannelsWithGrpcGcpExtensionEnabled() { - // Set number of channels before enabling grpc-gcp channel pool + // Set number of channels explicitly, before enabling gRPC-GCP channel pool in SpannerOptions + // builder. int numChannels = 5; - SpannerOptions.Builder builder1 = + SpannerOptions options1 = SpannerOptions.newBuilder() .setProjectId("test-project") .setNumChannels(numChannels) - .enableGrpcGcpExtension(); + .enableGrpcGcpExtension() + .build(); - SpannerOptions options1 = builder1.build(); assertEquals(options1.getNumChannels(), numChannels); - // Set number of channels after enabling grpc-gcp channel pool - SpannerOptions.Builder builder2 = + // Set number of channels explicitly, after enabling gRPC-GCP channel pool in SpannerOptions + // builder. + SpannerOptions options2 = SpannerOptions.newBuilder() .setProjectId("test-project") .enableGrpcGcpExtension() - .setNumChannels(numChannels); + .setNumChannels(numChannels) + .build(); - SpannerOptions options2 = builder2.build(); assertEquals(options2.getNumChannels(), numChannels); } } From 5f516cb7a8b7925416858b09cd798c59173d1432 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta <93644539+rajatbhatta@users.noreply.github.com> Date: Wed, 7 Sep 2022 19:21:52 +0530 Subject: [PATCH 07/16] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java --- .../src/main/java/com/google/cloud/spanner/SpannerOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index abb33f899e..5143a82ace 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1182,7 +1182,7 @@ public SpannerOptions build() { // As we are using plain text, we should never send any credentials. this.setCredentials(NoCredentials.getInstance()); } - // Set the number of channels if not set + // Set the number of channels to DEFAULT_CHANNELS if not set if (this.numChannels == null) { // By default, we create 4 channels per {@link SpannerOptions} this.numChannels = DEFAULT_CHANNELS; From afec666fa3382a7fbbb621304d7cd6bc1bed3764 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta <93644539+rajatbhatta@users.noreply.github.com> Date: Wed, 7 Sep 2022 19:21:58 +0530 Subject: [PATCH 08/16] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java --- .../src/main/java/com/google/cloud/spanner/SpannerOptions.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 5143a82ace..18abb920f1 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1184,7 +1184,6 @@ public SpannerOptions build() { } // Set the number of channels to DEFAULT_CHANNELS if not set if (this.numChannels == null) { - // By default, we create 4 channels per {@link SpannerOptions} this.numChannels = DEFAULT_CHANNELS; } From 1fe3d48620cdce7980a73bb9633112b13d2d816c Mon Sep 17 00:00:00 2001 From: sriharshach Date: Thu, 8 Sep 2022 10:49:42 +0000 Subject: [PATCH 09/16] lint fixes --- .../java/com/google/cloud/spanner/SpannerOptions.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index abb33f899e..bb23866c2d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -74,7 +74,6 @@ /** Options for the Cloud Spanner service. */ public class SpannerOptions extends ServiceOptions { - private static final long serialVersionUID = 2789571558532701170L; private static SpannerEnvironment environment = SpannerEnvironmentImpl.INSTANCE; @@ -136,7 +135,6 @@ public class SpannerOptions extends ServiceOptions { * {@link SpannerOptions}. */ public interface CallCredentialsProvider { - /** Return the {@link CallCredentials} to use for a gRPC call. */ CallCredentials getCallCredentials(); } @@ -191,7 +189,6 @@ public interface CallCredentialsProvider { * } */ public interface CallContextConfigurator { - /** * Configure a {@link ApiCallContext} for a specific RPC call. * @@ -312,7 +309,6 @@ static SpannerMethod valueOf(ReqT request, MethodDescriptor */ public static class SpannerCallContextTimeoutConfigurator implements CallContextConfigurator { - private Duration commitTimeout; private Duration rollbackTimeout; @@ -459,7 +455,6 @@ public SpannerCallContextTimeoutConfigurator withPartitionReadTimeout( /** Default implementation of {@code SpannerFactory}. */ private static class DefaultSpannerFactory implements SpannerFactory { - private static final DefaultSpannerFactory INSTANCE = new DefaultSpannerFactory(); @Override @@ -470,7 +465,6 @@ public Spanner create(SpannerOptions serviceOptions) { /** Default implementation of {@code SpannerRpcFactory}. */ private static class DefaultSpannerRpcFactory implements SpannerRpcFactory { - private static final DefaultSpannerRpcFactory INSTANCE = new DefaultSpannerRpcFactory(); @Override @@ -483,7 +477,6 @@ public ServiceRpc create(SpannerOptions options) { /** {@link ExecutorProvider} that is used for {@link AsyncResultSet}. */ public interface CloseableExecutorProvider extends ExecutorProvider, AutoCloseable { - /** Overridden to suppress the throws declaration of the super interface. */ @Override void close(); @@ -494,7 +487,6 @@ public interface CloseableExecutorProvider extends ExecutorProvider, AutoCloseab * ScheduledExecutorService}. */ public static class FixedCloseableExecutorProvider implements CloseableExecutorProvider { - private final ScheduledExecutorService executor; private FixedCloseableExecutorProvider(ScheduledExecutorService executor) { @@ -612,7 +604,6 @@ private SpannerOptions(Builder builder) { * variables. */ public interface SpannerEnvironment { - /** * The optimizer version to use. Must return an empty string to indicate that no value has been * set. @@ -635,7 +626,6 @@ default String getOptimizerStatisticsPackage() { * variables. */ private static class SpannerEnvironmentImpl implements SpannerEnvironment { - private static final SpannerEnvironmentImpl INSTANCE = new SpannerEnvironmentImpl(); private static final String SPANNER_OPTIMIZER_VERSION_ENV_VAR = "SPANNER_OPTIMIZER_VERSION"; private static final String SPANNER_OPTIMIZER_STATISTICS_PACKAGE_ENV_VAR = @@ -658,7 +648,6 @@ public String getOptimizerStatisticsPackage() { /** Builder for {@link SpannerOptions} instances. */ public static class Builder extends ServiceOptions.Builder { - static final int DEFAULT_PREFETCH_CHUNKS = 4; static final QueryOptions DEFAULT_QUERY_OPTIONS = QueryOptions.getDefaultInstance(); static final RetrySettings DEFAULT_ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS = From e68b2f59d9e997eed15a87f9466424c46a57ffad Mon Sep 17 00:00:00 2001 From: harshachinta Date: Thu, 6 Oct 2022 15:03:05 +0000 Subject: [PATCH 10/16] Modify comments for better code readability --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 4ab01ee59f..952ab4545d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -90,6 +90,8 @@ public class SpannerOptions extends ServiceOptions { "https://www.googleapis.com/auth/spanner.data"); private static final int MAX_CHANNELS = 256; private static final int DEFAULT_CHANNELS = 4; + // Set the default number of channels to GRPC_GCP_ENABLED_DEFAULT_CHANNELS when gRPC-GCP extension is enabled, + // to make sure there are sufficient channels available to move the sessions to a different channel if a network connection in a particular channel fails. @VisibleForTesting static final int GRPC_GCP_ENABLED_DEFAULT_CHANNELS = 8; private final TransportChannelProvider channelProvider; @@ -1133,8 +1135,6 @@ public Builder enableGrpcGcpExtension() { public Builder enableGrpcGcpExtension(GcpManagedChannelOptions options) { this.grpcGcpExtensionEnabled = true; this.grpcGcpOptions = options; - // By default, set number of channels to GRPC_GCP_ENABLED_DEFAULT_CHANNELS if gRPC-GCP - // extension is enabled if (this.numChannels == null) { this.numChannels = GRPC_GCP_ENABLED_DEFAULT_CHANNELS; } @@ -1171,7 +1171,6 @@ public SpannerOptions build() { // As we are using plain text, we should never send any credentials. this.setCredentials(NoCredentials.getInstance()); } - // Set the number of channels to DEFAULT_CHANNELS if not set if (this.numChannels == null) { this.numChannels = DEFAULT_CHANNELS; } From a334e09e3206c08f313984e35ab66c36501c8673 Mon Sep 17 00:00:00 2001 From: harshachinta Date: Thu, 6 Oct 2022 15:21:11 +0000 Subject: [PATCH 11/16] lint fix --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 952ab4545d..574251b18c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -90,8 +90,10 @@ public class SpannerOptions extends ServiceOptions { "https://www.googleapis.com/auth/spanner.data"); private static final int MAX_CHANNELS = 256; private static final int DEFAULT_CHANNELS = 4; - // Set the default number of channels to GRPC_GCP_ENABLED_DEFAULT_CHANNELS when gRPC-GCP extension is enabled, - // to make sure there are sufficient channels available to move the sessions to a different channel if a network connection in a particular channel fails. + // Set the default number of channels to GRPC_GCP_ENABLED_DEFAULT_CHANNELS when gRPC-GCP extension + // is enabled, + // to make sure there are sufficient channels available to move the sessions to a different + // channel if a network connection in a particular channel fails. @VisibleForTesting static final int GRPC_GCP_ENABLED_DEFAULT_CHANNELS = 8; private final TransportChannelProvider channelProvider; From 27962ea00ebb7883d8c479f7668fdbd92cb02001 Mon Sep 17 00:00:00 2001 From: harshachinta Date: Tue, 11 Oct 2022 13:35:29 +0000 Subject: [PATCH 12/16] add test to verify default num of channels when gRPC-GCP is not enabled --- .../com/google/cloud/spanner/SpannerOptions.java | 7 +++---- .../google/cloud/spanner/SpannerOptionsTest.java | 13 ++++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 574251b18c..e71e98b85e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -89,11 +89,10 @@ public class SpannerOptions extends ServiceOptions { "https://www.googleapis.com/auth/spanner.admin", "https://www.googleapis.com/auth/spanner.data"); private static final int MAX_CHANNELS = 256; - private static final int DEFAULT_CHANNELS = 4; + @VisibleForTesting static final int DEFAULT_CHANNELS = 4; // Set the default number of channels to GRPC_GCP_ENABLED_DEFAULT_CHANNELS when gRPC-GCP extension - // is enabled, - // to make sure there are sufficient channels available to move the sessions to a different - // channel if a network connection in a particular channel fails. + // is enabled, to make sure there are sufficient channels available to move the sessions to a + // different channel if a network connection in a particular channel fails. @VisibleForTesting static final int GRPC_GCP_ENABLED_DEFAULT_CHANNELS = 8; private final TransportChannelProvider channelProvider; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index ff7618b339..dd7a9b8a03 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -924,7 +924,14 @@ public void testDefaultNumChannelsWithGrpcGcpExtensionEnabled() { SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").enableGrpcGcpExtension().build(); - assertEquals(options.getNumChannels(), SpannerOptions.GRPC_GCP_ENABLED_DEFAULT_CHANNELS); + assertEquals(SpannerOptions.GRPC_GCP_ENABLED_DEFAULT_CHANNELS, options.getNumChannels()); + } + + @Test + public void testDefaultNumChannelsWithGrpcGcpExtensionDisabled() { + SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build(); + + assertEquals(SpannerOptions.DEFAULT_CHANNELS, options.getNumChannels()); } @Test @@ -939,7 +946,7 @@ public void testNumChannelsWithGrpcGcpExtensionEnabled() { .enableGrpcGcpExtension() .build(); - assertEquals(options1.getNumChannels(), numChannels); + assertEquals(numChannels, options1.getNumChannels()); // Set number of channels explicitly, after enabling gRPC-GCP channel pool in SpannerOptions // builder. @@ -950,6 +957,6 @@ public void testNumChannelsWithGrpcGcpExtensionEnabled() { .setNumChannels(numChannels) .build(); - assertEquals(options2.getNumChannels(), numChannels); + assertEquals(numChannels, options2.getNumChannels()); } } From 68e72d4cb33a52837461898c176f03dc650715bd Mon Sep 17 00:00:00 2001 From: harshachinta Date: Tue, 18 Oct 2022 09:21:03 +0000 Subject: [PATCH 13/16] Add tests to verify number of channels and the instances created --- .../cloud/spanner/SpannerOptionsTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index dd7a9b8a03..55a15809a4 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -20,6 +20,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; @@ -959,4 +960,31 @@ public void testNumChannelsWithGrpcGcpExtensionEnabled() { assertEquals(numChannels, options2.getNumChannels()); } + + @Test + public void checkCreatedInstanceWhenGrpcGcpExtensionDisabled() { + SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build(); + SpannerOptions options1 = options.toBuilder().build(); + assertEquals(false, options.isGrpcGcpExtensionEnabled()); + assertEquals(options.isGrpcGcpExtensionEnabled(), options1.isGrpcGcpExtensionEnabled()); + + Spanner spanner1 = options.getService(); + Spanner spanner2 = options1.getService(); + + assertNotSame(spanner1, spanner2); + } + + @Test + public void checkCreatedInstanceWhenGrpcGcpExtensionEnabled() { + SpannerOptions options = + SpannerOptions.newBuilder().setProjectId("test-project").enableGrpcGcpExtension().build(); + SpannerOptions options1 = options.toBuilder().build(); + assertEquals(true, options.isGrpcGcpExtensionEnabled()); + assertEquals(options.isGrpcGcpExtensionEnabled(), options1.isGrpcGcpExtensionEnabled()); + + Spanner spanner1 = options.getService(); + Spanner spanner2 = options1.getService(); + + assertNotSame(spanner1, spanner2); + } } From 33682fcc32735046a45fbc4a18173282ec1d61cc Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 18 Oct 2022 11:28:50 +0000 Subject: [PATCH 14/16] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 096d382434..ea02aaff81 100644 --- a/README.md +++ b/README.md @@ -49,20 +49,20 @@ If you are using Maven without BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.1.1') +implementation platform('com.google.cloud:libraries-bom:26.1.3') implementation 'com.google.cloud:google-cloud-spanner' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-spanner:6.29.1' +implementation 'com.google.cloud:google-cloud-spanner:6.31.2' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.29.1" +libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.31.2" ``` ## Authentication From faa74edd4d0d24913646f80f2c17bb060e3ddf4a Mon Sep 17 00:00:00 2001 From: harshachinta Date: Wed, 19 Oct 2022 09:53:06 +0000 Subject: [PATCH 15/16] code refactoring for setting the default numChannels --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index e71e98b85e..3747255e8f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1136,9 +1136,6 @@ public Builder enableGrpcGcpExtension() { public Builder enableGrpcGcpExtension(GcpManagedChannelOptions options) { this.grpcGcpExtensionEnabled = true; this.grpcGcpOptions = options; - if (this.numChannels == null) { - this.numChannels = GRPC_GCP_ENABLED_DEFAULT_CHANNELS; - } return this; } @@ -1173,7 +1170,8 @@ public SpannerOptions build() { this.setCredentials(NoCredentials.getInstance()); } if (this.numChannels == null) { - this.numChannels = DEFAULT_CHANNELS; + this.numChannels = this.grpcGcpExtensionEnabled ? + GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; } return new SpannerOptions(this); From a88d4e3d3a9aee0e026d03da95cfa684db45b4e7 Mon Sep 17 00:00:00 2001 From: harshachinta Date: Wed, 19 Oct 2022 11:41:03 +0000 Subject: [PATCH 16/16] lint fix --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 3747255e8f..6b06f7d4fb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1170,8 +1170,8 @@ public SpannerOptions build() { this.setCredentials(NoCredentials.getInstance()); } if (this.numChannels == null) { - this.numChannels = this.grpcGcpExtensionEnabled ? - GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; + this.numChannels = + this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; } return new SpannerOptions(this);