Skip to content

Commit 1c43098

Browse files
authored
util: OutlierDetection should use Ticker, not TimeProvider (#12110)
TimeProvider provides wall time. That can move forward and backward as time is adjusted. OutlierDetection is measuring durations, so it should use a monotonic clock. Fixes #11622
1 parent d5b4fb5 commit 1c43098

File tree

3 files changed

+10
-10
lines changed

3 files changed

+10
-10
lines changed

util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static java.util.concurrent.TimeUnit.NANOSECONDS;
2323

2424
import com.google.common.annotations.VisibleForTesting;
25+
import com.google.common.base.Ticker;
2526
import com.google.common.collect.ForwardingMap;
2627
import com.google.common.collect.ImmutableList;
2728
import com.google.common.collect.ImmutableSet;
@@ -39,7 +40,6 @@
3940
import io.grpc.Status;
4041
import io.grpc.SynchronizationContext;
4142
import io.grpc.SynchronizationContext.ScheduledHandle;
42-
import io.grpc.internal.TimeProvider;
4343
import java.net.SocketAddress;
4444
import java.util.ArrayList;
4545
import java.util.Collection;
@@ -82,7 +82,7 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer {
8282
private final SynchronizationContext syncContext;
8383
private final Helper childHelper;
8484
private final GracefulSwitchLoadBalancer switchLb;
85-
private TimeProvider timeProvider;
85+
private Ticker ticker;
8686
private final ScheduledExecutorService timeService;
8787
private ScheduledHandle detectionTimerHandle;
8888
private Long detectionTimerStartNanos;
@@ -95,14 +95,14 @@ public final class OutlierDetectionLoadBalancer extends LoadBalancer {
9595
/**
9696
* Creates a new instance of {@link OutlierDetectionLoadBalancer}.
9797
*/
98-
public OutlierDetectionLoadBalancer(Helper helper, TimeProvider timeProvider) {
98+
public OutlierDetectionLoadBalancer(Helper helper, Ticker ticker) {
9999
logger = helper.getChannelLogger();
100100
childHelper = new ChildHelper(checkNotNull(helper, "helper"));
101101
switchLb = new GracefulSwitchLoadBalancer(childHelper);
102102
endpointTrackerMap = new EndpointTrackerMap();
103103
this.syncContext = checkNotNull(helper.getSynchronizationContext(), "syncContext");
104104
this.timeService = checkNotNull(helper.getScheduledExecutorService(), "timeService");
105-
this.timeProvider = timeProvider;
105+
this.ticker = ticker;
106106
logger.log(ChannelLogLevel.DEBUG, "OutlierDetection lb created.");
107107
}
108108

@@ -151,7 +151,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
151151
// If a timer has started earlier we cancel it and use the difference between the start
152152
// time and now as the interval.
153153
initialDelayNanos = Math.max(0L,
154-
config.intervalNanos - (timeProvider.currentTimeNanos() - detectionTimerStartNanos));
154+
config.intervalNanos - (ticker.read() - detectionTimerStartNanos));
155155
}
156156

157157
// If a timer has been previously created we need to cancel it and reset all the call counters
@@ -201,7 +201,7 @@ final class DetectionTimer implements Runnable {
201201

202202
@Override
203203
public void run() {
204-
detectionTimerStartNanos = timeProvider.currentTimeNanos();
204+
detectionTimerStartNanos = ticker.read();
205205

206206
endpointTrackerMap.swapCounters();
207207

@@ -638,7 +638,7 @@ public boolean maxEjectionTimeElapsed(long currentTimeNanos) {
638638
config.baseEjectionTimeNanos * ejectionTimeMultiplier,
639639
maxEjectionDurationSecs);
640640

641-
return currentTimeNanos > maxEjectionTimeNanos;
641+
return currentTimeNanos - maxEjectionTimeNanos > 0;
642642
}
643643

644644
/** Tracks both successful and failed call counts. */

util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancerProvider.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616

1717
package io.grpc.util;
1818

19+
import com.google.common.base.Ticker;
1920
import io.grpc.Internal;
2021
import io.grpc.LoadBalancer;
2122
import io.grpc.LoadBalancer.Helper;
2223
import io.grpc.LoadBalancerProvider;
2324
import io.grpc.NameResolver.ConfigOrError;
2425
import io.grpc.Status;
2526
import io.grpc.internal.JsonUtil;
26-
import io.grpc.internal.TimeProvider;
2727
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
2828
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig.FailurePercentageEjection;
2929
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig.SuccessRateEjection;
@@ -34,7 +34,7 @@ public final class OutlierDetectionLoadBalancerProvider extends LoadBalancerProv
3434

3535
@Override
3636
public LoadBalancer newLoadBalancer(Helper helper) {
37-
return new OutlierDetectionLoadBalancer(helper, TimeProvider.SYSTEM_TIME_PROVIDER);
37+
return new OutlierDetectionLoadBalancer(helper, Ticker.systemTicker());
3838
}
3939

4040
@Override

util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
227227
when(mockStreamTracerFactory.newClientStreamTracer(any(),
228228
any())).thenReturn(mockStreamTracer);
229229

230-
loadBalancer = new OutlierDetectionLoadBalancer(mockHelper, fakeClock.getTimeProvider());
230+
loadBalancer = new OutlierDetectionLoadBalancer(mockHelper, fakeClock.getTicker());
231231
}
232232

233233
@Test

0 commit comments

Comments
 (0)