From 44f27fc90fa3f9f4914574fb0476e971da4c02ff Mon Sep 17 00:00:00 2001 From: Sri Harsha CH <57220027+harshachinta@users.noreply.github.com> Date: Wed, 19 Oct 2022 14:18:20 +0000 Subject: [PATCH] feat: increase default number of channels when gRPC-GCP channel pool is enabled (#1997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * increase default number of channels when grpc-gcp channel pool is * 🦉 Updates from OwlBot post-processor Co-authored-by: Rajat Bhatta <93644539+rajatbhatta@users.noreply.github.com> Co-authored-by: Owl Bot --- .../google/cloud/spanner/SpannerOptions.java | 16 +++-- .../cloud/spanner/SpannerOptionsTest.java | 69 +++++++++++++++++++ 2 files changed, 81 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 b117750654..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 @@ -89,6 +89,11 @@ 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; + @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. + @VisibleForTesting static final int GRPC_GCP_ENABLED_DEFAULT_CHANNELS = 8; private final TransportChannelProvider channelProvider; @SuppressWarnings("rawtypes") @@ -669,8 +674,7 @@ public static class Builder private GrpcInterceptorProvider interceptorProvider; - /** By default, we create 4 channels per {@link SpannerOptions} */ - private int numChannels = 4; + private Integer numChannels; private String transportChannelExecutorThreadNameFormat = "Cloud-Spanner-TransportChannel-%d"; @@ -1122,8 +1126,7 @@ public Builder setHost(String host) { /** Enables gRPC-GCP extension with the default settings. */ public Builder enableGrpcGcpExtension() { - this.grpcGcpExtensionEnabled = true; - return this; + return this.enableGrpcGcpExtension(null); } /** @@ -1166,6 +1169,11 @@ public SpannerOptions build() { // As we are using plain text, we should never send any credentials. this.setCredentials(NoCredentials.getInstance()); } + if (this.numChannels == null) { + this.numChannels = + this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : 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 7d5c8a553a..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; @@ -918,4 +919,72 @@ public void testCustomAsyncExecutorProvider() { .build(); assertSame(service, options.getAsyncExecutorProvider().getExecutor()); } + + @Test + public void testDefaultNumChannelsWithGrpcGcpExtensionEnabled() { + SpannerOptions options = + SpannerOptions.newBuilder().setProjectId("test-project").enableGrpcGcpExtension().build(); + + 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 + public void testNumChannelsWithGrpcGcpExtensionEnabled() { + // Set number of channels explicitly, before enabling gRPC-GCP channel pool in SpannerOptions + // builder. + int numChannels = 5; + SpannerOptions options1 = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setNumChannels(numChannels) + .enableGrpcGcpExtension() + .build(); + + assertEquals(numChannels, options1.getNumChannels()); + + // Set number of channels explicitly, after enabling gRPC-GCP channel pool in SpannerOptions + // builder. + SpannerOptions options2 = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .enableGrpcGcpExtension() + .setNumChannels(numChannels) + .build(); + + 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); + } }