From ee9825c6827702b4cfddf8351f1962645491d948 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 7 Aug 2026 18:13:04 +0000 Subject: [PATCH 01/15] feat(grpc-gcp): add GcpFallbackState and probing recovery to GcpFallbackChannel --- .../grpc/fallback/GcpFallbackChannel.java | 229 +++++++---- .../fallback/GcpFallbackChannelOptions.java | 50 +++ .../cloud/grpc/fallback/GcpFallbackState.java | 209 ++++++++++ .../grpc/fallback/GcpFallbackChannelTest.java | 382 ++++++++++++++++++ 4 files changed, 786 insertions(+), 84 deletions(-) create mode 100644 grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index b991234848c1..d7c01ccce11a 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -20,7 +20,6 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.NANOSECONDS; -import com.google.cloud.grpc.GcpThreadFactory; import com.google.common.annotations.VisibleForTesting; import io.grpc.CallOptions; import io.grpc.Channel; @@ -31,9 +30,10 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -50,14 +50,17 @@ public class GcpFallbackChannel extends ManagedChannel { private final Channel primaryChannel; // Wrapped fallback channel to be used for RPCs. private final Channel fallbackChannel; - private final AtomicLong primarySuccesses = new AtomicLong(0); - private final AtomicLong primaryFailures = new AtomicLong(0); - private final AtomicLong fallbackSuccesses = new AtomicLong(0); - private final AtomicLong fallbackFailures = new AtomicLong(0); - private boolean inFallbackMode = false; + private final GcpFallbackState fallbackState; + private final boolean ownsFallbackState; private final GcpFallbackOpenTelemetry openTelemetry; + private final AtomicBoolean localInFallbackMode = new AtomicBoolean(false); + private final AtomicLong localProbeSuccesses = new AtomicLong(0); + private final AtomicLong localFirstPrimaryProbeSuccessNanos = new AtomicLong(0); + private final ScheduledExecutorService execService; + private ScheduledFuture primaryProbeFuture = null; + private ScheduledFuture fallbackProbeFuture = null; public GcpFallbackChannel( GcpFallbackChannelOptions options, @@ -82,13 +85,15 @@ public GcpFallbackChannel( checkNotNull(options); checkNotNull(primaryChannelBuilder); checkNotNull(fallbackChannelBuilder); - if (execService != null) { - this.execService = execService; + this.options = options; + if (options.getSharedState() != null) { + this.fallbackState = options.getSharedState(); + this.ownsFallbackState = false; } else { - this.execService = - Executors.newScheduledThreadPool(3, GcpThreadFactory.newThreadFactory("gcp-fallback-%d")); + this.fallbackState = new GcpFallbackState(); // Private state for backward compatibility + this.ownsFallbackState = true; } - this.options = options; + this.execService = fallbackState.getOrCreateExecutorService(execService, options); if (options.getGcpOpenTelemetry() != null) { this.openTelemetry = options.getGcpOpenTelemetry(); } else { @@ -149,13 +154,15 @@ public GcpFallbackChannel( checkNotNull(options); checkNotNull(primaryChannel); checkNotNull(fallbackChannel); - if (execService != null) { - this.execService = execService; + this.options = options; + if (options.getSharedState() != null) { + this.fallbackState = options.getSharedState(); + this.ownsFallbackState = false; } else { - this.execService = - Executors.newScheduledThreadPool(3, GcpThreadFactory.newThreadFactory("gcp-fallback-%d")); + this.fallbackState = new GcpFallbackState(); // Private state for backward compatibility + this.ownsFallbackState = true; } - this.options = options; + this.execService = fallbackState.getOrCreateExecutorService(execService, options); if (options.getGcpOpenTelemetry() != null) { this.openTelemetry = options.getGcpOpenTelemetry(); } else { @@ -175,105 +182,115 @@ public GcpFallbackChannel( } public boolean isInFallbackMode() { - return inFallbackMode || primaryChannel == null; + if (fallbackState.getInFallbackMode().get()) { + if (localInFallbackMode.compareAndSet(false, true)) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } + } + return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; + } + + @VisibleForTesting + GcpFallbackState getFallbackState() { + return fallbackState; + } + + @VisibleForTesting + AtomicBoolean getLocalInFallbackMode() { + return localInFallbackMode; + } + + @VisibleForTesting + AtomicLong getLocalProbeSuccesses() { + return localProbeSuccesses; } private void init() { if (options.getPrimaryProbingFunction() != null) { - execService.scheduleAtFixedRate( - this::probePrimary, - options.getPrimaryProbingInterval().toMillis(), - options.getPrimaryProbingInterval().toMillis(), - MILLISECONDS); + this.primaryProbeFuture = + fallbackState.scheduleTask( + this::probePrimary, + options.getPrimaryProbingInterval().toMillis(), + options.getPrimaryProbingInterval().toMillis(), + MILLISECONDS); } if (options.getFallbackProbingFunction() != null) { - execService.scheduleAtFixedRate( - this::probeFallback, - options.getFallbackProbingInterval().toMillis(), - options.getFallbackProbingInterval().toMillis(), - MILLISECONDS); - } - - if (options.isEnableFallback() - && options.getPeriod() != null - && options.getPeriod().toMillis() > 0) { - execService.scheduleAtFixedRate( - this::checkErrorRates, - options.getPeriod().toMillis(), - options.getPeriod().toMillis(), - MILLISECONDS); + this.fallbackProbeFuture = + fallbackState.scheduleTask( + this::probeFallback, + options.getFallbackProbingInterval().toMillis(), + options.getFallbackProbingInterval().toMillis(), + MILLISECONDS); } + + fallbackState.startPeriodicEvaluation(options, execService); } private void checkErrorRates() { - long successes = primarySuccesses.getAndSet(0); - long failures = primaryFailures.getAndSet(0); - float errRate = 0f; - if (failures + successes > 0) { - errRate = (float) failures / (failures + successes); - } - // Report primary error rate. - openTelemetry.getModule().reportErrorRate(options.getPrimaryChannelName(), errRate); - - if (!isInFallbackMode() && options.isEnableFallback() && fallbackChannel != null) { - if (failures >= options.getMinFailedCalls() && errRate >= options.getErrorRateThreshold()) { - if (inFallbackMode != true) { - openTelemetry - .getModule() - .reportFallback(options.getPrimaryChannelName(), options.getFallbackChannelName()); - } - inFallbackMode = true; - } - } - successes = fallbackSuccesses.getAndSet(0); - failures = fallbackFailures.getAndSet(0); - errRate = 0f; - if (failures + successes > 0) { - errRate = (float) failures / (failures + successes); - } - // Report fallback error rate. - openTelemetry.getModule().reportErrorRate(options.getFallbackChannelName(), errRate); - - openTelemetry - .getModule() - .reportCurrentChannel(options.getPrimaryChannelName(), inFallbackMode == false); - openTelemetry - .getModule() - .reportCurrentChannel(options.getFallbackChannelName(), inFallbackMode == true); + fallbackState.checkErrorRates(options, openTelemetry); } private void processPrimaryStatusCode(Status.Code statusCode) { if (options.getErroneousStates().contains(statusCode)) { - // Count error. - primaryFailures.incrementAndGet(); + fallbackState.getPrimaryFailures().incrementAndGet(); } else { - // Count success. - primarySuccesses.incrementAndGet(); + fallbackState.getPrimarySuccesses().incrementAndGet(); } - // Report status code. openTelemetry.getModule().reportStatus(options.getPrimaryChannelName(), statusCode); } private void processFallbackStatusCode(Status.Code statusCode) { if (options.getErroneousStates().contains(statusCode)) { - // Count error. - fallbackFailures.incrementAndGet(); + fallbackState.getFallbackFailures().incrementAndGet(); } else { - // Count success. - fallbackSuccesses.incrementAndGet(); + fallbackState.getFallbackSuccesses().incrementAndGet(); } - // Report status code. openTelemetry.getModule().reportStatus(options.getFallbackChannelName(), statusCode); } private void probePrimary() { + if (fallbackState.getInFallbackMode().get()) { + if (localInFallbackMode.compareAndSet(false, true)) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } + } + if (!localInFallbackMode.get() && primaryChannel != null) { + return; + } String result = ""; if (primaryDelegateChannel == null) { result = INIT_FAILURE_REASON; } else { result = options.getPrimaryProbingFunction().apply(primaryDelegateChannel); } + if ("OK".equals(result)) { + localFirstPrimaryProbeSuccessNanos.compareAndSet(0, System.nanoTime()); + long firstSuccessNanos = localFirstPrimaryProbeSuccessNanos.get(); + long primaryProbeSuccessCount = localProbeSuccesses.incrementAndGet(); + + boolean durationSatisfied = true; + if (options.getMinPrimaryProbeSuccessDuration() != null + && !options.getMinPrimaryProbeSuccessDuration().isZero() + && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { + long elapsedNanos = System.nanoTime() - firstSuccessNanos; + durationSatisfied = elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); + } + + if (primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() + && durationSatisfied) { + fallbackState.getInFallbackMode().set(false); + localInFallbackMode.set(false); + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } + } else { + localInFallbackMode.set(true); + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } // Report metric based on result. openTelemetry.getModule().reportProbeResult(options.getPrimaryChannelName(), result); } @@ -308,27 +325,71 @@ public String authority() { return primaryChannel.authority(); } + @Override + public io.grpc.ConnectivityState getState(boolean requestConnection) { + if (isInFallbackMode()) { + if (fallbackDelegateChannel != null) { + return fallbackDelegateChannel.getState(requestConnection); + } + return io.grpc.ConnectivityState.SHUTDOWN; + } + + if (primaryDelegateChannel != null) { + return primaryDelegateChannel.getState(requestConnection); + } + return io.grpc.ConnectivityState.SHUTDOWN; + } + + @Override + public void notifyWhenStateChanged(io.grpc.ConnectivityState source, Runnable callback) { + if (isInFallbackMode()) { + if (fallbackDelegateChannel != null) { + fallbackDelegateChannel.notifyWhenStateChanged(source, callback); + } + } else { + if (primaryDelegateChannel != null) { + primaryDelegateChannel.notifyWhenStateChanged(source, callback); + } + } + } + @Override public ManagedChannel shutdown() { + if (primaryProbeFuture != null) { + primaryProbeFuture.cancel(false); + } + if (fallbackProbeFuture != null) { + fallbackProbeFuture.cancel(false); + } if (primaryDelegateChannel != null) { primaryDelegateChannel.shutdown(); } if (fallbackDelegateChannel != null) { fallbackDelegateChannel.shutdown(); } - execService.shutdown(); + if (ownsFallbackState) { + fallbackState.shutdown(); + } return this; } @Override public ManagedChannel shutdownNow() { + if (primaryProbeFuture != null) { + primaryProbeFuture.cancel(true); + } + if (fallbackProbeFuture != null) { + fallbackProbeFuture.cancel(true); + } if (primaryDelegateChannel != null) { primaryDelegateChannel.shutdownNow(); } if (fallbackDelegateChannel != null) { fallbackDelegateChannel.shutdownNow(); } - execService.shutdownNow(); + if (ownsFallbackState) { + fallbackState.shutdownNow(); + } return this; } diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java index 31d5cd981907..a26c4d172be4 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java @@ -25,6 +25,7 @@ import java.time.Duration; import java.util.EnumSet; import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; public class GcpFallbackChannelOptions { @@ -37,9 +38,13 @@ public class GcpFallbackChannelOptions { private final Function fallbackProbingFunction; private final Duration primaryProbingInterval; private final Duration fallbackProbingInterval; + private final int minPrimaryProbeSuccessCount; + private final Duration minPrimaryProbeSuccessDuration; private final String primaryChannelName; private final String fallbackChannelName; private final GcpFallbackOpenTelemetry openTelemetry; + private final ScheduledExecutorService sharedExecutorService; + private final GcpFallbackState sharedState; public GcpFallbackChannelOptions(Builder builder) { this.enableFallback = builder.enableFallback; @@ -51,9 +56,13 @@ public GcpFallbackChannelOptions(Builder builder) { this.fallbackProbingFunction = builder.fallbackProbingFunction; this.primaryProbingInterval = builder.primaryProbingInterval; this.fallbackProbingInterval = builder.fallbackProbingInterval; + this.minPrimaryProbeSuccessCount = builder.minPrimaryProbeSuccessCount; + this.minPrimaryProbeSuccessDuration = builder.minPrimaryProbeSuccessDuration; this.primaryChannelName = builder.primaryChannelName; this.fallbackChannelName = builder.fallbackChannelName; this.openTelemetry = builder.openTelemetry; + this.sharedExecutorService = builder.sharedExecutorService; + this.sharedState = builder.sharedState; } public static Builder newBuilder() { @@ -96,6 +105,14 @@ public Duration getFallbackProbingInterval() { return fallbackProbingInterval; } + public int getMinPrimaryProbeSuccessCount() { + return minPrimaryProbeSuccessCount; + } + + public Duration getMinPrimaryProbeSuccessDuration() { + return minPrimaryProbeSuccessDuration; + } + public String getPrimaryChannelName() { return primaryChannelName; } @@ -108,6 +125,14 @@ public GcpFallbackOpenTelemetry getGcpOpenTelemetry() { return openTelemetry; } + public ScheduledExecutorService getSharedExecutorService() { + return sharedExecutorService; + } + + public GcpFallbackState getSharedState() { + return sharedState; + } + public static class Builder { private boolean enableFallback = true; private float errorRateThreshold = 1f; @@ -122,10 +147,15 @@ public static class Builder { private Duration primaryProbingInterval = Duration.ofMinutes(1); private Duration fallbackProbingInterval = Duration.ofMinutes(15); + private int minPrimaryProbeSuccessCount = 10; + private Duration minPrimaryProbeSuccessDuration = Duration.ZERO; + private String primaryChannelName = "primary"; private String fallbackChannelName = "fallback"; private GcpFallbackOpenTelemetry openTelemetry = null; + private ScheduledExecutorService sharedExecutorService = null; + private GcpFallbackState sharedState = null; public Builder() {} @@ -184,6 +214,16 @@ public Builder setFallbackProbingInterval(Duration fallbackProbingInterval) { return this; } + public Builder setMinPrimaryProbeSuccessCount(int minPrimaryProbeSuccessCount) { + this.minPrimaryProbeSuccessCount = minPrimaryProbeSuccessCount; + return this; + } + + public Builder setMinPrimaryProbeSuccessDuration(Duration minPrimaryProbeSuccessDuration) { + this.minPrimaryProbeSuccessDuration = minPrimaryProbeSuccessDuration; + return this; + } + public Builder setPrimaryChannelName(String primaryChannelName) { this.primaryChannelName = primaryChannelName; return this; @@ -199,6 +239,16 @@ public Builder setGcpFallbackOpenTelemetry(GcpFallbackOpenTelemetry openTelemetr return this; } + public Builder setSharedExecutorService(ScheduledExecutorService sharedExecutorService) { + this.sharedExecutorService = sharedExecutorService; + return this; + } + + public Builder setSharedState(GcpFallbackState sharedState) { + this.sharedState = sharedState; + return this; + } + public GcpFallbackChannelOptions build() { return new GcpFallbackChannelOptions(this); } diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java new file mode 100644 index 000000000000..a9f4a8834251 --- /dev/null +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -0,0 +1,209 @@ +/* + * Copyright 2026 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 + * + * https://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.grpc.fallback; + +import com.google.cloud.grpc.GcpThreadFactory; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; + +/** + * Shared thread-safe state container for coordinated pool-wide failover, recovery, and background + * tasks. + * + *

All channels in a pool share this state instance and its background executor service, + * consolidating probing and error evaluation threads across the entire channel pool. + */ +public class GcpFallbackState { + private final AtomicLong primarySuccesses = new AtomicLong(0); + private final AtomicLong primaryFailures = new AtomicLong(0); + private final AtomicLong fallbackSuccesses = new AtomicLong(0); + private final AtomicLong fallbackFailures = new AtomicLong(0); + private final AtomicBoolean inFallbackMode = new AtomicBoolean(false); + private final AtomicBoolean evaluationStarted = new AtomicBoolean(false); + + private ScheduledExecutorService execService = null; + private boolean ownsExecutor = false; + private ScheduledFuture scheduledEvaluationFuture = null; + + public AtomicLong getPrimarySuccesses() { + return primarySuccesses; + } + + public AtomicLong getPrimaryFailures() { + return primaryFailures; + } + + public AtomicLong getFallbackSuccesses() { + return fallbackSuccesses; + } + + public AtomicLong getFallbackFailures() { + return fallbackFailures; + } + + public AtomicBoolean getInFallbackMode() { + return inFallbackMode; + } + + /** + * Retrieves or lazily initializes the shared background executor service. + * + * @param externalExec optional external executor service to use (e.g. from test or options). + * @param options optional fallback channel configuration options. + * @return the active ScheduledExecutorService. + */ + public synchronized ScheduledExecutorService getOrCreateExecutorService( + ScheduledExecutorService externalExec, GcpFallbackChannelOptions options) { + if (this.execService != null) { + return this.execService; + } + if (externalExec != null) { + this.execService = externalExec; + this.ownsExecutor = + (options == null + || options.getSharedState() == null + || options.getSharedExecutorService() == null); + } else if (options != null && options.getSharedExecutorService() != null) { + this.execService = options.getSharedExecutorService(); + this.ownsExecutor = false; + } else { + this.execService = + Executors.newScheduledThreadPool( + 3, GcpThreadFactory.newThreadFactory("gcp-fallback-state-%d")); + this.ownsExecutor = true; + } + return this.execService; + } + + /** Schedules a periodic task (e.g., probe) on the shared background executor service. */ + public synchronized ScheduledFuture scheduleTask( + Runnable command, long initialDelay, long period, TimeUnit unit) { + if (this.execService == null || this.execService.isShutdown()) { + return null; + } + return this.execService.scheduleAtFixedRate(command, initialDelay, period, unit); + } + + /** + * Starts the periodic error rate evaluation loop exactly once across all channels sharing this + * state. + * + * @param options the fallback channel configuration options. + * @param externalExec optional executor service to use if not yet initialized. + */ + public void startPeriodicEvaluation( + GcpFallbackChannelOptions options, ScheduledExecutorService externalExec) { + if (options == null + || !options.isEnableFallback() + || options.getPeriod() == null + || options.getPeriod().toMillis() <= 0) { + return; + } + if (evaluationStarted.compareAndSet(false, true)) { + ScheduledExecutorService executor = getOrCreateExecutorService(externalExec, options); + GcpFallbackOpenTelemetry openTelemetry = + options.getGcpOpenTelemetry() != null + ? options.getGcpOpenTelemetry() + : GcpFallbackOpenTelemetry.newBuilder().build(); + + scheduledEvaluationFuture = + executor.scheduleAtFixedRate( + () -> checkErrorRates(options, openTelemetry), + options.getPeriod().toMillis(), + options.getPeriod().toMillis(), + TimeUnit.MILLISECONDS); + } + } + + /** + * Evaluates error rates across all channels sharing this state and updates fallback mode. + * + * @param options the fallback channel configuration options. + * @param openTelemetry telemetry module for recording error metrics. + */ + public void checkErrorRates( + GcpFallbackChannelOptions options, GcpFallbackOpenTelemetry openTelemetry) { + long successes = primarySuccesses.getAndSet(0); + long failures = primaryFailures.getAndSet(0); + float errRate = 0f; + if (failures + successes > 0) { + errRate = (float) failures / (failures + successes); + } + if (openTelemetry != null && openTelemetry.getModule() != null) { + openTelemetry.getModule().reportErrorRate(options.getPrimaryChannelName(), errRate); + } + + if (!inFallbackMode.get() && options.isEnableFallback()) { + if (failures >= options.getMinFailedCalls() && errRate >= options.getErrorRateThreshold()) { + inFallbackMode.set(true); // Coordinated instant pool-wide switch + if (openTelemetry != null && openTelemetry.getModule() != null) { + openTelemetry + .getModule() + .reportFallback(options.getPrimaryChannelName(), options.getFallbackChannelName()); + } + } + } + + successes = fallbackSuccesses.getAndSet(0); + failures = fallbackFailures.getAndSet(0); + errRate = 0f; + if (failures + successes > 0) { + errRate = (float) failures / (failures + successes); + } + if (openTelemetry != null && openTelemetry.getModule() != null) { + openTelemetry.getModule().reportErrorRate(options.getFallbackChannelName(), errRate); + openTelemetry + .getModule() + .reportCurrentChannel(options.getPrimaryChannelName(), !inFallbackMode.get()); + openTelemetry + .getModule() + .reportCurrentChannel(options.getFallbackChannelName(), inFallbackMode.get()); + } + } + + /** Stops any running scheduled evaluation. */ + public synchronized void stopPeriodicEvaluation() { + if (scheduledEvaluationFuture != null) { + scheduledEvaluationFuture.cancel(false); + scheduledEvaluationFuture = null; + } + evaluationStarted.set(false); + } + + /** Shuts down the state, cancelling evaluation and shutting down internal executor if owned. */ + public synchronized void shutdown() { + stopPeriodicEvaluation(); + if (ownsExecutor && execService != null && !execService.isShutdown()) { + execService.shutdown(); + } + } + + /** + * Shuts down the state immediately, cancelling evaluation and terminating internal executor if + * owned. + */ + public synchronized void shutdownNow() { + stopPeriodicEvaluation(); + if (ownsExecutor && execService != null && !execService.isShutdown()) { + execService.shutdownNow(); + } + } +} diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 210c482d588f..52f2f04b519b 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -41,6 +41,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -82,6 +83,7 @@ import java.util.List; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; import javax.annotation.Nonnull; @@ -1082,6 +1084,7 @@ public void testProbingTasksScheduled_ifConfigured() { .build(); initializeChannelAndCaptureTasks(options); + gcpFallbackChannel.getFallbackState().getInFallbackMode().set(true); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1118,6 +1121,7 @@ public void testProbing_reportsMetrics() throws InterruptedException { .build(); initializeChannelAndCaptureTasks(options); + gcpFallbackChannel.getFallbackState().getInFallbackMode().set(true); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1212,6 +1216,7 @@ public void testProbing_reportsInitFailureForFallback() throws InterruptedExcept .build(); initializeChannelWithInvalidFallbackBuilderAndCaptureTasks(options); + gcpFallbackChannel.getFallbackState().getInFallbackMode().set(true); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1286,4 +1291,381 @@ public void testConstructor_failsWhenBothBuildersFail() { new GcpFallbackChannel( getDefaultOptions(), mockPrimaryInvalidBuilder, mockFallbackInvalidBuilder)); } + + @SuppressWarnings({"unchecked"}) + private void simulateCallOnChannel( + GcpFallbackChannel channel, + Status statusToReturn, + ManagedChannel primaryDelegate, + ManagedChannel fallbackDelegate, + ClientCall primaryCall, + ClientCall fallbackCall, + boolean expectFallbackRouting) { + final ClientCall.Listener dummyCallListener = mock(ClientCall.Listener.class); + final Metadata requestHeaders = new Metadata(); + + ClientCall testCall = channel.newCall(methodDescriptor, callOptions); + assertNotNull(testCall); + + ClientCall targetCall; + if (expectFallbackRouting) { + verify(fallbackDelegate).newCall(methodDescriptor, callOptions); + verify(primaryDelegate, never()).newCall(methodDescriptor, callOptions); + targetCall = fallbackCall; + } else { + verify(primaryDelegate).newCall(methodDescriptor, callOptions); + verify(fallbackDelegate, never()).newCall(methodDescriptor, callOptions); + targetCall = primaryCall; + } + + testCall.start(dummyCallListener, requestHeaders); + + ArgumentCaptor> delegateListenerCaptor = + ArgumentCaptor.forClass(ClientCall.Listener.class); + verify(targetCall).start(delegateListenerCaptor.capture(), eq(requestHeaders)); + delegateListenerCaptor.getValue().onClose(statusToReturn, new Metadata()); + + clearInvocations(primaryDelegate, fallbackDelegate, targetCall); + } + + @Test + public void testSharedState_singleEvaluationScheduled() { + GcpFallbackState sharedState = new GcpFallbackState(); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder().setSharedState(sharedState).build(); + + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); + + try { + // Periodic evaluation was started on mockExec1 by the shared state + verify(mockExec1) + .scheduleAtFixedRate( + any(Runnable.class), + eq(options.getPeriod().toMillis()), + eq(options.getPeriod().toMillis()), + eq(MILLISECONDS)); + + // Channel 2 sharing the same state did NOT schedule a duplicate evaluation loop + verify(mockExec2, never()) + .scheduleAtFixedRate( + any(Runnable.class), + eq(options.getPeriod().toMillis()), + eq(options.getPeriod().toMillis()), + eq(MILLISECONDS)); + } finally { + channel1.shutdownNow(); + channel2.shutdownNow(); + } + } + + @Test + public void testSharedState_coordinatedFailover() { + GcpFallbackState sharedState = new GcpFallbackState(); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setMinFailedCalls(3) + .setErrorRateThreshold(0.5f) + .build(); + + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); + + try { + verify(mockExec1) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options.getPeriod().toMillis()), + eq(options.getPeriod().toMillis()), + eq(MILLISECONDS)); + Runnable checkErrorRates = taskCaptor.getValue(); + + // Both channels initially in primary mode + assertFalse(channel1.isInFallbackMode()); + assertFalse(channel2.isInFallbackMode()); + + // Channel 1 processes 2 failures, Channel 2 processes 1 failure (total 3 failures on shared + // state) + simulateCallOnChannel( + channel1, + Status.UNAVAILABLE, + mockPrimaryDelegateChannel, + mockFallbackDelegateChannel, + mockPrimaryClientCall, + mockFallbackClientCall, + false); + simulateCallOnChannel( + channel1, + Status.UNAVAILABLE, + mockPrimaryDelegateChannel, + mockFallbackDelegateChannel, + mockPrimaryClientCall, + mockFallbackClientCall, + false); + simulateCallOnChannel( + channel2, + Status.UNAVAILABLE, + mockPrimaryDelegateChannel, + mockFallbackDelegateChannel, + mockPrimaryClientCall, + mockFallbackClientCall, + false); + + // Run checkErrorRates on shared state + checkErrorRates.run(); + + // Both channels must now be in fallback mode + assertTrue(channel1.isInFallbackMode()); + assertTrue(channel2.isInFallbackMode()); + + // Subsequent call on Channel 2 routes to fallback channel + simulateCallOnChannel( + channel2, + Status.OK, + mockPrimaryDelegateChannel, + mockFallbackDelegateChannel, + mockPrimaryClientCall, + mockFallbackClientCall, + true); + } finally { + channel1.shutdownNow(); + channel2.shutdownNow(); + } + } + + @Test + public void testSharedState_channelShutdownLeavesSiblingChannelsFunctional() { + GcpFallbackState sharedState = new GcpFallbackState(); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder().setSharedState(sharedState).build(); + + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); + + try { + // Shutting down channel1 does not shut down the shared executor while sibling channels are + // active + channel1.shutdown(); + verify(mockExec1, never()).shutdown(); + + // Channel 2 can still transition and read shared fallback state + sharedState.getInFallbackMode().set(true); + assertTrue(channel2.isInFallbackMode()); + } finally { + channel2.shutdownNow(); + sharedState.shutdown(); + verify(mockExec1).shutdown(); + } + } + + @Test + public void testSharedState_probingRequiresBothCountAndDurationToRecover() + throws InterruptedException { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); + + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(2) + .setMinPrimaryProbeSuccessDuration(Duration.ofMillis(50)) + .build(); + + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + verify(mockExec) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask = taskCaptor.getValue(); + + // Probe 1: Success, but count < 2 and duration not yet met + probeTask.run(); + assertTrue(channel.isInFallbackMode()); + assertEquals(1, channel.getLocalProbeSuccesses().get()); + + // Probe 2 immediately: count == 2, but duration (50ms) not elapsed yet! + probeTask.run(); + assertTrue(channel.isInFallbackMode()); + assertEquals(2, channel.getLocalProbeSuccesses().get()); + + // Wait for duration window to pass + Thread.sleep(60); + + // Probe 3: count >= 2 and duration >= 50ms satisfied -> Recover! + probeTask.run(); + assertFalse(channel.isInFallbackMode()); + assertEquals(0, channel.getLocalProbeSuccesses().get()); + } finally { + channel.shutdownNow(); + sharedState.shutdown(); + } + } + + @Test + public void testSharedState_probingFailureResetsDurationTimer() { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); + + AtomicBoolean probeOk = new AtomicBoolean(true); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setPrimaryProbingFunction(channel -> probeOk.get() ? "OK" : "UNAVAILABLE") + .setMinPrimaryProbeSuccessCount(5) + .setMinPrimaryProbeSuccessDuration(Duration.ofMinutes(10)) + .build(); + + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + verify(mockExec) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask = taskCaptor.getValue(); + + // Successful probe initializes local probe counter + probeTask.run(); + assertEquals(1, channel.getLocalProbeSuccesses().get()); + + // Failing probe resets local probe count to 0 + probeOk.set(false); + probeTask.run(); + assertEquals(0, channel.getLocalProbeSuccesses().get()); + assertTrue(channel.isInFallbackMode()); + } finally { + channel.shutdownNow(); + sharedState.shutdown(); + } + } + + @Test + public void testProbePrimary_skippedWhenNotInFallbackMode() { + AtomicLong probeCalls = new AtomicLong(0); + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(false); + + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setPrimaryProbingFunction( + channel -> { + probeCalls.incrementAndGet(); + return "OK"; + }) + .build(); + + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + verify(mockExec) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask = taskCaptor.getValue(); + + // Channel is NOT in fallback mode -> probe task should immediately return without probing! + assertFalse(channel.isInFallbackMode()); + probeTask.run(); + assertEquals(0, probeCalls.get()); + assertEquals(0, channel.getLocalProbeSuccesses().get()); + } finally { + channel.shutdownNow(); + sharedState.shutdown(); + } + } + + @Test + public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysInFallback() { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); // Pool-wide fallback active + + GcpFallbackChannelOptions options1 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + GcpFallbackChannelOptions options2 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setPrimaryProbingFunction(channel -> "UNAVAILABLE") + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor1 = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options1, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options2, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); + + try { + verify(mockExec1, atLeastOnce()) + .scheduleAtFixedRate( + taskCaptor1.capture(), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask1 = taskCaptor1.getAllValues().get(0); + + assertTrue(channel1.isInFallbackMode()); + assertTrue(channel2.isInFallbackMode()); + + // Run probe on channel 1 -> channel 1 recovers to DirectPath + probeTask1.run(); + + assertFalse("Channel 1 should recover to DirectPath", channel1.isInFallbackMode()); + assertTrue("Channel 2 should remain in CloudPath fallback mode", channel2.isInFallbackMode()); + assertFalse("Global fallback should be unlatched", sharedState.getInFallbackMode().get()); + } finally { + channel1.shutdownNow(); + channel2.shutdownNow(); + sharedState.shutdown(); + } + } } From 5321aa2367a4e14ba62f8c7e78002033eb99032d Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 7 Aug 2026 20:35:08 +0000 Subject: [PATCH 02/15] feat(grpc-gcp): add enableRecovery and enablePerChannelRecovery options to GcpFallbackChannel --- .../grpc/fallback/GcpFallbackChannel.java | 15 ++- .../fallback/GcpFallbackChannelOptions.java | 24 ++++ .../cloud/grpc/fallback/GcpFallbackState.java | 2 +- .../grpc/fallback/GcpFallbackChannelTest.java | 111 ++++++++++++++++++ 4 files changed, 148 insertions(+), 4 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index d7c01ccce11a..3aecb917ac28 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -188,7 +188,11 @@ public boolean isInFallbackMode() { localFirstPrimaryProbeSuccessNanos.set(0); } } - return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; + if (options.isEnablePerChannelRecovery()) { + return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; + } + return (fallbackState.getInFallbackMode().get() && fallbackChannel != null) + || primaryChannel == null; } @VisibleForTesting @@ -257,7 +261,11 @@ private void probePrimary() { localFirstPrimaryProbeSuccessNanos.set(0); } } - if (!localInFallbackMode.get() && primaryChannel != null) { + boolean inFallback = + options.isEnablePerChannelRecovery() + ? localInFallbackMode.get() + : fallbackState.getInFallbackMode().get(); + if (!inFallback && primaryChannel != null) { return; } String result = ""; @@ -279,7 +287,8 @@ private void probePrimary() { durationSatisfied = elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); } - if (primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() + if (options.isEnableRecovery() + && primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() && durationSatisfied) { fallbackState.getInFallbackMode().set(false); localInFallbackMode.set(false); diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java index a26c4d172be4..0fcc21fd4e34 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java @@ -40,6 +40,8 @@ public class GcpFallbackChannelOptions { private final Duration fallbackProbingInterval; private final int minPrimaryProbeSuccessCount; private final Duration minPrimaryProbeSuccessDuration; + private final boolean enableRecovery; + private final boolean enablePerChannelRecovery; private final String primaryChannelName; private final String fallbackChannelName; private final GcpFallbackOpenTelemetry openTelemetry; @@ -58,6 +60,8 @@ public GcpFallbackChannelOptions(Builder builder) { this.fallbackProbingInterval = builder.fallbackProbingInterval; this.minPrimaryProbeSuccessCount = builder.minPrimaryProbeSuccessCount; this.minPrimaryProbeSuccessDuration = builder.minPrimaryProbeSuccessDuration; + this.enableRecovery = builder.enableRecovery; + this.enablePerChannelRecovery = builder.enablePerChannelRecovery; this.primaryChannelName = builder.primaryChannelName; this.fallbackChannelName = builder.fallbackChannelName; this.openTelemetry = builder.openTelemetry; @@ -113,6 +117,14 @@ public Duration getMinPrimaryProbeSuccessDuration() { return minPrimaryProbeSuccessDuration; } + public boolean isEnableRecovery() { + return enableRecovery; + } + + public boolean isEnablePerChannelRecovery() { + return enablePerChannelRecovery; + } + public String getPrimaryChannelName() { return primaryChannelName; } @@ -149,6 +161,8 @@ public static class Builder { private int minPrimaryProbeSuccessCount = 10; private Duration minPrimaryProbeSuccessDuration = Duration.ZERO; + private boolean enableRecovery = false; + private boolean enablePerChannelRecovery = false; private String primaryChannelName = "primary"; private String fallbackChannelName = "fallback"; @@ -224,6 +238,16 @@ public Builder setMinPrimaryProbeSuccessDuration(Duration minPrimaryProbeSuccess return this; } + public Builder setEnableRecovery(boolean enableRecovery) { + this.enableRecovery = enableRecovery; + return this; + } + + public Builder setEnablePerChannelRecovery(boolean enablePerChannelRecovery) { + this.enablePerChannelRecovery = enablePerChannelRecovery; + return this; + } + public Builder setPrimaryChannelName(String primaryChannelName) { this.primaryChannelName = primaryChannelName; return this; diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index a9f4a8834251..fbd9a60d4f22 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -153,7 +153,7 @@ public void checkErrorRates( if (!inFallbackMode.get() && options.isEnableFallback()) { if (failures >= options.getMinFailedCalls() && errRate >= options.getErrorRateThreshold()) { - inFallbackMode.set(true); // Coordinated instant pool-wide switch + inFallbackMode.set(true); if (openTelemetry != null && openTelemetry.getModule() != null) { openTelemetry .getModule() diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 52f2f04b519b..5b4e753787db 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -1485,6 +1485,7 @@ public void testSharedState_probingRequiresBothCountAndDurationToRecover() GcpFallbackChannelOptions options = getDefaultOptionsBuilder() .setSharedState(sharedState) + .setEnableRecovery(true) .setPrimaryProbingFunction(channel -> "OK") .setMinPrimaryProbeSuccessCount(2) .setMinPrimaryProbeSuccessDuration(Duration.ofMillis(50)) @@ -1537,6 +1538,7 @@ public void testSharedState_probingFailureResetsDurationTimer() { GcpFallbackChannelOptions options = getDefaultOptionsBuilder() .setSharedState(sharedState) + .setEnableRecovery(true) .setPrimaryProbingFunction(channel -> probeOk.get() ? "OK" : "UNAVAILABLE") .setMinPrimaryProbeSuccessCount(5) .setMinPrimaryProbeSuccessDuration(Duration.ofMinutes(10)) @@ -1622,6 +1624,8 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI GcpFallbackChannelOptions options1 = getDefaultOptionsBuilder() .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(true) .setPrimaryProbingFunction(channel -> "OK") .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) @@ -1630,6 +1634,8 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI GcpFallbackChannelOptions options2 = getDefaultOptionsBuilder() .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(true) .setPrimaryProbingFunction(channel -> "UNAVAILABLE") .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) @@ -1668,4 +1674,109 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI sharedState.shutdown(); } } + + @Test + public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsRecoverTogether() { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); // Pool-wide fallback active + + GcpFallbackChannelOptions options1 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(false) // Pool-level recovery + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + GcpFallbackChannelOptions options2 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(false) // Pool-level recovery + .setPrimaryProbingFunction(channel -> "UNAVAILABLE") + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor1 = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options1, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options2, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); + + try { + verify(mockExec1, atLeastOnce()) + .scheduleAtFixedRate( + taskCaptor1.capture(), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask1 = taskCaptor1.getAllValues().get(0); + + assertTrue(channel1.isInFallbackMode()); + assertTrue(channel2.isInFallbackMode()); + + // Run probe on channel 1 -> channel 1 recovers to DirectPath and unlatches global fallback + probeTask1.run(); + + // With enablePerChannelRecovery=false, both channel 1 and channel 2 recover to DirectPath + // together + assertFalse("Channel 1 should recover to DirectPath", channel1.isInFallbackMode()); + assertFalse( + "Channel 2 should also recover to DirectPath with pool", channel2.isInFallbackMode()); + assertFalse("Global fallback should be unlatched", sharedState.getInFallbackMode().get()); + } finally { + channel1.shutdownNow(); + channel2.shutdownNow(); + sharedState.shutdown(); + } + } + + @Test + public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); // Pool-wide fallback active + + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(false) // Recovery disabled by default + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + verify(mockExec) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask = taskCaptor.getValue(); + + assertTrue(channel.isInFallbackMode()); + + // Run probe -> probe succeeds, but enableRecovery is false + probeTask.run(); + + // Channel must remain in fallback mode + assertTrue(channel.isInFallbackMode()); + assertTrue(sharedState.getInFallbackMode().get()); + } finally { + channel.shutdownNow(); + sharedState.shutdown(); + } + } } From 945a8563cf2706cbe64ae466b5ed2db9c83b20e5 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 7 Aug 2026 21:22:37 +0000 Subject: [PATCH 03/15] fixes --- .../grpc/fallback/GcpFallbackChannel.java | 21 +++-- .../cloud/grpc/fallback/GcpFallbackState.java | 2 +- .../grpc/fallback/GcpFallbackChannelTest.java | 77 +++++++++++++++++++ 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 3aecb917ac28..671562b5aad4 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -59,8 +59,8 @@ public class GcpFallbackChannel extends ManagedChannel { private final AtomicLong localFirstPrimaryProbeSuccessNanos = new AtomicLong(0); private final ScheduledExecutorService execService; - private ScheduledFuture primaryProbeFuture = null; - private ScheduledFuture fallbackProbeFuture = null; + private volatile ScheduledFuture primaryProbeFuture = null; + private volatile ScheduledFuture fallbackProbeFuture = null; public GcpFallbackChannel( GcpFallbackChannelOptions options, @@ -187,6 +187,11 @@ public boolean isInFallbackMode() { localProbeSuccesses.set(0); localFirstPrimaryProbeSuccessNanos.set(0); } + } else if (!options.isEnablePerChannelRecovery()) { + if (localInFallbackMode.compareAndSet(true, false)) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } } if (options.isEnablePerChannelRecovery()) { return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; @@ -260,6 +265,11 @@ private void probePrimary() { localProbeSuccesses.set(0); localFirstPrimaryProbeSuccessNanos.set(0); } + } else if (!options.isEnablePerChannelRecovery()) { + if (localInFallbackMode.compareAndSet(true, false)) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } } boolean inFallback = options.isEnablePerChannelRecovery() @@ -275,15 +285,16 @@ private void probePrimary() { result = options.getPrimaryProbingFunction().apply(primaryDelegateChannel); } if ("OK".equals(result)) { - localFirstPrimaryProbeSuccessNanos.compareAndSet(0, System.nanoTime()); - long firstSuccessNanos = localFirstPrimaryProbeSuccessNanos.get(); + long nowNanos = System.nanoTime(); + long firstSuccessNanos = + localFirstPrimaryProbeSuccessNanos.updateAndGet(prev -> prev == 0 ? nowNanos : prev); long primaryProbeSuccessCount = localProbeSuccesses.incrementAndGet(); boolean durationSatisfied = true; if (options.getMinPrimaryProbeSuccessDuration() != null && !options.getMinPrimaryProbeSuccessDuration().isZero() && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { - long elapsedNanos = System.nanoTime() - firstSuccessNanos; + long elapsedNanos = nowNanos - firstSuccessNanos; durationSatisfied = elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); } diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index fbd9a60d4f22..266d0176947d 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -41,7 +41,7 @@ public class GcpFallbackState { private ScheduledExecutorService execService = null; private boolean ownsExecutor = false; - private ScheduledFuture scheduledEvaluationFuture = null; + private volatile ScheduledFuture scheduledEvaluationFuture = null; public AtomicLong getPrimarySuccesses() { return primarySuccesses; diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 5b4e753787db..898220a26887 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -41,6 +41,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; @@ -1779,4 +1780,80 @@ public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { sharedState.shutdown(); } } + + @Test + public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); // Cycle 1: Fallback active + + GcpFallbackChannelOptions options1 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(false) + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(2) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + GcpFallbackChannelOptions options2 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(false) + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(2) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options1, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options2, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + verify(mockExec, atLeast(2)) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask1 = taskCaptor.getAllValues().get(0); + Runnable probeTask2 = taskCaptor.getAllValues().get(1); + + // Both channels enter fallback + assertTrue(channel1.isInFallbackMode()); + assertTrue(channel2.isInFallbackMode()); + + // Channel 1 probes twice -> recovers pool + probeTask1.run(); + probeTask1.run(); + assertFalse(channel1.isInFallbackMode()); + assertFalse(channel2.isInFallbackMode()); + + // Now Cycle 2: Incident occurs again, pool enters fallback + sharedState.getInFallbackMode().set(true); + assertTrue(channel2.isInFallbackMode()); + + // Channel 2's localProbeSuccesses must be reset to 0 in new cycle + assertEquals(0, channel2.getLocalProbeSuccesses().get()); + + // Probe once: count is 1 (< 2 required), must still be in fallback + probeTask2.run(); + assertEquals(1, channel2.getLocalProbeSuccesses().get()); + assertTrue(channel2.isInFallbackMode()); + + // Probe second time: count is 2 (>= 2 required) -> recovers! + probeTask2.run(); + assertFalse(channel2.isInFallbackMode()); + assertFalse(sharedState.getInFallbackMode().get()); + } finally { + channel1.shutdownNow(); + channel2.shutdownNow(); + sharedState.shutdown(); + } + } } From 4f0d6ac75bd6d540eaa4b2c951e19829023193db Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Thu, 3 Sep 2026 22:03:12 +0000 Subject: [PATCH 04/15] fixes --- .../grpc/fallback/GcpFallbackChannel.java | 110 +++++++++++------- .../cloud/grpc/fallback/GcpFallbackState.java | 5 +- .../grpc/fallback/GcpFallbackChannelTest.java | 98 ++++++++++++++++ 3 files changed, 169 insertions(+), 44 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 671562b5aad4..ca892d123a4a 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -57,6 +57,8 @@ public class GcpFallbackChannel extends ManagedChannel { private final AtomicBoolean localInFallbackMode = new AtomicBoolean(false); private final AtomicLong localProbeSuccesses = new AtomicLong(0); private final AtomicLong localFirstPrimaryProbeSuccessNanos = new AtomicLong(0); + private final java.util.concurrent.locks.ReentrantLock stateLock = + new java.util.concurrent.locks.ReentrantLock(); private final ScheduledExecutorService execService; private volatile ScheduledFuture primaryProbeFuture = null; @@ -181,23 +183,41 @@ public GcpFallbackChannel( init(); } - public boolean isInFallbackMode() { - if (fallbackState.getInFallbackMode().get()) { - if (localInFallbackMode.compareAndSet(false, true)) { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); + private void syncFallbackModeState(boolean globalFallback) { + if (globalFallback) { + if (!localInFallbackMode.get()) { + stateLock.lock(); + try { + if (localInFallbackMode.compareAndSet(false, true)) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } + } finally { + stateLock.unlock(); + } } } else if (!options.isEnablePerChannelRecovery()) { - if (localInFallbackMode.compareAndSet(true, false)) { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); + if (localInFallbackMode.get()) { + stateLock.lock(); + try { + if (localInFallbackMode.compareAndSet(true, false)) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } + } finally { + stateLock.unlock(); + } } } + } + + public boolean isInFallbackMode() { + boolean globalFallback = fallbackState.getInFallbackMode().get(); + syncFallbackModeState(globalFallback); if (options.isEnablePerChannelRecovery()) { return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; } - return (fallbackState.getInFallbackMode().get() && fallbackChannel != null) - || primaryChannel == null; + return (globalFallback && fallbackChannel != null) || primaryChannel == null; } @VisibleForTesting @@ -260,21 +280,12 @@ private void processFallbackStatusCode(Status.Code statusCode) { } private void probePrimary() { - if (fallbackState.getInFallbackMode().get()) { - if (localInFallbackMode.compareAndSet(false, true)) { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - } - } else if (!options.isEnablePerChannelRecovery()) { - if (localInFallbackMode.compareAndSet(true, false)) { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - } - } + boolean globalFallback = fallbackState.getInFallbackMode().get(); + syncFallbackModeState(globalFallback); boolean inFallback = options.isEnablePerChannelRecovery() ? localInFallbackMode.get() - : fallbackState.getInFallbackMode().get(); + : globalFallback; if (!inFallback && primaryChannel != null) { return; } @@ -285,31 +296,44 @@ private void probePrimary() { result = options.getPrimaryProbingFunction().apply(primaryDelegateChannel); } if ("OK".equals(result)) { - long nowNanos = System.nanoTime(); - long firstSuccessNanos = - localFirstPrimaryProbeSuccessNanos.updateAndGet(prev -> prev == 0 ? nowNanos : prev); - long primaryProbeSuccessCount = localProbeSuccesses.incrementAndGet(); - - boolean durationSatisfied = true; - if (options.getMinPrimaryProbeSuccessDuration() != null - && !options.getMinPrimaryProbeSuccessDuration().isZero() - && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { - long elapsedNanos = nowNanos - firstSuccessNanos; - durationSatisfied = elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); + stateLock.lock(); + try { + if (localInFallbackMode.get()) { + long nowNanos = System.nanoTime(); + long firstSuccessNanos = + localFirstPrimaryProbeSuccessNanos.updateAndGet(prev -> prev == 0 ? nowNanos : prev); + long primaryProbeSuccessCount = localProbeSuccesses.incrementAndGet(); + + boolean durationSatisfied = true; + if (options.getMinPrimaryProbeSuccessDuration() != null + && !options.getMinPrimaryProbeSuccessDuration().isZero() + && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { + long elapsedNanos = nowNanos - firstSuccessNanos; + durationSatisfied = + elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); + } + + if (options.isEnableRecovery() + && primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() + && durationSatisfied) { + fallbackState.getInFallbackMode().set(false); + localInFallbackMode.set(false); + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + } + } + } finally { + stateLock.unlock(); } - - if (options.isEnableRecovery() - && primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() - && durationSatisfied) { - fallbackState.getInFallbackMode().set(false); - localInFallbackMode.set(false); + } else { + stateLock.lock(); + try { + localInFallbackMode.set(true); localProbeSuccesses.set(0); localFirstPrimaryProbeSuccessNanos.set(0); + } finally { + stateLock.unlock(); } - } else { - localInFallbackMode.set(true); - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); } // Report metric based on result. openTelemetry.getModule().reportProbeResult(options.getPrimaryChannelName(), result); diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index 266d0176947d..5591c2c8ad89 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -109,7 +109,7 @@ public synchronized ScheduledFuture scheduleTask( * @param options the fallback channel configuration options. * @param externalExec optional executor service to use if not yet initialized. */ - public void startPeriodicEvaluation( + public synchronized void startPeriodicEvaluation( GcpFallbackChannelOptions options, ScheduledExecutorService externalExec) { if (options == null || !options.isEnableFallback() @@ -119,6 +119,9 @@ public void startPeriodicEvaluation( } if (evaluationStarted.compareAndSet(false, true)) { ScheduledExecutorService executor = getOrCreateExecutorService(externalExec, options); + if (executor == null || executor.isShutdown()) { + return; + } GcpFallbackOpenTelemetry openTelemetry = options.getGcpOpenTelemetry() != null ? options.getGcpOpenTelemetry() diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index d06a608396f3..5ff8eae06664 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -1855,4 +1855,102 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { sharedState.shutdown(); } } + + @Test + public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() + throws InterruptedException, java.util.concurrent.ExecutionException { + GcpFallbackState sharedState = new GcpFallbackState(); + sharedState.getInFallbackMode().set(true); + + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(true) + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(1000) + .setMinPrimaryProbeSuccessDuration(Duration.ofHours(1)) + .build(); + + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + verify(mockExec) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(options.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask = taskCaptor.getValue(); + + assertTrue(channel.isInFallbackMode()); + probeTask.run(); + assertEquals(1, channel.getLocalProbeSuccesses().get()); + + int threadCount = 50; + int iterationsPerThread = 200; + java.util.concurrent.ExecutorService threadPool = + java.util.concurrent.Executors.newFixedThreadPool(threadCount); + java.util.concurrent.CountDownLatch startLatch = + new java.util.concurrent.CountDownLatch(1); + java.util.List> futures = new java.util.ArrayList<>(); + + for (int i = 0; i < threadCount; i++) { + futures.add( + threadPool.submit( + () -> { + try { + startLatch.await(); + for (int j = 0; j < iterationsPerThread; j++) { + assertTrue(channel.isInFallbackMode()); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + })); + } + + startLatch.countDown(); + // Run probe while concurrent threads call isInFallbackMode + for (int i = 0; i < 9; i++) { + probeTask.run(); + } + + for (java.util.concurrent.Future future : futures) { + future.get(); + } + threadPool.shutdown(); + + // Ensure that concurrent callers did not reset probe count back to 0 + assertEquals(10, channel.getLocalProbeSuccesses().get()); + assertTrue(channel.isInFallbackMode()); + } finally { + channel.shutdownNow(); + sharedState.shutdown(); + } + } + + @Test + public void testShutdown_whenSuppliedSharedExecutorService_leavesExecutorRunning() { + ScheduledExecutorService sharedExec = mock(ScheduledExecutorService.class); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder() + .setSharedExecutorService(sharedExec) + .build(); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder); + + try { + channel.shutdown(); + verify(sharedExec, never()).shutdown(); + } finally { + channel.shutdownNow(); + verify(sharedExec, never()).shutdownNow(); + } + } } From dfc89caf0928ca4485dbf6d8c72cd51f7ed9c142 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 4 Sep 2026 02:03:10 +0000 Subject: [PATCH 05/15] fix test executor --- .../grpc/fallback/GcpFallbackChannel.java | 16 +++-- .../cloud/grpc/fallback/GcpFallbackState.java | 33 ++++++---- .../grpc/fallback/GcpFallbackChannelTest.java | 66 ++++++++----------- 3 files changed, 58 insertions(+), 57 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index ca892d123a4a..c9e5c795e212 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -92,10 +92,13 @@ public GcpFallbackChannel( this.fallbackState = options.getSharedState(); this.ownsFallbackState = false; } else { - this.fallbackState = new GcpFallbackState(); // Private state for backward compatibility + this.fallbackState = + execService != null + ? new GcpFallbackState(execService) + : new GcpFallbackState(); this.ownsFallbackState = true; } - this.execService = fallbackState.getOrCreateExecutorService(execService, options); + this.execService = fallbackState.getOrCreateExecutorService(options); if (options.getGcpOpenTelemetry() != null) { this.openTelemetry = options.getGcpOpenTelemetry(); } else { @@ -161,10 +164,13 @@ public GcpFallbackChannel( this.fallbackState = options.getSharedState(); this.ownsFallbackState = false; } else { - this.fallbackState = new GcpFallbackState(); // Private state for backward compatibility + this.fallbackState = + execService != null + ? new GcpFallbackState(execService) + : new GcpFallbackState(); this.ownsFallbackState = true; } - this.execService = fallbackState.getOrCreateExecutorService(execService, options); + this.execService = fallbackState.getOrCreateExecutorService(options); if (options.getGcpOpenTelemetry() != null) { this.openTelemetry = options.getGcpOpenTelemetry(); } else { @@ -254,7 +260,7 @@ private void init() { MILLISECONDS); } - fallbackState.startPeriodicEvaluation(options, execService); + fallbackState.startPeriodicEvaluation(options); } private void checkErrorRates() { diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index 5591c2c8ad89..0745de33aa61 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -17,6 +17,7 @@ package com.google.cloud.grpc.fallback; import com.google.cloud.grpc.GcpThreadFactory; +import com.google.common.annotations.VisibleForTesting; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -43,6 +44,19 @@ public class GcpFallbackState { private boolean ownsExecutor = false; private volatile ScheduledFuture scheduledEvaluationFuture = null; + public GcpFallbackState() {} + + /** + * Constructs a fallback state with an explicit executor service for testing. + * + * @param execService the executor service to use. + */ + @VisibleForTesting + public GcpFallbackState(ScheduledExecutorService execService) { + this.execService = execService; + this.ownsExecutor = true; + } + public AtomicLong getPrimarySuccesses() { return primarySuccesses; } @@ -64,24 +78,17 @@ public AtomicBoolean getInFallbackMode() { } /** - * Retrieves or lazily initializes the shared background executor service. + * Retrieves or lazily initializes the background executor service. * - * @param externalExec optional external executor service to use (e.g. from test or options). * @param options optional fallback channel configuration options. * @return the active ScheduledExecutorService. */ public synchronized ScheduledExecutorService getOrCreateExecutorService( - ScheduledExecutorService externalExec, GcpFallbackChannelOptions options) { + GcpFallbackChannelOptions options) { if (this.execService != null) { return this.execService; } - if (externalExec != null) { - this.execService = externalExec; - this.ownsExecutor = - (options == null - || options.getSharedState() == null - || options.getSharedExecutorService() == null); - } else if (options != null && options.getSharedExecutorService() != null) { + if (options != null && options.getSharedExecutorService() != null) { this.execService = options.getSharedExecutorService(); this.ownsExecutor = false; } else { @@ -107,10 +114,8 @@ public synchronized ScheduledFuture scheduleTask( * state. * * @param options the fallback channel configuration options. - * @param externalExec optional executor service to use if not yet initialized. */ - public synchronized void startPeriodicEvaluation( - GcpFallbackChannelOptions options, ScheduledExecutorService externalExec) { + public synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { if (options == null || !options.isEnableFallback() || options.getPeriod() == null @@ -118,7 +123,7 @@ public synchronized void startPeriodicEvaluation( return; } if (evaluationStarted.compareAndSet(false, true)) { - ScheduledExecutorService executor = getOrCreateExecutorService(externalExec, options); + ScheduledExecutorService executor = getOrCreateExecutorService(options); if (executor == null || executor.isShutdown()) { return; } diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 5ff8eae06664..6ba7a404c7a3 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -1330,12 +1330,11 @@ private void simulateCallOnChannel( @Test public void testSharedState_singleEvaluationScheduled() { - GcpFallbackState sharedState = new GcpFallbackState(); - GcpFallbackChannelOptions options = - getDefaultOptionsBuilder().setSharedState(sharedState).build(); - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec1); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder().setSharedState(sharedState).build(); GcpFallbackChannel channel1 = new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); @@ -1366,7 +1365,9 @@ public void testSharedState_singleEvaluationScheduled() { @Test public void testSharedState_coordinatedFailover() { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec1); GcpFallbackChannelOptions options = getDefaultOptionsBuilder() .setSharedState(sharedState) @@ -1374,9 +1375,6 @@ public void testSharedState_coordinatedFailover() { .setErrorRateThreshold(0.5f) .build(); - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); - ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); - ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel1 = @@ -1448,12 +1446,11 @@ public void testSharedState_coordinatedFailover() { @Test public void testSharedState_channelShutdownLeavesSiblingChannelsFunctional() { - GcpFallbackState sharedState = new GcpFallbackState(); - GcpFallbackChannelOptions options = - getDefaultOptionsBuilder().setSharedState(sharedState).build(); - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec1); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder().setSharedState(sharedState).build(); GcpFallbackChannel channel1 = new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); @@ -1479,7 +1476,8 @@ public void testSharedState_channelShutdownLeavesSiblingChannelsFunctional() { @Test public void testSharedState_probingRequiresBothCountAndDurationToRecover() throws InterruptedException { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); sharedState.getInFallbackMode().set(true); GcpFallbackChannelOptions options = @@ -1490,8 +1488,6 @@ public void testSharedState_probingRequiresBothCountAndDurationToRecover() .setMinPrimaryProbeSuccessCount(2) .setMinPrimaryProbeSuccessDuration(Duration.ofMillis(50)) .build(); - - ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel = @@ -1531,7 +1527,8 @@ public void testSharedState_probingRequiresBothCountAndDurationToRecover() @Test public void testSharedState_probingFailureResetsDurationTimer() { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); sharedState.getInFallbackMode().set(true); AtomicBoolean probeOk = new AtomicBoolean(true); @@ -1543,8 +1540,6 @@ public void testSharedState_probingFailureResetsDurationTimer() { .setMinPrimaryProbeSuccessCount(5) .setMinPrimaryProbeSuccessDuration(Duration.ofMinutes(10)) .build(); - - ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel = @@ -1576,8 +1571,9 @@ public void testSharedState_probingFailureResetsDurationTimer() { @Test public void testProbePrimary_skippedWhenNotInFallbackMode() { + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); AtomicLong probeCalls = new AtomicLong(0); - GcpFallbackState sharedState = new GcpFallbackState(); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); sharedState.getInFallbackMode().set(false); GcpFallbackChannelOptions options = @@ -1589,8 +1585,6 @@ public void testProbePrimary_skippedWhenNotInFallbackMode() { return "OK"; }) .build(); - - ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel = @@ -1618,7 +1612,9 @@ public void testProbePrimary_skippedWhenNotInFallbackMode() { @Test public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysInFallback() { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec1); sharedState.getInFallbackMode().set(true); // Pool-wide fallback active GcpFallbackChannelOptions options1 = @@ -1640,9 +1636,6 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); - - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); - ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor1 = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel1 = @@ -1677,7 +1670,9 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI @Test public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsRecoverTogether() { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec1); sharedState.getInFallbackMode().set(true); // Pool-wide fallback active GcpFallbackChannelOptions options1 = @@ -1699,9 +1694,6 @@ public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsReco .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); - - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); - ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor1 = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel1 = @@ -1739,7 +1731,8 @@ public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsReco @Test public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); sharedState.getInFallbackMode().set(true); // Pool-wide fallback active GcpFallbackChannelOptions options = @@ -1750,8 +1743,6 @@ public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); - - ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel = @@ -1782,7 +1773,8 @@ public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { @Test public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); sharedState.getInFallbackMode().set(true); // Cycle 1: Fallback active GcpFallbackChannelOptions options1 = @@ -1804,8 +1796,6 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { .setMinPrimaryProbeSuccessCount(2) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); - - ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel1 = @@ -1856,10 +1846,11 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { } } - @Test + @Test public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() throws InterruptedException, java.util.concurrent.ExecutionException { - GcpFallbackState sharedState = new GcpFallbackState(); + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); sharedState.getInFallbackMode().set(true); GcpFallbackChannelOptions options = @@ -1872,7 +1863,6 @@ public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() .setMinPrimaryProbeSuccessDuration(Duration.ofHours(1)) .build(); - ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); GcpFallbackChannel channel = From 132ee86b468369a23a2c0a6fd3ddd0b0428ad697 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 4 Sep 2026 02:08:13 +0000 Subject: [PATCH 06/15] fix --- .../grpc/fallback/GcpFallbackChannel.java | 15 +++++++-- .../grpc/fallback/GcpFallbackChannelTest.java | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index c9e5c795e212..869f8db48f47 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -453,7 +453,10 @@ public boolean isShutdown() { return false; } - return execService.isShutdown(); + if (ownsFallbackState && options.getSharedExecutorService() == null) { + return execService.isShutdown(); + } + return true; } @Override @@ -466,7 +469,10 @@ public boolean isTerminated() { return false; } - return execService.isTerminated(); + if (ownsFallbackState && options.getSharedExecutorService() == null) { + return execService.isTerminated(); + } + return true; } @Override @@ -488,6 +494,9 @@ public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedE awaitTimeNanos = endTimeNanos - System.nanoTime(); } - return execService.awaitTermination(awaitTimeNanos, NANOSECONDS); + if (ownsFallbackState && options.getSharedExecutorService() == null) { + return execService.awaitTermination(awaitTimeNanos, NANOSECONDS); + } + return true; } } diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 6ba7a404c7a3..2b1fe6e3fa10 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -1943,4 +1943,37 @@ public void testShutdown_whenSuppliedSharedExecutorService_leavesExecutorRunning verify(sharedExec, never()).shutdownNow(); } } + + @Test + public void testSharedState_channelLifecycleIndependentOfSharedExecutor() + throws InterruptedException { + ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec); + GcpFallbackChannelOptions options = + getDefaultOptionsBuilder().setSharedState(sharedState).build(); + + when(mockPrimaryDelegateChannel.awaitTermination(anyLong(), any(TimeUnit.class))) + .thenReturn(true); + when(mockFallbackDelegateChannel.awaitTermination(anyLong(), any(TimeUnit.class))) + .thenReturn(true); + when(mockPrimaryDelegateChannel.isShutdown()).thenReturn(true); + when(mockFallbackDelegateChannel.isShutdown()).thenReturn(true); + when(mockPrimaryDelegateChannel.isTerminated()).thenReturn(true); + when(mockFallbackDelegateChannel.isTerminated()).thenReturn(true); + + GcpFallbackChannel channel = + new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder, mockExec); + + try { + channel.shutdown(); + assertTrue(channel.isShutdown()); + assertTrue(channel.isTerminated()); + assertTrue(channel.awaitTermination(1, TimeUnit.SECONDS)); + // Sibling channels / shared state keep executor alive + verify(mockExec, never()).shutdown(); + verify(mockExec, never()).awaitTermination(anyLong(), any(TimeUnit.class)); + } finally { + sharedState.shutdown(); + } + } } From f5ada917d3dd0fda493df283f51dd4ce974a35ed Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 4 Sep 2026 02:25:41 +0000 Subject: [PATCH 07/15] formatting --- .../com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 2b1fe6e3fa10..e27bf41d502b 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -1846,7 +1846,7 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { } } - @Test + @Test public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() throws InterruptedException, java.util.concurrent.ExecutionException { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); From 075dc59ef3d2aab7d8a1e27e8b35dd01fd714d37 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 4 Sep 2026 17:01:41 +0000 Subject: [PATCH 08/15] remove old code --- .../com/google/cloud/grpc/fallback/GcpFallbackChannel.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 869f8db48f47..4d9c7cb40058 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -263,10 +263,6 @@ private void init() { fallbackState.startPeriodicEvaluation(options); } - private void checkErrorRates() { - fallbackState.checkErrorRates(options, openTelemetry); - } - private void processPrimaryStatusCode(Status.Code statusCode) { if (options.getErroneousStates().contains(statusCode)) { fallbackState.getPrimaryFailures().incrementAndGet(); From 2416c26e3a3867c0ab53af593eca85f525d94572 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Fri, 4 Sep 2026 20:17:40 +0000 Subject: [PATCH 09/15] fix evaluationStarted --- .../java/com/google/cloud/grpc/fallback/GcpFallbackState.java | 1 + 1 file changed, 1 insertion(+) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index 0745de33aa61..b6e8fd5099fa 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -125,6 +125,7 @@ public synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions optio if (evaluationStarted.compareAndSet(false, true)) { ScheduledExecutorService executor = getOrCreateExecutorService(options); if (executor == null || executor.isShutdown()) { + evaluationStarted.set(false); return; } GcpFallbackOpenTelemetry openTelemetry = From e3e21aed0a769af64f61e7533a1861adcf30cb10 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Tue, 15 Sep 2026 22:31:13 +0000 Subject: [PATCH 10/15] add fallback generation --- .../grpc/fallback/GcpFallbackChannel.java | 110 +++++++------- .../cloud/grpc/fallback/GcpFallbackState.java | 59 ++++++-- .../grpc/fallback/GcpFallbackChannelTest.java | 141 +++++++++++++++--- 3 files changed, 220 insertions(+), 90 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 4d9c7cb40058..8f4c55675424 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -55,10 +56,10 @@ public class GcpFallbackChannel extends ManagedChannel { private final GcpFallbackOpenTelemetry openTelemetry; private final AtomicBoolean localInFallbackMode = new AtomicBoolean(false); + private final AtomicLong localGeneration = new AtomicLong(0); private final AtomicLong localProbeSuccesses = new AtomicLong(0); private final AtomicLong localFirstPrimaryProbeSuccessNanos = new AtomicLong(0); - private final java.util.concurrent.locks.ReentrantLock stateLock = - new java.util.concurrent.locks.ReentrantLock(); + private final ReentrantLock stateLock = new ReentrantLock(); private final ScheduledExecutorService execService; private volatile ScheduledFuture primaryProbeFuture = null; @@ -189,41 +190,26 @@ public GcpFallbackChannel( init(); } - private void syncFallbackModeState(boolean globalFallback) { - if (globalFallback) { - if (!localInFallbackMode.get()) { - stateLock.lock(); - try { - if (localInFallbackMode.compareAndSet(false, true)) { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - } - } finally { - stateLock.unlock(); - } - } - } else if (!options.isEnablePerChannelRecovery()) { - if (localInFallbackMode.get()) { - stateLock.lock(); - try { - if (localInFallbackMode.compareAndSet(true, false)) { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - } - } finally { - stateLock.unlock(); + private void syncFallbackModeState() { + if (localGeneration.get() < fallbackState.getGeneration()) { + stateLock.lock(); + try { + long poolGen = fallbackState.getGeneration(); + if (localGeneration.get() < poolGen) { + localInFallbackMode.set(fallbackState.getTargetFallbackMode()); + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + localGeneration.set(poolGen); } + } finally { + stateLock.unlock(); } } } public boolean isInFallbackMode() { - boolean globalFallback = fallbackState.getInFallbackMode().get(); - syncFallbackModeState(globalFallback); - if (options.isEnablePerChannelRecovery()) { - return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; - } - return (globalFallback && fallbackChannel != null) || primaryChannel == null; + syncFallbackModeState(); + return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; } @VisibleForTesting @@ -236,12 +222,20 @@ AtomicBoolean getLocalInFallbackMode() { return localInFallbackMode; } + @VisibleForTesting + AtomicLong getLocalGeneration() { + return localGeneration; + } + @VisibleForTesting AtomicLong getLocalProbeSuccesses() { return localProbeSuccesses; } private void init() { + localGeneration.set(fallbackState.getGeneration()); + localInFallbackMode.set( + fallbackState.isInFallbackMode() && fallbackState.getTargetFallbackMode()); if (options.getPrimaryProbingFunction() != null) { this.primaryProbeFuture = fallbackState.scheduleTask( @@ -264,10 +258,12 @@ private void init() { } private void processPrimaryStatusCode(Status.Code statusCode) { - if (options.getErroneousStates().contains(statusCode)) { - fallbackState.getPrimaryFailures().incrementAndGet(); - } else { - fallbackState.getPrimarySuccesses().incrementAndGet(); + if (fallbackChannel != null) { + if (options.getErroneousStates().contains(statusCode)) { + fallbackState.getPrimaryFailures().incrementAndGet(); + } else { + fallbackState.getPrimarySuccesses().incrementAndGet(); + } } openTelemetry.getModule().reportStatus(options.getPrimaryChannelName(), statusCode); } @@ -282,25 +278,26 @@ private void processFallbackStatusCode(Status.Code statusCode) { } private void probePrimary() { - boolean globalFallback = fallbackState.getInFallbackMode().get(); - syncFallbackModeState(globalFallback); - boolean inFallback = - options.isEnablePerChannelRecovery() - ? localInFallbackMode.get() - : globalFallback; - if (!inFallback && primaryChannel != null) { + if (!localInFallbackMode.get() && primaryChannel != null) { + return; + } + syncFallbackModeState(); + if (!localInFallbackMode.get() && primaryChannel != null) { return; } + long probeStartGen = localGeneration.get(); String result = ""; if (primaryDelegateChannel == null) { result = INIT_FAILURE_REASON; } else { result = options.getPrimaryProbingFunction().apply(primaryDelegateChannel); } - if ("OK".equals(result)) { - stateLock.lock(); - try { - if (localInFallbackMode.get()) { + stateLock.lock(); + try { + syncFallbackModeState(); + if (localGeneration.get() == probeStartGen + && (localInFallbackMode.get() || primaryChannel == null)) { + if ("OK".equals(result)) { long nowNanos = System.nanoTime(); long firstSuccessNanos = localFirstPrimaryProbeSuccessNanos.updateAndGet(prev -> prev == 0 ? nowNanos : prev); @@ -316,26 +313,23 @@ private void probePrimary() { } if (options.isEnableRecovery() + && fallbackChannel != null && primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() && durationSatisfied) { - fallbackState.getInFallbackMode().set(false); + long recoveredGen = + fallbackState.recordRecovery(options.isEnablePerChannelRecovery()); localInFallbackMode.set(false); localProbeSuccesses.set(0); localFirstPrimaryProbeSuccessNanos.set(0); + localGeneration.set(recoveredGen); } + } else { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); } - } finally { - stateLock.unlock(); - } - } else { - stateLock.lock(); - try { - localInFallbackMode.set(true); - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - } finally { - stateLock.unlock(); } + } finally { + stateLock.unlock(); } // Report metric based on result. openTelemetry.getModule().reportProbeResult(options.getPrimaryChannelName(), result); diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index b6e8fd5099fa..cc637fd58744 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -37,6 +37,8 @@ public class GcpFallbackState { private final AtomicLong primaryFailures = new AtomicLong(0); private final AtomicLong fallbackSuccesses = new AtomicLong(0); private final AtomicLong fallbackFailures = new AtomicLong(0); + private final AtomicLong generation = new AtomicLong(0); + private final AtomicBoolean targetFallbackMode = new AtomicBoolean(false); private final AtomicBoolean inFallbackMode = new AtomicBoolean(false); private final AtomicBoolean evaluationStarted = new AtomicBoolean(false); @@ -52,29 +54,57 @@ public GcpFallbackState() {} * @param execService the executor service to use. */ @VisibleForTesting - public GcpFallbackState(ScheduledExecutorService execService) { + GcpFallbackState(ScheduledExecutorService execService) { this.execService = execService; this.ownsExecutor = true; } - public AtomicLong getPrimarySuccesses() { + AtomicLong getPrimarySuccesses() { return primarySuccesses; } - public AtomicLong getPrimaryFailures() { + AtomicLong getPrimaryFailures() { return primaryFailures; } - public AtomicLong getFallbackSuccesses() { + AtomicLong getFallbackSuccesses() { return fallbackSuccesses; } - public AtomicLong getFallbackFailures() { + AtomicLong getFallbackFailures() { return fallbackFailures; } - public AtomicBoolean getInFallbackMode() { - return inFallbackMode; + public boolean isInFallbackMode() { + return inFallbackMode.get(); + } + + long getGeneration() { + return generation.get(); + } + + boolean getTargetFallbackMode() { + return targetFallbackMode.get(); + } + + /** Bumps the generation counter with a directive to target fallback mode. */ + synchronized void triggerFallback() { + inFallbackMode.set(true); + targetFallbackMode.set(true); + generation.incrementAndGet(); + } + + /** Records channel recovery by clearing primary error counts and updating fallback mode. */ + synchronized long recordRecovery(boolean perChannelRecovery) { + if (inFallbackMode.get()) { + primaryFailures.set(0); + primarySuccesses.set(0); + inFallbackMode.set(false); + } + if (!perChannelRecovery && targetFallbackMode.compareAndSet(true, false)) { + generation.incrementAndGet(); + } + return generation.get(); } /** @@ -83,7 +113,7 @@ public AtomicBoolean getInFallbackMode() { * @param options optional fallback channel configuration options. * @return the active ScheduledExecutorService. */ - public synchronized ScheduledExecutorService getOrCreateExecutorService( + synchronized ScheduledExecutorService getOrCreateExecutorService( GcpFallbackChannelOptions options) { if (this.execService != null) { return this.execService; @@ -101,7 +131,7 @@ public synchronized ScheduledExecutorService getOrCreateExecutorService( } /** Schedules a periodic task (e.g., probe) on the shared background executor service. */ - public synchronized ScheduledFuture scheduleTask( + synchronized ScheduledFuture scheduleTask( Runnable command, long initialDelay, long period, TimeUnit unit) { if (this.execService == null || this.execService.isShutdown()) { return null; @@ -115,7 +145,7 @@ public synchronized ScheduledFuture scheduleTask( * * @param options the fallback channel configuration options. */ - public synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { + synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { if (options == null || !options.isEnableFallback() || options.getPeriod() == null @@ -148,8 +178,9 @@ public synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions optio * @param options the fallback channel configuration options. * @param openTelemetry telemetry module for recording error metrics. */ - public void checkErrorRates( + void checkErrorRates( GcpFallbackChannelOptions options, GcpFallbackOpenTelemetry openTelemetry) { + boolean wasInFallback = inFallbackMode.get(); long successes = primarySuccesses.getAndSet(0); long failures = primaryFailures.getAndSet(0); float errRate = 0f; @@ -160,9 +191,9 @@ public void checkErrorRates( openTelemetry.getModule().reportErrorRate(options.getPrimaryChannelName(), errRate); } - if (!inFallbackMode.get() && options.isEnableFallback()) { + if (!wasInFallback && options.isEnableFallback()) { if (failures >= options.getMinFailedCalls() && errRate >= options.getErrorRateThreshold()) { - inFallbackMode.set(true); + triggerFallback(); if (openTelemetry != null && openTelemetry.getModule() != null) { openTelemetry .getModule() @@ -189,7 +220,7 @@ public void checkErrorRates( } /** Stops any running scheduled evaluation. */ - public synchronized void stopPeriodicEvaluation() { + synchronized void stopPeriodicEvaluation() { if (scheduledEvaluationFuture != null) { scheduledEvaluationFuture.cancel(false); scheduledEvaluationFuture = null; diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index e27bf41d502b..3ac67aebe720 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -721,8 +721,13 @@ public void testNoFallback_minCallsNotMet() { @Test public void testNoFallback_fallbackChannelBuildFails() { + GcpFallbackState sharedState = new GcpFallbackState(mockScheduledExecutorService); GcpFallbackChannelOptions options = - getDefaultOptionsBuilder().setMinFailedCalls(1).setErrorRateThreshold(0.1f).build(); + getDefaultOptionsBuilder() + .setMinFailedCalls(1) + .setErrorRateThreshold(0.1f) + .setSharedState(sharedState) + .build(); initializeChannelWithInvalidFallbackBuilderAndCaptureTasks(options); assertFalse("Should not be in fallback mode initially.", gcpFallbackChannel.isInFallbackMode()); @@ -738,6 +743,10 @@ public void testNoFallback_fallbackChannelBuildFails() { checkErrorRatesTask.run(); assertFalse("Should not be in fallback mode.", gcpFallbackChannel.isInFallbackMode()); + assertFalse( + "Shared state should not trip fallback when fallback channel is null.", + sharedState.isInFallbackMode()); + assertEquals(0L, sharedState.getGeneration()); assertEquals(primaryAuthority, gcpFallbackChannel.authority()); } @@ -1084,7 +1093,8 @@ public void testProbingTasksScheduled_ifConfigured() { .build(); initializeChannelAndCaptureTasks(options); - gcpFallbackChannel.getFallbackState().getInFallbackMode().set(true); + gcpFallbackChannel.getFallbackState().triggerFallback(); + gcpFallbackChannel.isInFallbackMode(); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1121,7 +1131,8 @@ public void testProbing_reportsMetrics() throws InterruptedException { .build(); initializeChannelAndCaptureTasks(options); - gcpFallbackChannel.getFallbackState().getInFallbackMode().set(true); + gcpFallbackChannel.getFallbackState().triggerFallback(); + gcpFallbackChannel.isInFallbackMode(); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1216,7 +1227,8 @@ public void testProbing_reportsInitFailureForFallback() throws InterruptedExcept .build(); initializeChannelWithInvalidFallbackBuilderAndCaptureTasks(options); - gcpFallbackChannel.getFallbackState().getInFallbackMode().set(true); + gcpFallbackChannel.getFallbackState().triggerFallback(); + gcpFallbackChannel.isInFallbackMode(); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1464,7 +1476,7 @@ public void testSharedState_channelShutdownLeavesSiblingChannelsFunctional() { verify(mockExec1, never()).shutdown(); // Channel 2 can still transition and read shared fallback state - sharedState.getInFallbackMode().set(true); + sharedState.triggerFallback(); assertTrue(channel2.isInFallbackMode()); } finally { channel2.shutdownNow(); @@ -1478,7 +1490,7 @@ public void testSharedState_probingRequiresBothCountAndDurationToRecover() throws InterruptedException { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec); - sharedState.getInFallbackMode().set(true); + sharedState.triggerFallback(); GcpFallbackChannelOptions options = getDefaultOptionsBuilder() @@ -1529,7 +1541,7 @@ public void testSharedState_probingRequiresBothCountAndDurationToRecover() public void testSharedState_probingFailureResetsDurationTimer() { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec); - sharedState.getInFallbackMode().set(true); + sharedState.triggerFallback(); AtomicBoolean probeOk = new AtomicBoolean(true); GcpFallbackChannelOptions options = @@ -1574,7 +1586,6 @@ public void testProbePrimary_skippedWhenNotInFallbackMode() { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); AtomicLong probeCalls = new AtomicLong(0); GcpFallbackState sharedState = new GcpFallbackState(mockExec); - sharedState.getInFallbackMode().set(false); GcpFallbackChannelOptions options = getDefaultOptionsBuilder() @@ -1615,7 +1626,7 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec1); - sharedState.getInFallbackMode().set(true); // Pool-wide fallback active + sharedState.triggerFallback(); // Pool-wide fallback active GcpFallbackChannelOptions options1 = getDefaultOptionsBuilder() @@ -1660,7 +1671,7 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI assertFalse("Channel 1 should recover to DirectPath", channel1.isInFallbackMode()); assertTrue("Channel 2 should remain in CloudPath fallback mode", channel2.isInFallbackMode()); - assertFalse("Global fallback should be unlatched", sharedState.getInFallbackMode().get()); + assertFalse("Global fallback should be unlatched", sharedState.isInFallbackMode()); } finally { channel1.shutdownNow(); channel2.shutdownNow(); @@ -1673,7 +1684,7 @@ public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsReco ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec1); - sharedState.getInFallbackMode().set(true); // Pool-wide fallback active + sharedState.triggerFallback(); // Pool-wide fallback active GcpFallbackChannelOptions options1 = getDefaultOptionsBuilder() @@ -1721,7 +1732,7 @@ public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsReco assertFalse("Channel 1 should recover to DirectPath", channel1.isInFallbackMode()); assertFalse( "Channel 2 should also recover to DirectPath with pool", channel2.isInFallbackMode()); - assertFalse("Global fallback should be unlatched", sharedState.getInFallbackMode().get()); + assertFalse("Global fallback should be unlatched", sharedState.isInFallbackMode()); } finally { channel1.shutdownNow(); channel2.shutdownNow(); @@ -1733,7 +1744,7 @@ public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsReco public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec); - sharedState.getInFallbackMode().set(true); // Pool-wide fallback active + sharedState.triggerFallback(); // Pool-wide fallback active GcpFallbackChannelOptions options = getDefaultOptionsBuilder() @@ -1764,7 +1775,7 @@ public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { // Channel must remain in fallback mode assertTrue(channel.isInFallbackMode()); - assertTrue(sharedState.getInFallbackMode().get()); + assertTrue(sharedState.isInFallbackMode()); } finally { channel.shutdownNow(); sharedState.shutdown(); @@ -1775,7 +1786,7 @@ public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec); - sharedState.getInFallbackMode().set(true); // Cycle 1: Fallback active + sharedState.triggerFallback(); // Cycle 1: Fallback active GcpFallbackChannelOptions options1 = getDefaultOptionsBuilder() @@ -1824,7 +1835,7 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { assertFalse(channel2.isInFallbackMode()); // Now Cycle 2: Incident occurs again, pool enters fallback - sharedState.getInFallbackMode().set(true); + sharedState.triggerFallback(); assertTrue(channel2.isInFallbackMode()); // Channel 2's localProbeSuccesses must be reset to 0 in new cycle @@ -1838,7 +1849,7 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { // Probe second time: count is 2 (>= 2 required) -> recovers! probeTask2.run(); assertFalse(channel2.isInFallbackMode()); - assertFalse(sharedState.getInFallbackMode().get()); + assertFalse(sharedState.isInFallbackMode()); } finally { channel1.shutdownNow(); channel2.shutdownNow(); @@ -1851,7 +1862,7 @@ public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() throws InterruptedException, java.util.concurrent.ExecutionException { ScheduledExecutorService mockExec = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec); - sharedState.getInFallbackMode().set(true); + sharedState.triggerFallback(); GcpFallbackChannelOptions options = getDefaultOptionsBuilder() @@ -1976,4 +1987,98 @@ public void testSharedState_channelLifecycleIndependentOfSharedExecutor() sharedState.shutdown(); } } + + @Test + public void + testLazyChannelSwitching_idleChannelAndFalsePositiveProbeCannotClearFallbackPrematurely() { + ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); + ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(mockExec1); + + AtomicLong channel2ProbeCalls = new AtomicLong(0); + GcpFallbackChannelOptions options1 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(true) + .setPrimaryProbingFunction(channel -> "OK") + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + GcpFallbackChannelOptions options2 = + getDefaultOptionsBuilder() + .setSharedState(sharedState) + .setEnableRecovery(true) + .setEnablePerChannelRecovery(true) + .setPrimaryProbingFunction( + channel -> { + channel2ProbeCalls.incrementAndGet(); + return "OK"; + }) + .setMinPrimaryProbeSuccessCount(1) + .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .build(); + + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + + GcpFallbackChannel channel1 = + new GcpFallbackChannel(options1, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); + GcpFallbackChannel channel2 = + new GcpFallbackChannel(options2, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); + + try { + verify(mockExec1, atLeast(2)) + .scheduleAtFixedRate( + taskCaptor.capture(), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(options1.getPrimaryProbingInterval().toMillis()), + eq(MILLISECONDS)); + Runnable probeTask1 = taskCaptor.getAllValues().get(0); + Runnable probeTask2 = taskCaptor.getAllValues().get(1); + + assertEquals(0, sharedState.getGeneration()); + + // 1. Shared stats trip -> generation counter is bumped with directive "prefer fallback" + sharedState.triggerFallback(); + assertEquals(1, sharedState.getGeneration()); + assertTrue(sharedState.getTargetFallbackMode()); + + // 2. Channel 2 is idle (no calls yet). Its local generation is still 0. + assertEquals(0, channel2.getLocalGeneration().get()); + assertFalse(channel2.getLocalInFallbackMode().get()); + + // Idle Channel 2's background probe runs: because switching is lazy, it has not switched yet + // and must NOT probe or clear the pool fallback state. + probeTask2.run(); + assertEquals(0, channel2ProbeCalls.get()); + assertEquals(1, sharedState.getGeneration()); + assertTrue(sharedState.getTargetFallbackMode()); + + // 3. Channel 1 makes a call -> checks its local generation against pool generation (0 < 1) + // and lazily switches itself to fallback. + assertTrue(channel1.isInFallbackMode()); + assertEquals(1, channel1.getLocalGeneration().get()); + + // 4. Channel 1 probes primary and recovers (or gets a false-positive probe success). + probeTask1.run(); + assertFalse("Channel 1 should recover locally", channel1.isInFallbackMode()); + + // Pool generation and prefer-fallback directive remain intact for generation 1! + assertEquals(1, sharedState.getGeneration()); + assertTrue(sharedState.getTargetFallbackMode()); + + // 5. Idle Channel 2 now makes its next call AFTER Channel 1 already recovered. + // Because its local generation (0) is behind the pool generation (1), it still switches + // itself to fallback on its next call! + assertTrue( + "Idle Channel 2 must switch to fallback on next call despite Channel 1 recovery", + channel2.isInFallbackMode()); + assertEquals(1, channel2.getLocalGeneration().get()); + } finally { + channel1.shutdownNow(); + channel2.shutdownNow(); + sharedState.shutdown(); + } + } } From 5731104d6526d94a9a971bbbd4663c9dd9e78888 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Wed, 16 Sep 2026 21:46:17 +0000 Subject: [PATCH 11/15] fixes --- .../grpc/fallback/GcpFallbackChannel.java | 38 ++++--- .../cloud/grpc/fallback/GcpFallbackState.java | 105 ++++++++++++------ 2 files changed, 96 insertions(+), 47 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 8f4c55675424..8d39925f810e 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -194,12 +194,14 @@ private void syncFallbackModeState() { if (localGeneration.get() < fallbackState.getGeneration()) { stateLock.lock(); try { - long poolGen = fallbackState.getGeneration(); - if (localGeneration.get() < poolGen) { - localInFallbackMode.set(fallbackState.getTargetFallbackMode()); - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - localGeneration.set(poolGen); + synchronized (fallbackState) { + long poolGen = fallbackState.getGeneration(); + if (localGeneration.get() < poolGen) { + localInFallbackMode.set(fallbackState.getTargetFallbackMode()); + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + localGeneration.set(poolGen); + } } } finally { stateLock.unlock(); @@ -233,9 +235,10 @@ AtomicLong getLocalProbeSuccesses() { } private void init() { - localGeneration.set(fallbackState.getGeneration()); - localInFallbackMode.set( - fallbackState.isInFallbackMode() && fallbackState.getTargetFallbackMode()); + synchronized (fallbackState) { + localGeneration.set(fallbackState.getGeneration()); + localInFallbackMode.set(fallbackState.getTargetFallbackMode()); + } if (options.getPrimaryProbingFunction() != null) { this.primaryProbeFuture = fallbackState.scheduleTask( @@ -258,7 +261,7 @@ private void init() { } private void processPrimaryStatusCode(Status.Code statusCode) { - if (fallbackChannel != null) { + if (fallbackChannel != null && !localInFallbackMode.get()) { if (options.getErroneousStates().contains(statusCode)) { fallbackState.getPrimaryFailures().incrementAndGet(); } else { @@ -317,11 +320,16 @@ private void probePrimary() { && primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() && durationSatisfied) { long recoveredGen = - fallbackState.recordRecovery(options.isEnablePerChannelRecovery()); - localInFallbackMode.set(false); - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - localGeneration.set(recoveredGen); + fallbackState.recordRecovery( + probeStartGen, options.isEnablePerChannelRecovery()); + if (recoveredGen != -1) { + localInFallbackMode.set(false); + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + localGeneration.set(recoveredGen); + } else { + syncFallbackModeState(); + } } } else { localProbeSuccesses.set(0); diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index cc637fd58744..665799a26de1 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -26,11 +26,27 @@ import java.util.concurrent.atomic.AtomicLong; /** - * Shared thread-safe state container for coordinated pool-wide failover, recovery, and background - * tasks. + * Shared state that coordinates failover across a pool of channels. All channels in a pool share + * this instance and its background executor, so probing and error rate evaluation run once for the + * whole pool. * - *

All channels in a pool share this state instance and its background executor service, - * consolidating probing and error evaluation threads across the entire channel pool. + * targetFallbackMode: whether channels should route to the fallback channel. + * generation: incremented on every pool-wide transition. A channel re-reads targetFallbackMode when + * generation advances past the value it last saw. + * inFallbackMode: whether all traffic is on the fallback channel. Gates error rate evaluation. + * + * Failover: the primary error rate crossing the threshold sets targetFallbackMode and increments + * generation. All channels switch to the fallback channel. + * + * Recovery, per-channel disabled: the first channel whose probes succeed clears targetFallbackMode + * and increments generation. The whole pool returns to the primary channel. + * + * Recovery, per-channel enabled: a recovering channel updates only its own state, leaving + * targetFallbackMode and generation unchanged. Other channels stay on the fallback channel until + * their own probes succeed. inFallbackMode and targetFallbackMode diverge until then. + * + * A shared instance passed to setSharedState must be shut down by the caller. A state a channel + * created for itself is shut down with that channel. */ public class GcpFallbackState { private final AtomicLong primarySuccesses = new AtomicLong(0); @@ -75,7 +91,16 @@ AtomicLong getFallbackFailures() { return fallbackFailures; } - public boolean isInFallbackMode() { + /** + * Returns whether periodic error-rate evaluation is armed, i.e. whether any traffic is currently + * reaching the primary channel. + * + *

This is not "are the pool's channels routing to the fallback channel". Under + * per-channel recovery this returns {@code false} as soon as the first channel recovers, while + * other channels may still be in fallback. To ask whether a specific channel is in fallback, call + * {@link GcpFallbackChannel#isInFallbackMode()} on that channel. + */ + boolean isInFallbackMode() { return inFallbackMode.get(); } @@ -94,8 +119,17 @@ synchronized void triggerFallback() { generation.incrementAndGet(); } - /** Records channel recovery by clearing primary error counts and updating fallback mode. */ - synchronized long recordRecovery(boolean perChannelRecovery) { + /** + * Records channel recovery by clearing primary error counts and updating fallback mode. + * + * @param expectedGen the generation at which the recovery probe started. + * @param perChannelRecovery whether recovery is scoped per channel rather than pool-wide. + * @return the resulting pool generation, or -1 if the pool generation changed concurrently. + */ + synchronized long recordRecovery(long expectedGen, boolean perChannelRecovery) { + if (generation.get() != expectedGen) { + return -1; + } if (inFallbackMode.get()) { primaryFailures.set(0); primarySuccesses.set(0); @@ -180,42 +214,49 @@ synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { */ void checkErrorRates( GcpFallbackChannelOptions options, GcpFallbackOpenTelemetry openTelemetry) { - boolean wasInFallback = inFallbackMode.get(); - long successes = primarySuccesses.getAndSet(0); - long failures = primaryFailures.getAndSet(0); - float errRate = 0f; - if (failures + successes > 0) { - errRate = (float) failures / (failures + successes); - } - if (openTelemetry != null && openTelemetry.getModule() != null) { - openTelemetry.getModule().reportErrorRate(options.getPrimaryChannelName(), errRate); + float primaryErrRate = 0f; + boolean fallbackTriggered = false; + boolean currentInFallback; + synchronized (this) { + boolean wasInFallback = inFallbackMode.get(); + long successes = primarySuccesses.getAndSet(0); + long failures = primaryFailures.getAndSet(0); + if (failures + successes > 0) { + primaryErrRate = (float) failures / (failures + successes); + } + if (!wasInFallback && options.isEnableFallback()) { + if (failures >= options.getMinFailedCalls() + && primaryErrRate >= options.getErrorRateThreshold()) { + triggerFallback(); + fallbackTriggered = true; + } + } + currentInFallback = inFallbackMode.get(); } - if (!wasInFallback && options.isEnableFallback()) { - if (failures >= options.getMinFailedCalls() && errRate >= options.getErrorRateThreshold()) { - triggerFallback(); - if (openTelemetry != null && openTelemetry.getModule() != null) { - openTelemetry - .getModule() - .reportFallback(options.getPrimaryChannelName(), options.getFallbackChannelName()); - } + if (openTelemetry != null && openTelemetry.getModule() != null) { + openTelemetry.getModule().reportErrorRate(options.getPrimaryChannelName(), primaryErrRate); + if (fallbackTriggered) { + openTelemetry + .getModule() + .reportFallback(options.getPrimaryChannelName(), options.getFallbackChannelName()); } } - successes = fallbackSuccesses.getAndSet(0); - failures = fallbackFailures.getAndSet(0); - errRate = 0f; - if (failures + successes > 0) { - errRate = (float) failures / (failures + successes); + long fallbackSucc = fallbackSuccesses.getAndSet(0); + long fallbackFail = fallbackFailures.getAndSet(0); + float fallbackErrRate = 0f; + if (fallbackFail + fallbackSucc > 0) { + fallbackErrRate = (float) fallbackFail / (fallbackFail + fallbackSucc); } if (openTelemetry != null && openTelemetry.getModule() != null) { - openTelemetry.getModule().reportErrorRate(options.getFallbackChannelName(), errRate); + openTelemetry.getModule().reportErrorRate(options.getFallbackChannelName(), fallbackErrRate); openTelemetry .getModule() - .reportCurrentChannel(options.getPrimaryChannelName(), !inFallbackMode.get()); + .reportCurrentChannel(options.getPrimaryChannelName(), !currentInFallback); openTelemetry .getModule() - .reportCurrentChannel(options.getFallbackChannelName(), inFallbackMode.get()); + .reportCurrentChannel(options.getFallbackChannelName(), currentInFallback); } } From bfce1d5ae24ead308c0fe2b8fefbfd2bde08c81e Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Thu, 17 Sep 2026 02:14:19 +0000 Subject: [PATCH 12/15] refactor(grpc-gcp): defer per channel recovery --- .../grpc/fallback/GcpFallbackChannel.java | 129 ++++-------- .../fallback/GcpFallbackChannelOptions.java | 12 -- .../cloud/grpc/fallback/GcpFallbackState.java | 62 ++---- .../grpc/fallback/GcpFallbackChannelTest.java | 194 +++--------------- 4 files changed, 79 insertions(+), 318 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 8d39925f810e..984cf5edfeb5 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -33,9 +33,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -55,11 +53,9 @@ public class GcpFallbackChannel extends ManagedChannel { private final boolean ownsFallbackState; private final GcpFallbackOpenTelemetry openTelemetry; - private final AtomicBoolean localInFallbackMode = new AtomicBoolean(false); - private final AtomicLong localGeneration = new AtomicLong(0); + private final AtomicLong localProbeGeneration = new AtomicLong(0); private final AtomicLong localProbeSuccesses = new AtomicLong(0); private final AtomicLong localFirstPrimaryProbeSuccessNanos = new AtomicLong(0); - private final ReentrantLock stateLock = new ReentrantLock(); private final ScheduledExecutorService execService; private volatile ScheduledFuture primaryProbeFuture = null; @@ -94,9 +90,7 @@ public GcpFallbackChannel( this.ownsFallbackState = false; } else { this.fallbackState = - execService != null - ? new GcpFallbackState(execService) - : new GcpFallbackState(); + execService != null ? new GcpFallbackState(execService) : new GcpFallbackState(); this.ownsFallbackState = true; } this.execService = fallbackState.getOrCreateExecutorService(options); @@ -166,9 +160,7 @@ public GcpFallbackChannel( this.ownsFallbackState = false; } else { this.fallbackState = - execService != null - ? new GcpFallbackState(execService) - : new GcpFallbackState(); + execService != null ? new GcpFallbackState(execService) : new GcpFallbackState(); this.ownsFallbackState = true; } this.execService = fallbackState.getOrCreateExecutorService(options); @@ -190,28 +182,17 @@ public GcpFallbackChannel( init(); } - private void syncFallbackModeState() { - if (localGeneration.get() < fallbackState.getGeneration()) { - stateLock.lock(); - try { - synchronized (fallbackState) { - long poolGen = fallbackState.getGeneration(); - if (localGeneration.get() < poolGen) { - localInFallbackMode.set(fallbackState.getTargetFallbackMode()); - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - localGeneration.set(poolGen); - } - } - } finally { - stateLock.unlock(); - } + private void resetProbeStatsIfGenerationChanged() { + long currentGen = fallbackState.getGeneration(); + if (localProbeGeneration.get() != currentGen) { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); + localProbeGeneration.set(currentGen); } } public boolean isInFallbackMode() { - syncFallbackModeState(); - return (localInFallbackMode.get() && fallbackChannel != null) || primaryChannel == null; + return (fallbackState.isInFallbackMode() && fallbackChannel != null) || primaryChannel == null; } @VisibleForTesting @@ -219,26 +200,14 @@ GcpFallbackState getFallbackState() { return fallbackState; } - @VisibleForTesting - AtomicBoolean getLocalInFallbackMode() { - return localInFallbackMode; - } - - @VisibleForTesting - AtomicLong getLocalGeneration() { - return localGeneration; - } - @VisibleForTesting AtomicLong getLocalProbeSuccesses() { + resetProbeStatsIfGenerationChanged(); return localProbeSuccesses; } private void init() { - synchronized (fallbackState) { - localGeneration.set(fallbackState.getGeneration()); - localInFallbackMode.set(fallbackState.getTargetFallbackMode()); - } + localProbeGeneration.set(fallbackState.getGeneration()); if (options.getPrimaryProbingFunction() != null) { this.primaryProbeFuture = fallbackState.scheduleTask( @@ -261,7 +230,7 @@ private void init() { } private void processPrimaryStatusCode(Status.Code statusCode) { - if (fallbackChannel != null && !localInFallbackMode.get()) { + if (fallbackChannel != null && !fallbackState.isInFallbackMode()) { if (options.getErroneousStates().contains(statusCode)) { fallbackState.getPrimaryFailures().incrementAndGet(); } else { @@ -281,63 +250,41 @@ private void processFallbackStatusCode(Status.Code statusCode) { } private void probePrimary() { - if (!localInFallbackMode.get() && primaryChannel != null) { - return; - } - syncFallbackModeState(); - if (!localInFallbackMode.get() && primaryChannel != null) { + if (!fallbackState.isInFallbackMode() && primaryChannel != null) { return; } - long probeStartGen = localGeneration.get(); + resetProbeStatsIfGenerationChanged(); + long probeStartGen = localProbeGeneration.get(); String result = ""; if (primaryDelegateChannel == null) { result = INIT_FAILURE_REASON; } else { result = options.getPrimaryProbingFunction().apply(primaryDelegateChannel); } - stateLock.lock(); - try { - syncFallbackModeState(); - if (localGeneration.get() == probeStartGen - && (localInFallbackMode.get() || primaryChannel == null)) { - if ("OK".equals(result)) { - long nowNanos = System.nanoTime(); - long firstSuccessNanos = - localFirstPrimaryProbeSuccessNanos.updateAndGet(prev -> prev == 0 ? nowNanos : prev); - long primaryProbeSuccessCount = localProbeSuccesses.incrementAndGet(); - - boolean durationSatisfied = true; - if (options.getMinPrimaryProbeSuccessDuration() != null - && !options.getMinPrimaryProbeSuccessDuration().isZero() - && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { - long elapsedNanos = nowNanos - firstSuccessNanos; - durationSatisfied = - elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); - } - - if (options.isEnableRecovery() - && fallbackChannel != null - && primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() - && durationSatisfied) { - long recoveredGen = - fallbackState.recordRecovery( - probeStartGen, options.isEnablePerChannelRecovery()); - if (recoveredGen != -1) { - localInFallbackMode.set(false); - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); - localGeneration.set(recoveredGen); - } else { - syncFallbackModeState(); - } - } - } else { - localProbeSuccesses.set(0); - localFirstPrimaryProbeSuccessNanos.set(0); + if ("".equals(result) && fallbackState.getGeneration() == probeStartGen) { + if (options.isEnableRecovery() && fallbackChannel != null) { + long nowNanos = System.nanoTime(); + long firstSuccessNanos = + localFirstPrimaryProbeSuccessNanos.updateAndGet(prev -> prev == 0 ? nowNanos : prev); + long primaryProbeSuccessCount = localProbeSuccesses.incrementAndGet(); + + boolean durationSatisfied = true; + if (options.getMinPrimaryProbeSuccessDuration() != null + && !options.getMinPrimaryProbeSuccessDuration().isZero() + && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { + long elapsedNanos = nowNanos - firstSuccessNanos; + durationSatisfied = + elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); + } + + if (primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() + && durationSatisfied) { + fallbackState.recordRecovery(probeStartGen); } } - } finally { - stateLock.unlock(); + } else { + localProbeSuccesses.set(0); + localFirstPrimaryProbeSuccessNanos.set(0); } // Report metric based on result. openTelemetry.getModule().reportProbeResult(options.getPrimaryChannelName(), result); diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java index 0fcc21fd4e34..077897612257 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java @@ -41,7 +41,6 @@ public class GcpFallbackChannelOptions { private final int minPrimaryProbeSuccessCount; private final Duration minPrimaryProbeSuccessDuration; private final boolean enableRecovery; - private final boolean enablePerChannelRecovery; private final String primaryChannelName; private final String fallbackChannelName; private final GcpFallbackOpenTelemetry openTelemetry; @@ -61,7 +60,6 @@ public GcpFallbackChannelOptions(Builder builder) { this.minPrimaryProbeSuccessCount = builder.minPrimaryProbeSuccessCount; this.minPrimaryProbeSuccessDuration = builder.minPrimaryProbeSuccessDuration; this.enableRecovery = builder.enableRecovery; - this.enablePerChannelRecovery = builder.enablePerChannelRecovery; this.primaryChannelName = builder.primaryChannelName; this.fallbackChannelName = builder.fallbackChannelName; this.openTelemetry = builder.openTelemetry; @@ -121,10 +119,6 @@ public boolean isEnableRecovery() { return enableRecovery; } - public boolean isEnablePerChannelRecovery() { - return enablePerChannelRecovery; - } - public String getPrimaryChannelName() { return primaryChannelName; } @@ -162,7 +156,6 @@ public static class Builder { private int minPrimaryProbeSuccessCount = 10; private Duration minPrimaryProbeSuccessDuration = Duration.ZERO; private boolean enableRecovery = false; - private boolean enablePerChannelRecovery = false; private String primaryChannelName = "primary"; private String fallbackChannelName = "fallback"; @@ -243,11 +236,6 @@ public Builder setEnableRecovery(boolean enableRecovery) { return this; } - public Builder setEnablePerChannelRecovery(boolean enablePerChannelRecovery) { - this.enablePerChannelRecovery = enablePerChannelRecovery; - return this; - } - public Builder setPrimaryChannelName(String primaryChannelName) { this.primaryChannelName = primaryChannelName; return this; diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index 665799a26de1..66e6c5b4b19e 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -26,27 +26,8 @@ import java.util.concurrent.atomic.AtomicLong; /** - * Shared state that coordinates failover across a pool of channels. All channels in a pool share - * this instance and its background executor, so probing and error rate evaluation run once for the - * whole pool. - * - * targetFallbackMode: whether channels should route to the fallback channel. - * generation: incremented on every pool-wide transition. A channel re-reads targetFallbackMode when - * generation advances past the value it last saw. - * inFallbackMode: whether all traffic is on the fallback channel. Gates error rate evaluation. - * - * Failover: the primary error rate crossing the threshold sets targetFallbackMode and increments - * generation. All channels switch to the fallback channel. - * - * Recovery, per-channel disabled: the first channel whose probes succeed clears targetFallbackMode - * and increments generation. The whole pool returns to the primary channel. - * - * Recovery, per-channel enabled: a recovering channel updates only its own state, leaving - * targetFallbackMode and generation unchanged. Other channels stay on the fallback channel until - * their own probes succeed. inFallbackMode and targetFallbackMode diverge until then. - * - * A shared instance passed to setSharedState must be shut down by the caller. A state a channel - * created for itself is shut down with that channel. + * Shared thread-safe state that coordinates failover, recovery, and periodic error evaluation + * across a pool of GcpFallbackChannel instances. */ public class GcpFallbackState { private final AtomicLong primarySuccesses = new AtomicLong(0); @@ -54,12 +35,12 @@ public class GcpFallbackState { private final AtomicLong fallbackSuccesses = new AtomicLong(0); private final AtomicLong fallbackFailures = new AtomicLong(0); private final AtomicLong generation = new AtomicLong(0); - private final AtomicBoolean targetFallbackMode = new AtomicBoolean(false); private final AtomicBoolean inFallbackMode = new AtomicBoolean(false); private final AtomicBoolean evaluationStarted = new AtomicBoolean(false); private ScheduledExecutorService execService = null; private boolean ownsExecutor = false; + private boolean isShutdown = false; private volatile ScheduledFuture scheduledEvaluationFuture = null; public GcpFallbackState() {} @@ -91,15 +72,7 @@ AtomicLong getFallbackFailures() { return fallbackFailures; } - /** - * Returns whether periodic error-rate evaluation is armed, i.e. whether any traffic is currently - * reaching the primary channel. - * - *

This is not "are the pool's channels routing to the fallback channel". Under - * per-channel recovery this returns {@code false} as soon as the first channel recovers, while - * other channels may still be in fallback. To ask whether a specific channel is in fallback, call - * {@link GcpFallbackChannel#isInFallbackMode()} on that channel. - */ + /** Returns whether the pool is currently in fallback mode. */ boolean isInFallbackMode() { return inFallbackMode.get(); } @@ -108,34 +81,25 @@ long getGeneration() { return generation.get(); } - boolean getTargetFallbackMode() { - return targetFallbackMode.get(); - } - - /** Bumps the generation counter with a directive to target fallback mode. */ + /** Bumps the generation counter and transitions the pool to fallback mode. */ synchronized void triggerFallback() { inFallbackMode.set(true); - targetFallbackMode.set(true); generation.incrementAndGet(); } /** - * Records channel recovery by clearing primary error counts and updating fallback mode. + * Records pool recovery by clearing primary error counts and updating fallback mode. * * @param expectedGen the generation at which the recovery probe started. - * @param perChannelRecovery whether recovery is scoped per channel rather than pool-wide. * @return the resulting pool generation, or -1 if the pool generation changed concurrently. */ - synchronized long recordRecovery(long expectedGen, boolean perChannelRecovery) { + synchronized long recordRecovery(long expectedGen) { if (generation.get() != expectedGen) { return -1; } - if (inFallbackMode.get()) { + if (inFallbackMode.compareAndSet(true, false)) { primaryFailures.set(0); primarySuccesses.set(0); - inFallbackMode.set(false); - } - if (!perChannelRecovery && targetFallbackMode.compareAndSet(true, false)) { generation.incrementAndGet(); } return generation.get(); @@ -167,7 +131,7 @@ synchronized ScheduledExecutorService getOrCreateExecutorService( /** Schedules a periodic task (e.g., probe) on the shared background executor service. */ synchronized ScheduledFuture scheduleTask( Runnable command, long initialDelay, long period, TimeUnit unit) { - if (this.execService == null || this.execService.isShutdown()) { + if (isShutdown || this.execService == null || this.execService.isShutdown()) { return null; } return this.execService.scheduleAtFixedRate(command, initialDelay, period, unit); @@ -180,7 +144,8 @@ synchronized ScheduledFuture scheduleTask( * @param options the fallback channel configuration options. */ synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { - if (options == null + if (isShutdown + || options == null || !options.isEnableFallback() || options.getPeriod() == null || options.getPeriod().toMillis() <= 0) { @@ -212,8 +177,7 @@ synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { * @param options the fallback channel configuration options. * @param openTelemetry telemetry module for recording error metrics. */ - void checkErrorRates( - GcpFallbackChannelOptions options, GcpFallbackOpenTelemetry openTelemetry) { + void checkErrorRates(GcpFallbackChannelOptions options, GcpFallbackOpenTelemetry openTelemetry) { float primaryErrRate = 0f; boolean fallbackTriggered = false; boolean currentInFallback; @@ -271,6 +235,7 @@ synchronized void stopPeriodicEvaluation() { /** Shuts down the state, cancelling evaluation and shutting down internal executor if owned. */ public synchronized void shutdown() { + isShutdown = true; stopPeriodicEvaluation(); if (ownsExecutor && execService != null && !execService.isShutdown()) { execService.shutdown(); @@ -282,6 +247,7 @@ public synchronized void shutdown() { * owned. */ public synchronized void shutdownNow() { + isShutdown = true; stopPeriodicEvaluation(); if (ownsExecutor && execService != null && !execService.isShutdown()) { execService.shutdownNow(); diff --git a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java index 3ac67aebe720..26e985323b1e 100644 --- a/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java +++ b/grpc-gcp-java/src/test/java/com/google/cloud/grpc/fallback/GcpFallbackChannelTest.java @@ -1094,7 +1094,6 @@ public void testProbingTasksScheduled_ifConfigured() { initializeChannelAndCaptureTasks(options); gcpFallbackChannel.getFallbackState().triggerFallback(); - gcpFallbackChannel.isInFallbackMode(); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1132,7 +1131,6 @@ public void testProbing_reportsMetrics() throws InterruptedException { initializeChannelAndCaptureTasks(options); gcpFallbackChannel.getFallbackState().triggerFallback(); - gcpFallbackChannel.isInFallbackMode(); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1228,7 +1226,6 @@ public void testProbing_reportsInitFailureForFallback() throws InterruptedExcept initializeChannelWithInvalidFallbackBuilderAndCaptureTasks(options); gcpFallbackChannel.getFallbackState().triggerFallback(); - gcpFallbackChannel.isInFallbackMode(); assertNotNull(primaryProbingTask); assertNotNull(fallbackProbingTask); @@ -1496,7 +1493,7 @@ public void testSharedState_probingRequiresBothCountAndDurationToRecover() getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setPrimaryProbingFunction(channel -> "OK") + .setPrimaryProbingFunction(channel -> "") .setMinPrimaryProbeSuccessCount(2) .setMinPrimaryProbeSuccessDuration(Duration.ofMillis(50)) .build(); @@ -1548,7 +1545,7 @@ public void testSharedState_probingFailureResetsDurationTimer() { getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setPrimaryProbingFunction(channel -> probeOk.get() ? "OK" : "UNAVAILABLE") + .setPrimaryProbingFunction(channel -> probeOk.get() ? "" : "UNAVAILABLE") .setMinPrimaryProbeSuccessCount(5) .setMinPrimaryProbeSuccessDuration(Duration.ofMinutes(10)) .build(); @@ -1593,7 +1590,7 @@ public void testProbePrimary_skippedWhenNotInFallbackMode() { .setPrimaryProbingFunction( channel -> { probeCalls.incrementAndGet(); - return "OK"; + return ""; }) .build(); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); @@ -1622,7 +1619,7 @@ public void testProbePrimary_skippedWhenNotInFallbackMode() { } @Test - public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysInFallback() { + public void testPoolLevelRecovery_allChannelsRecoverTogether() { ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); GcpFallbackState sharedState = new GcpFallbackState(mockExec1); @@ -1632,8 +1629,7 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setEnablePerChannelRecovery(true) - .setPrimaryProbingFunction(channel -> "OK") + .setPrimaryProbingFunction(channel -> "") .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); @@ -1642,65 +1638,6 @@ public void testPerChannelIndependentRecovery_oneChannelRecoversWhileOtherStaysI getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setEnablePerChannelRecovery(true) - .setPrimaryProbingFunction(channel -> "UNAVAILABLE") - .setMinPrimaryProbeSuccessCount(1) - .setMinPrimaryProbeSuccessDuration(Duration.ZERO) - .build(); - ArgumentCaptor taskCaptor1 = ArgumentCaptor.forClass(Runnable.class); - - GcpFallbackChannel channel1 = - new GcpFallbackChannel(options1, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); - GcpFallbackChannel channel2 = - new GcpFallbackChannel(options2, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); - - try { - verify(mockExec1, atLeastOnce()) - .scheduleAtFixedRate( - taskCaptor1.capture(), - eq(options1.getPrimaryProbingInterval().toMillis()), - eq(options1.getPrimaryProbingInterval().toMillis()), - eq(MILLISECONDS)); - Runnable probeTask1 = taskCaptor1.getAllValues().get(0); - - assertTrue(channel1.isInFallbackMode()); - assertTrue(channel2.isInFallbackMode()); - - // Run probe on channel 1 -> channel 1 recovers to DirectPath - probeTask1.run(); - - assertFalse("Channel 1 should recover to DirectPath", channel1.isInFallbackMode()); - assertTrue("Channel 2 should remain in CloudPath fallback mode", channel2.isInFallbackMode()); - assertFalse("Global fallback should be unlatched", sharedState.isInFallbackMode()); - } finally { - channel1.shutdownNow(); - channel2.shutdownNow(); - sharedState.shutdown(); - } - } - - @Test - public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsRecoverTogether() { - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); - ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); - GcpFallbackState sharedState = new GcpFallbackState(mockExec1); - sharedState.triggerFallback(); // Pool-wide fallback active - - GcpFallbackChannelOptions options1 = - getDefaultOptionsBuilder() - .setSharedState(sharedState) - .setEnableRecovery(true) - .setEnablePerChannelRecovery(false) // Pool-level recovery - .setPrimaryProbingFunction(channel -> "OK") - .setMinPrimaryProbeSuccessCount(1) - .setMinPrimaryProbeSuccessDuration(Duration.ZERO) - .build(); - - GcpFallbackChannelOptions options2 = - getDefaultOptionsBuilder() - .setSharedState(sharedState) - .setEnableRecovery(true) - .setEnablePerChannelRecovery(false) // Pool-level recovery .setPrimaryProbingFunction(channel -> "UNAVAILABLE") .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) @@ -1727,8 +1664,7 @@ public void testPoolLevelRecovery_whenPerChannelRecoveryDisabled_allChannelsReco // Run probe on channel 1 -> channel 1 recovers to DirectPath and unlatches global fallback probeTask1.run(); - // With enablePerChannelRecovery=false, both channel 1 and channel 2 recover to DirectPath - // together + // Both channel 1 and channel 2 recover to DirectPath together assertFalse("Channel 1 should recover to DirectPath", channel1.isInFallbackMode()); assertFalse( "Channel 2 should also recover to DirectPath with pool", channel2.isInFallbackMode()); @@ -1750,7 +1686,7 @@ public void testRecoveryDisabled_probingSucceedsButChannelRemainsInFallback() { getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(false) // Recovery disabled by default - .setPrimaryProbingFunction(channel -> "OK") + .setPrimaryProbingFunction(channel -> "") .setMinPrimaryProbeSuccessCount(1) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); @@ -1792,8 +1728,7 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setEnablePerChannelRecovery(false) - .setPrimaryProbingFunction(channel -> "OK") + .setPrimaryProbingFunction(channel -> "") .setMinPrimaryProbeSuccessCount(2) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); @@ -1802,8 +1737,7 @@ public void testPoolLevelRecovery_multipleFailoverCyclesResetProbeStatistics() { getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setEnablePerChannelRecovery(false) - .setPrimaryProbingFunction(channel -> "OK") + .setPrimaryProbingFunction(channel -> "") .setMinPrimaryProbeSuccessCount(2) .setMinPrimaryProbeSuccessDuration(Duration.ZERO) .build(); @@ -1868,8 +1802,7 @@ public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() getDefaultOptionsBuilder() .setSharedState(sharedState) .setEnableRecovery(true) - .setEnablePerChannelRecovery(true) - .setPrimaryProbingFunction(channel -> "OK") + .setPrimaryProbingFunction(channel -> "") .setMinPrimaryProbeSuccessCount(1000) .setMinPrimaryProbeSuccessDuration(Duration.ofHours(1)) .build(); @@ -1896,8 +1829,7 @@ public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() int iterationsPerThread = 200; java.util.concurrent.ExecutorService threadPool = java.util.concurrent.Executors.newFixedThreadPool(threadCount); - java.util.concurrent.CountDownLatch startLatch = - new java.util.concurrent.CountDownLatch(1); + java.util.concurrent.CountDownLatch startLatch = new java.util.concurrent.CountDownLatch(1); java.util.List> futures = new java.util.ArrayList<>(); for (int i = 0; i < threadCount; i++) { @@ -1939,9 +1871,7 @@ public void testConcurrentIsInFallbackModeDoesNotResetProbeSuccesses() public void testShutdown_whenSuppliedSharedExecutorService_leavesExecutorRunning() { ScheduledExecutorService sharedExec = mock(ScheduledExecutorService.class); GcpFallbackChannelOptions options = - getDefaultOptionsBuilder() - .setSharedExecutorService(sharedExec) - .build(); + getDefaultOptionsBuilder().setSharedExecutorService(sharedExec).build(); GcpFallbackChannel channel = new GcpFallbackChannel(options, mockPrimaryBuilder, mockFallbackBuilder); @@ -1989,96 +1919,26 @@ public void testSharedState_channelLifecycleIndependentOfSharedExecutor() } @Test - public void - testLazyChannelSwitching_idleChannelAndFalsePositiveProbeCannotClearFallbackPrematurely() { - ScheduledExecutorService mockExec1 = mock(ScheduledExecutorService.class); - ScheduledExecutorService mockExec2 = mock(ScheduledExecutorService.class); - GcpFallbackState sharedState = new GcpFallbackState(mockExec1); - - AtomicLong channel2ProbeCalls = new AtomicLong(0); - GcpFallbackChannelOptions options1 = + public void testSharedState_shutdownPreventsFurtherTaskSchedulingWithExternalExecutor() { + ScheduledExecutorService externalExec = mock(ScheduledExecutorService.class); + GcpFallbackState sharedState = new GcpFallbackState(); + GcpFallbackChannelOptions options = getDefaultOptionsBuilder() .setSharedState(sharedState) - .setEnableRecovery(true) - .setEnablePerChannelRecovery(true) - .setPrimaryProbingFunction(channel -> "OK") - .setMinPrimaryProbeSuccessCount(1) - .setMinPrimaryProbeSuccessDuration(Duration.ZERO) + .setSharedExecutorService(externalExec) .build(); - GcpFallbackChannelOptions options2 = - getDefaultOptionsBuilder() - .setSharedState(sharedState) - .setEnableRecovery(true) - .setEnablePerChannelRecovery(true) - .setPrimaryProbingFunction( - channel -> { - channel2ProbeCalls.incrementAndGet(); - return "OK"; - }) - .setMinPrimaryProbeSuccessCount(1) - .setMinPrimaryProbeSuccessDuration(Duration.ZERO) - .build(); + // Initialize executor via options + sharedState.getOrCreateExecutorService(options); - ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(Runnable.class); + // Shutdown the shared state (external executor is NOT shut down since it's not owned) + sharedState.shutdown(); + verify(externalExec, never()).shutdown(); - GcpFallbackChannel channel1 = - new GcpFallbackChannel(options1, mockPrimaryBuilder, mockFallbackBuilder, mockExec1); - GcpFallbackChannel channel2 = - new GcpFallbackChannel(options2, mockPrimaryBuilder, mockFallbackBuilder, mockExec2); - - try { - verify(mockExec1, atLeast(2)) - .scheduleAtFixedRate( - taskCaptor.capture(), - eq(options1.getPrimaryProbingInterval().toMillis()), - eq(options1.getPrimaryProbingInterval().toMillis()), - eq(MILLISECONDS)); - Runnable probeTask1 = taskCaptor.getAllValues().get(0); - Runnable probeTask2 = taskCaptor.getAllValues().get(1); - - assertEquals(0, sharedState.getGeneration()); - - // 1. Shared stats trip -> generation counter is bumped with directive "prefer fallback" - sharedState.triggerFallback(); - assertEquals(1, sharedState.getGeneration()); - assertTrue(sharedState.getTargetFallbackMode()); - - // 2. Channel 2 is idle (no calls yet). Its local generation is still 0. - assertEquals(0, channel2.getLocalGeneration().get()); - assertFalse(channel2.getLocalInFallbackMode().get()); - - // Idle Channel 2's background probe runs: because switching is lazy, it has not switched yet - // and must NOT probe or clear the pool fallback state. - probeTask2.run(); - assertEquals(0, channel2ProbeCalls.get()); - assertEquals(1, sharedState.getGeneration()); - assertTrue(sharedState.getTargetFallbackMode()); - - // 3. Channel 1 makes a call -> checks its local generation against pool generation (0 < 1) - // and lazily switches itself to fallback. - assertTrue(channel1.isInFallbackMode()); - assertEquals(1, channel1.getLocalGeneration().get()); - - // 4. Channel 1 probes primary and recovers (or gets a false-positive probe success). - probeTask1.run(); - assertFalse("Channel 1 should recover locally", channel1.isInFallbackMode()); - - // Pool generation and prefer-fallback directive remain intact for generation 1! - assertEquals(1, sharedState.getGeneration()); - assertTrue(sharedState.getTargetFallbackMode()); - - // 5. Idle Channel 2 now makes its next call AFTER Channel 1 already recovered. - // Because its local generation (0) is behind the pool generation (1), it still switches - // itself to fallback on its next call! - assertTrue( - "Idle Channel 2 must switch to fallback on next call despite Channel 1 recovery", - channel2.isInFallbackMode()); - assertEquals(1, channel2.getLocalGeneration().get()); - } finally { - channel1.shutdownNow(); - channel2.shutdownNow(); - sharedState.shutdown(); - } + // Subsequent task scheduling or periodic evaluation on the shut-down state must be rejected + assertNull(sharedState.scheduleTask(() -> {}, 1, 1, TimeUnit.SECONDS)); + sharedState.startPeriodicEvaluation(options); + verify(externalExec, never()) + .scheduleAtFixedRate(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class)); } } From 2cde3416d83146423628cb5178277fd35e9ca117 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Thu, 17 Sep 2026 15:02:36 +0000 Subject: [PATCH 13/15] formatting --- .../com/google/cloud/grpc/fallback/GcpFallbackChannel.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 984cf5edfeb5..7ca9e0c5b92b 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -273,8 +273,7 @@ private void probePrimary() { && !options.getMinPrimaryProbeSuccessDuration().isZero() && !options.getMinPrimaryProbeSuccessDuration().isNegative()) { long elapsedNanos = nowNanos - firstSuccessNanos; - durationSatisfied = - elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); + durationSatisfied = elapsedNanos >= options.getMinPrimaryProbeSuccessDuration().toNanos(); } if (primaryProbeSuccessCount >= options.getMinPrimaryProbeSuccessCount() From 187dea17de806cbfe748f74d0f345737eb8353c5 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Thu, 17 Sep 2026 17:57:13 +0000 Subject: [PATCH 14/15] address comments --- .../fallback/GcpFallbackChannelOptions.java | 4 ++++ .../cloud/grpc/fallback/GcpFallbackState.java | 20 ++++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java index 077897612257..8232c0782835 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannelOptions.java @@ -256,6 +256,10 @@ public Builder setSharedExecutorService(ScheduledExecutorService sharedExecutorS return this; } + /** + * Sets the shared fallback state across channels in a pool. Channels sharing this state should + * use consistent fallback evaluation options. + */ public Builder setSharedState(GcpFallbackState sharedState) { this.sharedState = sharedState; return this; diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java index 66e6c5b4b19e..ed87db98914a 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackState.java @@ -139,7 +139,8 @@ synchronized ScheduledFuture scheduleTask( /** * Starts the periodic error rate evaluation loop exactly once across all channels sharing this - * state. + * state. Channels sharing this state should use consistent evaluation options, as the first + * channel to start evaluation configures the shared loop. * * @param options the fallback channel configuration options. */ @@ -162,12 +163,17 @@ synchronized void startPeriodicEvaluation(GcpFallbackChannelOptions options) { ? options.getGcpOpenTelemetry() : GcpFallbackOpenTelemetry.newBuilder().build(); - scheduledEvaluationFuture = - executor.scheduleAtFixedRate( - () -> checkErrorRates(options, openTelemetry), - options.getPeriod().toMillis(), - options.getPeriod().toMillis(), - TimeUnit.MILLISECONDS); + try { + scheduledEvaluationFuture = + executor.scheduleAtFixedRate( + () -> checkErrorRates(options, openTelemetry), + options.getPeriod().toMillis(), + options.getPeriod().toMillis(), + TimeUnit.MILLISECONDS); + } catch (RuntimeException e) { + evaluationStarted.set(false); + throw e; + } } } From 7f20786910b533a831c2f3d477dc8adefd0d7a81 Mon Sep 17 00:00:00 2001 From: kinsaurralde Date: Thu, 17 Sep 2026 18:23:38 +0000 Subject: [PATCH 15/15] fixes --- .../com/google/cloud/grpc/fallback/GcpFallbackChannel.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java index 7ca9e0c5b92b..da84f3cdd741 100644 --- a/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java +++ b/grpc-gcp-java/src/main/java/com/google/cloud/grpc/fallback/GcpFallbackChannel.java @@ -184,10 +184,9 @@ public GcpFallbackChannel( private void resetProbeStatsIfGenerationChanged() { long currentGen = fallbackState.getGeneration(); - if (localProbeGeneration.get() != currentGen) { + if (localProbeGeneration.getAndSet(currentGen) != currentGen) { localProbeSuccesses.set(0); localFirstPrimaryProbeSuccessNanos.set(0); - localProbeGeneration.set(currentGen); } }