From ec44974abac75620e43d0b784de95c60021d22ac Mon Sep 17 00:00:00 2001 From: vimanikag Date: Thu, 29 May 2025 06:06:05 +0000 Subject: [PATCH 1/7] 11622 : OutlierDetection should use Ticker, not TimeProvider --- .../io/grpc/util/OutlierDetectionLoadBalancer.java | 13 +++++++------ .../util/OutlierDetectionLoadBalancerProvider.java | 4 ++-- .../grpc/util/OutlierDetectionLoadBalancerTest.java | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 59b2d38ecd9..b530f6c236a 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -22,6 +22,7 @@ import static java.util.concurrent.TimeUnit.NANOSECONDS; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Ticker; import com.google.common.collect.ForwardingMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -39,7 +40,6 @@ import io.grpc.Status; import io.grpc.SynchronizationContext; import io.grpc.SynchronizationContext.ScheduledHandle; -import io.grpc.internal.TimeProvider; import java.net.SocketAddress; import java.util.ArrayList; import java.util.Collection; @@ -82,7 +82,8 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer { private final SynchronizationContext syncContext; private final Helper childHelper; private final GracefulSwitchLoadBalancer switchLb; - private TimeProvider timeProvider; + //private TimeProvider timeProvider; + private Ticker ticker; private final ScheduledExecutorService timeService; private ScheduledHandle detectionTimerHandle; private Long detectionTimerStartNanos; @@ -95,14 +96,14 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer { /** * Creates a new instance of {@link OutlierDetectionLoadBalancer}. */ - public OutlierDetectionLoadBalancer(Helper helper, TimeProvider timeProvider) { + public OutlierDetectionLoadBalancer(Helper helper, Ticker ticker) { logger = helper.getChannelLogger(); childHelper = new ChildHelper(checkNotNull(helper, "helper")); switchLb = new GracefulSwitchLoadBalancer(childHelper); endpointTrackerMap = new EndpointTrackerMap(); this.syncContext = checkNotNull(helper.getSynchronizationContext(), "syncContext"); this.timeService = checkNotNull(helper.getScheduledExecutorService(), "timeService"); - this.timeProvider = timeProvider; + this.ticker = ticker; logger.log(ChannelLogLevel.DEBUG, "OutlierDetection lb created."); } @@ -151,7 +152,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // If a timer has started earlier we cancel it and use the difference between the start // time and now as the interval. initialDelayNanos = Math.max(0L, - config.intervalNanos - (timeProvider.currentTimeNanos() - detectionTimerStartNanos)); + config.intervalNanos - (ticker.read() - detectionTimerStartNanos)); } // If a timer has been previously created we need to cancel it and reset all the call counters @@ -201,7 +202,7 @@ class DetectionTimer implements Runnable { @Override public void run() { - detectionTimerStartNanos = timeProvider.currentTimeNanos(); + detectionTimerStartNanos = ticker.read(); endpointTrackerMap.swapCounters(); diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancerProvider.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancerProvider.java index b35e1144581..c76d68a03de 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancerProvider.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancerProvider.java @@ -16,6 +16,7 @@ package io.grpc.util; +import com.google.common.base.Ticker; import io.grpc.Internal; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.Helper; @@ -23,7 +24,6 @@ import io.grpc.NameResolver.ConfigOrError; import io.grpc.Status; import io.grpc.internal.JsonUtil; -import io.grpc.internal.TimeProvider; import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig; import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig.FailurePercentageEjection; import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig.SuccessRateEjection; @@ -34,7 +34,7 @@ public final class OutlierDetectionLoadBalancerProvider extends LoadBalancerProv @Override public LoadBalancer newLoadBalancer(Helper helper) { - return new OutlierDetectionLoadBalancer(helper, TimeProvider.SYSTEM_TIME_PROVIDER); + return new OutlierDetectionLoadBalancer(helper, Ticker.systemTicker()); } @Override diff --git a/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java b/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java index d81740e116a..c4eb4c7bae5 100644 --- a/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java @@ -227,7 +227,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { when(mockStreamTracerFactory.newClientStreamTracer(any(), any())).thenReturn(mockStreamTracer); - loadBalancer = new OutlierDetectionLoadBalancer(mockHelper, fakeClock.getTimeProvider()); + loadBalancer = new OutlierDetectionLoadBalancer(mockHelper, fakeClock.getTicker()); } @Test From 8f513b7041611e93fee8a8603c4d2cc2ad9d23d1 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Thu, 29 May 2025 06:09:04 +0000 Subject: [PATCH 2/7] 11622 : OutlierDetection should use Ticker, not TimeProvider --- .../src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index b530f6c236a..c72a5a7e408 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -82,7 +82,6 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer { private final SynchronizationContext syncContext; private final Helper childHelper; private final GracefulSwitchLoadBalancer switchLb; - //private TimeProvider timeProvider; private Ticker ticker; private final ScheduledExecutorService timeService; private ScheduledHandle detectionTimerHandle; From debbd7ae9e1b5bf8ca3488215f605937a6fe99f1 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 3 Jun 2025 10:07:24 +0000 Subject: [PATCH 3/7] 11243 : RLS cleanups --- .../main/java/io/grpc/util/OutlierDetectionLoadBalancer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index c72a5a7e408..5c8cd99aff7 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -148,10 +148,12 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // On the first go we use the configured interval. initialDelayNanos = config.intervalNanos; } else { + Long currentTickerTimeNanos = ticker.read(); + Long elepsedTimeNanos = currentTickerTimeNanos - detectionTimerStartNanos; // If a timer has started earlier we cancel it and use the difference between the start // time and now as the interval. initialDelayNanos = Math.max(0L, - config.intervalNanos - (ticker.read() - detectionTimerStartNanos)); + config.intervalNanos - elepsedTimeNanos); } // If a timer has been previously created we need to cancel it and reset all the call counters From 3735a6c5d54b0b02b19812143bcadcad8d781992 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 3 Jun 2025 12:34:53 +0000 Subject: [PATCH 4/7] 11622 : OutlierDetection should use Ticker, not TimeProvider --- .../src/main/java/io/grpc/binder/internal/PingTracker.java | 3 ++- netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java | 3 ++- .../java/io/grpc/util/OutlierDetectionLoadBalancer.java | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/PingTracker.java b/binder/src/main/java/io/grpc/binder/internal/PingTracker.java index 5a4300443ba..048b7602fd8 100644 --- a/binder/src/main/java/io/grpc/binder/internal/PingTracker.java +++ b/binder/src/main/java/io/grpc/binder/internal/PingTracker.java @@ -104,9 +104,10 @@ private synchronized void fail(Status status) { } private synchronized void success() { + long currentTickerTimeNanos = ticker.read(); if (!done) { done = true; - executor.execute(() -> callback.onSuccess(ticker.read() - startTimeNanos)); + executor.execute(() -> callback.onSuccess(currentTickerTimeNanos - startTimeNanos)); } } } diff --git a/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java b/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java index c4ec5913cde..ddfa5b692ef 100644 --- a/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java +++ b/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java @@ -199,7 +199,8 @@ public void updateWindow() throws Http2Exception { pingReturn++; setPinging(false); - long elapsedTime = (ticker.read() - lastPingTime); + long currentTickerTimeNanos = ticker.read(); + long elapsedTime = (currentTickerTimeNanos - lastPingTime); if (elapsedTime == 0) { elapsedTime = 1; } diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 5c8cd99aff7..667ea56fa4c 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -148,12 +148,12 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // On the first go we use the configured interval. initialDelayNanos = config.intervalNanos; } else { - Long currentTickerTimeNanos = ticker.read(); - Long elepsedTimeNanos = currentTickerTimeNanos - detectionTimerStartNanos; + long currentTickerTimeNanos = ticker.read(); + long elapsedTimeNanos = currentTickerTimeNanos - detectionTimerStartNanos; // If a timer has started earlier we cancel it and use the difference between the start // time and now as the interval. initialDelayNanos = Math.max(0L, - config.intervalNanos - elepsedTimeNanos); + config.intervalNanos - elapsedTimeNanos); } // If a timer has been previously created we need to cancel it and reset all the call counters From c831f1e86995cbe45b7f1d7986ef1a94e32b03f7 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Wed, 4 Jun 2025 04:32:01 +0000 Subject: [PATCH 5/7] 11622 : OutlierDetection should use Ticker, not TimeProvider --- binder/src/main/java/io/grpc/binder/internal/PingTracker.java | 3 +-- netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java | 2 +- .../main/java/io/grpc/util/OutlierDetectionLoadBalancer.java | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/PingTracker.java b/binder/src/main/java/io/grpc/binder/internal/PingTracker.java index 048b7602fd8..5a4300443ba 100644 --- a/binder/src/main/java/io/grpc/binder/internal/PingTracker.java +++ b/binder/src/main/java/io/grpc/binder/internal/PingTracker.java @@ -104,10 +104,9 @@ private synchronized void fail(Status status) { } private synchronized void success() { - long currentTickerTimeNanos = ticker.read(); if (!done) { done = true; - executor.execute(() -> callback.onSuccess(currentTickerTimeNanos - startTimeNanos)); + executor.execute(() -> callback.onSuccess(ticker.read() - startTimeNanos)); } } } diff --git a/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java b/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java index ddfa5b692ef..5f6201004af 100644 --- a/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java +++ b/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java @@ -200,7 +200,7 @@ public void updateWindow() throws Http2Exception { setPinging(false); long currentTickerTimeNanos = ticker.read(); - long elapsedTime = (currentTickerTimeNanos - lastPingTime); + long elapsedTime = Math.max(0L,(currentTickerTimeNanos - lastPingTime)); if (elapsedTime == 0) { elapsedTime = 1; } diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 667ea56fa4c..5532eebcea4 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -149,7 +149,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { initialDelayNanos = config.intervalNanos; } else { long currentTickerTimeNanos = ticker.read(); - long elapsedTimeNanos = currentTickerTimeNanos - detectionTimerStartNanos; + long elapsedTimeNanos = Math.max(0L,(currentTickerTimeNanos - detectionTimerStartNanos)); // If a timer has started earlier we cancel it and use the difference between the start // time and now as the interval. initialDelayNanos = Math.max(0L, From b8c0e0c339cc7a2b6076ffe054922c23ae7c8345 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Fri, 6 Jun 2025 14:57:36 +0000 Subject: [PATCH 6/7] 11622 : OutlierDetection should use Ticker, not TimeProvider --- netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java | 3 +-- .../main/java/io/grpc/util/OutlierDetectionLoadBalancer.java | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java b/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java index 5f6201004af..c4ec5913cde 100644 --- a/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java +++ b/netty/src/main/java/io/grpc/netty/AbstractNettyHandler.java @@ -199,8 +199,7 @@ public void updateWindow() throws Http2Exception { pingReturn++; setPinging(false); - long currentTickerTimeNanos = ticker.read(); - long elapsedTime = Math.max(0L,(currentTickerTimeNanos - lastPingTime)); + long elapsedTime = (ticker.read() - lastPingTime); if (elapsedTime == 0) { elapsedTime = 1; } diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 5532eebcea4..45a4905820d 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -148,12 +148,10 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // On the first go we use the configured interval. initialDelayNanos = config.intervalNanos; } else { - long currentTickerTimeNanos = ticker.read(); - long elapsedTimeNanos = Math.max(0L,(currentTickerTimeNanos - detectionTimerStartNanos)); // If a timer has started earlier we cancel it and use the difference between the start // time and now as the interval. initialDelayNanos = Math.max(0L, - config.intervalNanos - elapsedTimeNanos); + config.intervalNanos - Math.max(0L,(ticker.read() - detectionTimerStartNanos))); } // If a timer has been previously created we need to cancel it and reset all the call counters From aaf5016b9625a6c70ba5dc11076948fd0f1707e0 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Mon, 9 Jun 2025 15:50:12 +0000 Subject: [PATCH 7/7] 11622 : OutlierDetection should use Ticker, not TimeProvider --- .../main/java/io/grpc/util/OutlierDetectionLoadBalancer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 45a4905820d..c8c0e6dbcbc 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -151,7 +151,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // If a timer has started earlier we cancel it and use the difference between the start // time and now as the interval. initialDelayNanos = Math.max(0L, - config.intervalNanos - Math.max(0L,(ticker.read() - detectionTimerStartNanos))); + config.intervalNanos - (ticker.read() - detectionTimerStartNanos)); } // If a timer has been previously created we need to cancel it and reset all the call counters @@ -638,7 +638,7 @@ public boolean maxEjectionTimeElapsed(long currentTimeNanos) { config.baseEjectionTimeNanos * ejectionTimeMultiplier, maxEjectionDurationSecs); - return currentTimeNanos > maxEjectionTimeNanos; + return currentTimeNanos - maxEjectionTimeNanos > 0; } /** Tracks both successful and failed call counts. */