Merge pull request #54 from snehasishroy/handle-backwards-time-drift

Avoid duplicate ID in case of backwards time drift
appform-io · Jan 11, 2025 · 0411e60 · 0411e60
2 parents 88c7938 + 5bf8868
commit 0411e60
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 16 deletions.
diff --git a/...iscovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/CollisionChecker.java b/...iscovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/CollisionChecker.java
@@ -16,21 +16,20 @@
 
 package io.appform.ranger.discovery.bundle.id;
 
-import lombok.NonNull;
-import lombok.extern.slf4j.Slf4j;
-
 import java.util.BitSet;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
+import lombok.NonNull;
+import lombok.extern.slf4j.Slf4j;
 
 /**
  * Checks collisions between ids in given period
  */
 @Slf4j
 public class CollisionChecker {
     private final BitSet bitSet = new BitSet(1000);
-    private long currentInstant = 0;
+    private long lastResolvedTime = 0;
 
     private final Lock dataLock = new ReentrantLock();
 
@@ -44,30 +43,34 @@ public CollisionChecker(@NonNull TimeUnit resolution) {
         this.resolution = resolution;
     }
 
-    public boolean check(long timeInMillis, int location) {
+    public boolean check(long curTimeMs,
+                         int location) {
         dataLock.lock();
         try {
-            long resolvedTime = resolution.convert(timeInMillis, TimeUnit.MILLISECONDS);;
-            if (currentInstant != resolvedTime) {
-                currentInstant = resolvedTime;
+            long currentTime = resolution.convert(curTimeMs, TimeUnit.MILLISECONDS);
+            if (currentTime < lastResolvedTime) {
+                log.error("Clock has moved backwards. Rejecting requests until current time {} reaches {}", currentTime,
+                        lastResolvedTime);
+                return false;
+            }
+            if (lastResolvedTime != currentTime) {
+                lastResolvedTime = currentTime;
                 bitSet.clear();
             }
-
             if (bitSet.get(location)) {
                 return false;
             }
             bitSet.set(location);
             return true;
-        }
-        finally {
+        } finally {
             dataLock.unlock();
         }
     }
 
     public void free(long time, int location) {
         dataLock.lock();
         try {
-            if (currentInstant != time) {
+            if (lastResolvedTime != time) {
                 return;
             }
             bitSet.clear(location);

diff --git a/ranger-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/IdGenerator.java b/ranger-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/IdGenerator.java
@@ -306,12 +306,12 @@ public static Optional<Id> generate(final IdGenerationRequest request) {
 
     private static IdInfo random(CollisionChecker collisionChecker) {
         int randomGen;
-        long time;
+        long curTimeMs;
         do {
-            time = System.currentTimeMillis();
+            curTimeMs = System.currentTimeMillis();
             randomGen = SECURE_RANDOM.nextInt(Constants.MAX_ID_PER_MS);
-        } while (!collisionChecker.check(time, randomGen));
-        return new IdInfo(randomGen, time);
+        } while (!collisionChecker.check(curTimeMs, randomGen));
+        return new IdInfo(randomGen, curTimeMs);
     }
 
     private static IdValidationState validateId(List<IdValidationConstraint> inConstraints, Id id, boolean skipGlobal) {

diff --git a/...very-bundle/src/test/java/io/appform/ranger/discovery/bundle/id/CollisionCheckerTest.java b/...very-bundle/src/test/java/io/appform/ranger/discovery/bundle/id/CollisionCheckerTest.java
@@ -37,4 +37,13 @@ void testCheck() {
             Assertions.assertFalse(collisionChecker.check(101, i));
         });
     }
+
+    @Test
+    void testCheckGoingBackInTime() {
+        CollisionChecker collisionChecker = new CollisionChecker();
+        Assertions.assertTrue(collisionChecker.check(100, 1));
+        Assertions.assertFalse(collisionChecker.check(90, 1));
+        Assertions.assertFalse(collisionChecker.check(100, 1));
+        Assertions.assertTrue(collisionChecker.check(101, 1));
+    }
 }