enable GOOGLE_SPANNER_ENABLE_GCP_FALLBACK by default

Author: kinsaurralde

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

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

@@ -563,6 +563,7 @@ GcpFallbackChannelOptions createFallbackChannelOptions(
         .setPrimaryChannelName("directpath")
         .setFallbackChannelName("cloudpath")
         .setMinFailedCalls(minFailedCalls)
+        .setPeriod(Duration.ofMinutes(3))
         .setGcpFallbackOpenTelemetry(fallbackTelemetry)
         .build();
   }

java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java

@@ -1164,61 +1164,50 @@ 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<MetricData> 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<MetricData> 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();
     }
   }
 
@@ -1255,64 +1244,53 @@ 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<MetricData> 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<MetricData> 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();
     }
   }