This is an automated email from the ASF dual-hosted git repository. kturner pushed a commit to branch elasticity in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push: new 7d85cefb0d adds startedAfter method to Timer (#4824) 7d85cefb0d is described below commit 7d85cefb0d50fed67a95aece7d53f246635e61c8 Author: Keith Turner <ktur...@apache.org> AuthorDate: Fri Aug 23 12:45:17 2024 -0700 adds startedAfter method to Timer (#4824) This [comment][1] identified a flaky unit test. The test was likely flakey because it compared elapsed times on two timers. Each call to elapsed time would call System.nanoTime. Depending on how much nano time drifted between the two calls different results could be obtained making the code non-deteministic. Added a method to check if one timer started after another and used it for this check making the code deterministic. Its also much easier to understand the intent of the code with this change. [1]:https://github.com/apache/accumulo/pull/4821#issuecomment-2307244079 --- .../core/clientImpl/ClientTabletCacheImpl.java | 7 +++---- .../java/org/apache/accumulo/core/util/Timer.java | 8 ++++++++ .../org/apache/accumulo/core/util/TimerTest.java | 22 +++++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java index 7aa10260cb..78def35df0 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java @@ -19,7 +19,6 @@ package org.apache.accumulo.core.clientImpl; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.NANOSECONDS; import java.time.Duration; import java.util.ArrayList; @@ -889,10 +888,10 @@ public class ClientTabletCacheImpl extends ClientTabletCache { } if (tl == null || (locationNeed == LocationNeed.REQUIRED && tl.getTserverLocation().isEmpty() - && tl.getCreationTimer().elapsed(NANOSECONDS) > cacheCutoffTimer.elapsed(NANOSECONDS))) { + && cacheCutoffTimer.startedAfter(tl.getCreationTimer()))) { - // not in cache OR the cached entry was created before the cut off time, so obtain info from - // metadata table + // not in cache OR the cutoff timer was started after when the cached entry timer was started, + // so obtain info from metadata table if (lock) { wLock.lock(); try { diff --git a/core/src/main/java/org/apache/accumulo/core/util/Timer.java b/core/src/main/java/org/apache/accumulo/core/util/Timer.java index cf06789993..3884a234bd 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/Timer.java +++ b/core/src/main/java/org/apache/accumulo/core/util/Timer.java @@ -88,4 +88,12 @@ public final class Timer { return unit.convert(getElapsedNanos(), TimeUnit.NANOSECONDS); } + /** + * @return true if this timer was started/reset after the other timer was started/reset, false + * otherwise + */ + public boolean startedAfter(Timer otherTimer) { + return (startNanos - otherTimer.startNanos) > 0; + } + } diff --git a/core/src/test/java/org/apache/accumulo/core/util/TimerTest.java b/core/src/test/java/org/apache/accumulo/core/util/TimerTest.java index 67b40d07e3..23432de903 100644 --- a/core/src/test/java/org/apache/accumulo/core/util/TimerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/util/TimerTest.java @@ -93,7 +93,27 @@ public class TimerTest { assertEquals(0, elapsedSeconds, "Elapsed time in seconds should be 0 for 50 milliseconds of sleep."); } - } + @Test + public void testStartedAfter() throws Exception { + var timer1 = Timer.startNew(); + Thread.sleep(3); + var timer2 = Timer.startNew(); + + assertTrue(timer2.startedAfter(timer1)); + assertFalse(timer1.startedAfter(timer2)); + + Thread.sleep(3); + timer1.restart(); + + assertTrue(timer1.startedAfter(timer2)); + assertFalse(timer2.startedAfter(timer1)); + + Thread.sleep(3); + timer2.restart(); + + assertTrue(timer2.startedAfter(timer1)); + assertFalse(timer1.startedAfter(timer2)); + } }