diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 2995730664e2..9c542bc52365 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1041,7 +1041,7 @@ default boolean isEnableDirectAccess() { } default boolean isEnableGcpFallback() { - return false; + return true; } default boolean isEnableBuiltInMetrics() { @@ -1136,7 +1136,8 @@ public boolean isEnableDirectAccess() { @Override public boolean isEnableGcpFallback() { - return Boolean.parseBoolean(System.getenv(GOOGLE_SPANNER_ENABLE_GCP_FALLBACK)); + String enableGcpFallback = System.getenv(GOOGLE_SPANNER_ENABLE_GCP_FALLBACK); + return enableGcpFallback == null ? true : Boolean.parseBoolean(enableGcpFallback); } @Override diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 7c1b6be1c1bd..f91e2dcfd52a 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -230,7 +230,6 @@ import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -568,6 +567,7 @@ GcpFallbackChannelOptions createFallbackChannelOptions( .setPrimaryChannelName("directpath") .setFallbackChannelName("cloudpath") .setMinFailedCalls(minFailedCalls) + .setPeriod(Duration.ofMinutes(3)) .setGcpFallbackOpenTelemetry(fallbackTelemetry) .build(); } @@ -640,27 +640,13 @@ private void setupGcpFallback( createChannelProviderBuilder( options, headerProviderWithUserAgent, /* isEnableDirectAccess= */ false); - final ApiFunction existingCloudPathConfigurator = - cloudPathProviderBuilder.getChannelConfigurator(); - final AtomicReference cloudPathBuilderRef = new AtomicReference<>(); - cloudPathProviderBuilder.setChannelConfigurator( - builder -> { - ManagedChannelBuilder effectiveBuilder = builder; - if (existingCloudPathConfigurator != null) { - effectiveBuilder = existingCloudPathConfigurator.apply(effectiveBuilder); - } - cloudPathBuilderRef.set(effectiveBuilder); - return effectiveBuilder; - }); - - // Build the cloudPathProvider to extract the builder which will be provided to - // FallbackChannelBuilder. - try (TransportChannel ignored = cloudPathProviderBuilder.build().getTransportChannel()) { - } catch (Exception e) { + InstantiatingGrpcChannelProvider cloudPathProvider = cloudPathProviderBuilder.build(); + ManagedChannelBuilder cloudPathBuilder; + try { + cloudPathBuilder = cloudPathProvider.createDecoratedChannelBuilder(); + } catch (IOException e) { throw asSpannerException(e); } - - ManagedChannelBuilder cloudPathBuilder = cloudPathBuilderRef.get(); if (cloudPathBuilder == null) { throw new IllegalStateException("CloudPath builder was not captured."); } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java index 165557608ac3..823e24b64f9c 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java @@ -1347,61 +1347,49 @@ public void testFallbackIntegration_doesNotSwitchWhenThresholdNotMet() throws Ex OpenTelemetrySdk openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build(); - SpannerOptions.useEnvironment( - new SpannerOptions.SpannerEnvironment() { - @Override - public boolean isEnableGcpFallback() { - return true; - } - }); + SpannerOptions.Builder builder = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setEnableDirectAccess(true) + .setHost("http://localhost:1") // Closed port + .setCredentials(NoCredentials.getInstance()) + .setOpenTelemetry(openTelemetry); + // Make sure the ExecuteBatchDml RPC fails quickly to keep the test fast. + // Note that the timeout is actually not used. It is the fact that it does not retry that + // makes it fail fast. + builder + .getSpannerStubSettingsBuilder() + .executeBatchDmlSettings() + .setSimpleTimeoutNoRetriesDuration(Duration.ofSeconds(10)); + // Setup Options with invalid host to force error + SpannerOptions options = builder.build(); + + TestableGapicSpannerRpc rpc = new TestableGapicSpannerRpc(options); try { - SpannerOptions.Builder builder = - SpannerOptions.newBuilder() - .setProjectId("test-project") - .setEnableDirectAccess(true) - .setHost("http://localhost:1") // Closed port - .setCredentials(NoCredentials.getInstance()) - .setOpenTelemetry(openTelemetry); - // Make sure the ExecuteBatchDml RPC fails quickly to keep the test fast. - // Note that the timeout is actually not used. It is the fact that it does not retry that - // makes it fail fast. - builder - .getSpannerStubSettingsBuilder() - .executeBatchDmlSettings() - .setSimpleTimeoutNoRetriesDuration(Duration.ofSeconds(10)); - // Setup Options with invalid host to force error - SpannerOptions options = builder.build(); - - TestableGapicSpannerRpc rpc = new TestableGapicSpannerRpc(options); - try { - // Make a call that is expected to fail - SpannerException exception = - assertThrows( - SpannerException.class, - () -> - rpc.executeBatchDml( - com.google.spanner.v1.ExecuteBatchDmlRequest.newBuilder() - .setSession("projects/p/instances/i/databases/d/sessions/s") - .build(), - null)); - assertEquals(ErrorCode.UNAVAILABLE, exception.getErrorCode()); - - // Wait briefly for the 10ms period to trigger the fallback check - Thread.sleep(10); - - // Verify Fallback via Metrics - Collection metrics = metricReader.collectAllMetrics(); - boolean fallbackOccurred = - metrics.stream() - .anyMatch(md -> md.getName().contains("fallback_count") && hasValue(md)); - - assertFalse("Fallback metric should not be present", fallbackOccurred); - - } finally { - rpc.shutdown(); - } + // Make a call that is expected to fail + SpannerException exception = + assertThrows( + SpannerException.class, + () -> + rpc.executeBatchDml( + com.google.spanner.v1.ExecuteBatchDmlRequest.newBuilder() + .setSession("projects/p/instances/i/databases/d/sessions/s") + .build(), + null)); + assertEquals(ErrorCode.UNAVAILABLE, exception.getErrorCode()); + + // Wait briefly for the 10ms period to trigger the fallback check + Thread.sleep(10); + + // Verify Fallback via Metrics + Collection metrics = metricReader.collectAllMetrics(); + boolean fallbackOccurred = + metrics.stream().anyMatch(md -> md.getName().contains("fallback_count") && hasValue(md)); + + assertFalse("Fallback metric should not be present", fallbackOccurred); + } finally { - SpannerOptions.useDefaultEnvironment(); + rpc.shutdown(); } } @@ -1438,64 +1426,52 @@ public void testFallbackIntegration_switchesToFallbackOnFailure() throws Excepti OpenTelemetrySdk openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build(); - SpannerOptions.useEnvironment( - new SpannerOptions.SpannerEnvironment() { - @Override - public boolean isEnableGcpFallback() { - return true; - } - }); + SpannerOptions.Builder builder = + SpannerOptions.newBuilder() + .setProjectId("test-project") + .setEnableDirectAccess(true) + .setHost("http://localhost:1") // Closed port + .setCredentials(NoCredentials.getInstance()) + .setOpenTelemetry(openTelemetry); + // Make sure the ExecuteBatchDml RPC fails quickly to keep the test fast. + // Note that the timeout is actually not used. It is the fact that it does not retry that + // makes it fail fast. + builder + .getSpannerStubSettingsBuilder() + .executeBatchDmlSettings() + .setSimpleTimeoutNoRetriesDuration(Duration.ofSeconds(10)); + // Setup Options with invalid host to force error + SpannerOptions options = builder.build(); + + TestableGapicSpannerRpcWithLowerMinFailedCalls rpc = + new TestableGapicSpannerRpcWithLowerMinFailedCalls(options); try { - SpannerOptions.Builder builder = - SpannerOptions.newBuilder() - .setProjectId("test-project") - .setEnableDirectAccess(true) - .setHost("http://localhost:1") // Closed port - .setCredentials(NoCredentials.getInstance()) - .setOpenTelemetry(openTelemetry); - // Make sure the ExecuteBatchDml RPC fails quickly to keep the test fast. - // Note that the timeout is actually not used. It is the fact that it does not retry that - // makes it fail fast. - builder - .getSpannerStubSettingsBuilder() - .executeBatchDmlSettings() - .setSimpleTimeoutNoRetriesDuration(Duration.ofSeconds(10)); - // Setup Options with invalid host to force error - SpannerOptions options = builder.build(); - - TestableGapicSpannerRpcWithLowerMinFailedCalls rpc = - new TestableGapicSpannerRpcWithLowerMinFailedCalls(options); - try { - // Make a call that is expected to fail - SpannerException exception = - assertThrows( - SpannerException.class, - () -> - rpc.executeBatchDml( - com.google.spanner.v1.ExecuteBatchDmlRequest.newBuilder() - .setSession("projects/p/instances/i/databases/d/sessions/s") - .build(), - null)); - assertEquals(ErrorCode.UNAVAILABLE, exception.getErrorCode()); - - // Wait briefly for the 10ms period to trigger the fallback check - Thread.sleep(10); - - // Verify Fallback via Metrics - Collection metrics = metricReader.collectAllMetrics(); - boolean fallbackOccurred = - metrics.stream() - .anyMatch(md -> md.getName().contains("fallback_count") && hasValue(md)); - - assertTrue( - "Fallback metric should be present, indicating GcpFallbackChannel is active", - fallbackOccurred); - - } finally { - rpc.shutdown(); - } + // Make a call that is expected to fail + SpannerException exception = + assertThrows( + SpannerException.class, + () -> + rpc.executeBatchDml( + com.google.spanner.v1.ExecuteBatchDmlRequest.newBuilder() + .setSession("projects/p/instances/i/databases/d/sessions/s") + .build(), + null)); + assertEquals(ErrorCode.UNAVAILABLE, exception.getErrorCode()); + + // Wait briefly for the 10ms period to trigger the fallback check + Thread.sleep(10); + + // Verify Fallback via Metrics + Collection metrics = metricReader.collectAllMetrics(); + boolean fallbackOccurred = + metrics.stream().anyMatch(md -> md.getName().contains("fallback_count") && hasValue(md)); + + assertTrue( + "Fallback metric should be present, indicating GcpFallbackChannel is active", + fallbackOccurred); + } finally { - SpannerOptions.useDefaultEnvironment(); + rpc.shutdown(); } }