diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index 3bebba5db2..05cfb3e5f6 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -166,6 +166,7 @@ com.google.cloud grpc-gcp + 1.7.0 io.grpc 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 765114dc68..cfbb5a86f7 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 @@ -151,7 +151,6 @@ public class SpannerOptions extends ServiceOptions { private final InstanceAdminStubSettings instanceAdminStubSettings; private final DatabaseAdminStubSettings databaseAdminStubSettings; private final Duration partitionedDmlTimeout; - private final boolean grpcGcpExtensionEnabled; private final GcpManagedChannelOptions grpcGcpOptions; private final boolean autoThrottleAdministrativeRequests; private final RetrySettings retryAdministrativeRequestsSettings; @@ -798,7 +797,6 @@ protected SpannerOptions(Builder builder) { throw SpannerExceptionFactory.newSpannerException(e); } partitionedDmlTimeout = builder.partitionedDmlTimeout; - grpcGcpExtensionEnabled = builder.grpcGcpExtensionEnabled; grpcGcpOptions = builder.grpcGcpOptions; autoThrottleAdministrativeRequests = builder.autoThrottleAdministrativeRequests; retryAdministrativeRequestsSettings = builder.retryAdministrativeRequestsSettings; @@ -1025,7 +1023,6 @@ public static class Builder private DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder = DatabaseAdminStubSettings.newBuilder(); private Duration partitionedDmlTimeout = Duration.ofHours(2L); - private boolean grpcGcpExtensionEnabled = false; private GcpManagedChannelOptions grpcGcpOptions; private RetrySettings retryAdministrativeRequestsSettings = DEFAULT_ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS; @@ -1097,7 +1094,6 @@ protected Builder() { this.instanceAdminStubSettingsBuilder = options.instanceAdminStubSettings.toBuilder(); this.databaseAdminStubSettingsBuilder = options.databaseAdminStubSettings.toBuilder(); this.partitionedDmlTimeout = options.partitionedDmlTimeout; - this.grpcGcpExtensionEnabled = options.grpcGcpExtensionEnabled; this.grpcGcpOptions = options.grpcGcpOptions; this.autoThrottleAdministrativeRequests = options.autoThrottleAdministrativeRequests; this.retryAdministrativeRequestsSettings = options.retryAdministrativeRequestsSettings; @@ -1573,14 +1569,12 @@ public Builder enableGrpcGcpExtension() { * Multiplexed sessions are not supported for gRPC-GCP. */ public Builder enableGrpcGcpExtension(GcpManagedChannelOptions options) { - this.grpcGcpExtensionEnabled = true; this.grpcGcpOptions = options; return this; } /** Disables gRPC-GCP extension. */ public Builder disableGrpcGcpExtension() { - this.grpcGcpExtensionEnabled = false; return this; } @@ -1793,8 +1787,7 @@ public SpannerOptions build() { credentials = environment.getDefaultExperimentalHostCredentials(); } if (this.numChannels == null) { - this.numChannels = - this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; + this.numChannels = GRPC_GCP_ENABLED_DEFAULT_CHANNELS; } synchronized (lock) { @@ -1989,7 +1982,7 @@ public Duration getPartitionedDmlTimeoutDuration() { } public boolean isGrpcGcpExtensionEnabled() { - return grpcGcpExtensionEnabled; + return true; } public GcpManagedChannelOptions getGrpcGcpOptions() { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 371d73a0af..64b11f7a46 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -279,7 +279,6 @@ public class GapicSpannerRpc implements SpannerRpc { private final boolean leaderAwareRoutingEnabled; private final boolean endToEndTracingEnabled; private final int numChannels; - private final boolean isGrpcGcpExtensionEnabled; private final GrpcCallContext baseGrpcCallContext; @@ -335,7 +334,6 @@ public GapicSpannerRpc(final SpannerOptions options) { this.leaderAwareRoutingEnabled = options.isLeaderAwareRoutingEnabled(); this.endToEndTracingEnabled = options.isEndToEndTracingEnabled(); this.numChannels = options.getNumChannels(); - this.isGrpcGcpExtensionEnabled = options.isGrpcGcpExtensionEnabled(); this.baseGrpcCallContext = createBaseCallContext(); if (initializeStubs) { @@ -592,10 +590,6 @@ private static GcpManagedChannelOptions grpcGcpOptionsWithMetrics(SpannerOptions private static void maybeEnableGrpcGcpExtension( InstantiatingGrpcChannelProvider.Builder defaultChannelProviderBuilder, final SpannerOptions options) { - if (!options.isGrpcGcpExtensionEnabled()) { - return; - } - final String jsonApiConfig = parseGrpcGcpApiConfig(); final GcpManagedChannelOptions grpcGcpOptions = grpcGcpOptionsWithMetrics(options); @@ -2037,20 +2031,14 @@ GrpcCallContext newCallContext( GrpcCallContext context = this.baseGrpcCallContext; Long affinity = options == null ? null : Option.CHANNEL_HINT.getLong(options); if (affinity != null) { - if (this.isGrpcGcpExtensionEnabled) { - // Set channel affinity in gRPC-GCP. - // Compute bounded channel hint to prevent gRPC-GCP affinity map from getting unbounded. - int boundedChannelHint = affinity.intValue() % this.numChannels; - context = - context.withCallOptions( - context - .getCallOptions() - .withOption( - GcpManagedChannel.AFFINITY_KEY, String.valueOf(boundedChannelHint))); - } else { - // Set channel affinity in GAX. - context = context.withChannelAffinity(affinity.intValue()); - } + // Set channel affinity in gRPC-GCP. + // Compute bounded channel hint to prevent gRPC-GCP affinity map from getting unbounded. + int boundedChannelHint = affinity.intValue() % this.numChannels; + context = + context.withCallOptions( + context + .getCallOptions() + .withOption(GcpManagedChannel.AFFINITY_KEY, String.valueOf(boundedChannelHint))); } if (options != null) { // TODO(@odeke-em): Infer the affinity if it doesn't match up with in the request-id. diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java index a06eeb9166..01c4953272 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java @@ -70,13 +70,9 @@ public class ChannelUsageTest { @Parameter(0) public int numChannels; - @Parameter(1) - public boolean enableGcpPool; - - @Parameters(name = "num channels = {0}, enable GCP pool = {1}") + @Parameters(name = "num channels = {0}") public static Collection data() { - return Arrays.asList( - new Object[][] {{1, true}, {1, false}, {2, true}, {2, false}, {4, true}, {4, false}}); + return Arrays.asList(new Object[][] {{1}, {2}, {4}}); } private static final Statement SELECT1 = Statement.of("SELECT 1 AS COL1"); @@ -208,27 +204,10 @@ private SpannerOptions createSpannerOptions() { .build()) .setHost("http://" + endpoint) .setCredentials(NoCredentials.getInstance()); - if (enableGcpPool) { - builder.enableGrpcGcpExtension(); - } return builder.build(); } - @Test - public void testCreatesNumChannels() { - try (Spanner spanner = createSpannerOptions().getService()) { - assumeFalse( - "GRPC-GCP is currently not supported with multiplexed sessions", - isMultiplexedSessionsEnabled(spanner) && enableGcpPool); - DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d")); - try (ResultSet resultSet = client.singleUse().executeQuery(SELECT1)) { - while (resultSet.next()) {} - } - } - assertEquals(numChannels, batchCreateSessionLocalIps.size()); - } - @Test public void testUsesAllChannels() throws InterruptedException { final int multiplier = 2; 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 9fc065f944..08b2836223 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 @@ -1100,9 +1100,10 @@ public void testDefaultNumChannelsWithGrpcGcpExtensionDisabled() { SpannerOptions.newBuilder() .setProjectId("test-project") .setCredentials(NoCredentials.getInstance()) + .disableGrpcGcpExtension() .build(); - assertEquals(SpannerOptions.DEFAULT_CHANNELS, options.getNumChannels()); + assertEquals(SpannerOptions.GRPC_GCP_ENABLED_DEFAULT_CHANNELS, options.getNumChannels()); } @Test @@ -1135,9 +1136,10 @@ public void testNumChannelsWithGrpcGcpExtensionEnabled() { @Test public void checkCreatedInstanceWhenGrpcGcpExtensionDisabled() { - SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build(); + SpannerOptions options = + SpannerOptions.newBuilder().setProjectId("test-project").disableGrpcGcpExtension().build(); SpannerOptions options1 = options.toBuilder().build(); - assertEquals(false, options.isGrpcGcpExtensionEnabled()); + assertEquals(true, options.isGrpcGcpExtensionEnabled()); assertEquals(options.isGrpcGcpExtensionEnabled(), options1.isGrpcGcpExtensionEnabled()); Spanner spanner1 = options.getService(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionChannelHintTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionChannelHintTest.java index b68ef4667d..577af05579 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionChannelHintTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionChannelHintTest.java @@ -312,6 +312,7 @@ public void testTransactionRunner_usesSingleChannel() { return null; }); } + System.out.println("streamingReadLocalIps: " + streamingReadLocalIps); assertEquals(1, streamingReadLocalIps.size()); } }