core: Vastly separate types of clock in FakeClock

There was an attempt to use different epochs for the wall clock and the
monotonic clock. However, 123456789 is actually less than a second.
We want the gap between clocks to be at least a day. This issue was
discovered in #8968.

This separation found a bug in an RLS test where it was mixing epochs.
However, it was only a problem in the test. The code under test is
wrongly using wall clock for calculation durations, but that seems to be
a wide-spread problem and will need to be handled separately.
This commit is contained in:
Eric Anderson 2022-04-04 11:24:55 -07:00 committed by GitHub
parent 79f2562306
commit 004ee10a73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 4 deletions

View File

@ -357,7 +357,7 @@ public final class FakeClock {
public long currentTimeMillis() { public long currentTimeMillis() {
// Normally millis and nanos are of different epochs. Add an offset to simulate that. // Normally millis and nanos are of different epochs. Add an offset to simulate that.
return TimeUnit.NANOSECONDS.toMillis(currentTimeNanos + 123456789L); return TimeUnit.NANOSECONDS.toMillis(currentTimeNanos + 1234567890123456789L);
} }
/** /**

View File

@ -41,6 +41,8 @@ public class AdaptiveThrottlerTest {
@Test @Test
public void shouldThrottle() { public void shouldThrottle() {
long startTime = fakeClock.currentTimeMillis();
// initial states // initial states
assertThat(throttler.requestStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(0L); assertThat(throttler.requestStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(0L);
assertThat(throttler.throttledStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(0L); assertThat(throttler.throttledStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(0L);
@ -71,8 +73,8 @@ public class AdaptiveThrottlerTest {
.isWithin(TOLERANCE) .isWithin(TOLERANCE)
.of(1.0f / 3.0f); .of(1.0f / 3.0f);
// Skip half a second (half the duration). // Skip to half second mark from the beginning (half the duration).
fakeClock.forwardTime(500 - fakeClock.currentTimeMillis(), TimeUnit.MILLISECONDS); fakeClock.forwardTime(500 - (fakeClock.currentTimeMillis() - startTime), TimeUnit.MILLISECONDS);
// Request 3, throttled by backend // Request 3, throttled by backend
assertThat(throttler.shouldThrottle(0.4f)).isFalse(); assertThat(throttler.shouldThrottle(0.4f)).isFalse();
@ -96,7 +98,8 @@ public class AdaptiveThrottlerTest {
.of(3.0f / 5.0f); .of(3.0f / 5.0f);
// Skip to the point where only requests 3 and 4 are visible. // Skip to the point where only requests 3 and 4 are visible.
fakeClock.forwardTime(1250 - fakeClock.currentTimeMillis(), TimeUnit.MILLISECONDS); fakeClock.forwardTime(
1250 - (fakeClock.currentTimeMillis() - startTime), TimeUnit.MILLISECONDS);
assertThat(throttler.requestStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(2L); assertThat(throttler.requestStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(2L);
assertThat(throttler.throttledStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(2L); assertThat(throttler.throttledStat.get(fakeTimeProvider.currentTimeNanos())).isEqualTo(2L);