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));
+  }
 }

Reply via email to