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 <[email protected]>
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));
+ }
}