Lattice Puppy Stress Test Race Condition Fix and Code Cleanup (#2579)

--------- Co-authored-by: mapeng <[email protected]>
apache · Aug 2, 2023 · a9c8332 · a9c8332
1 parent 0d4e2f5
commit a9c8332
Show file tree

Hide file tree

Showing 8 changed files with 12 additions and 25 deletions.
diff --git a/meta-client/src/test/java/org/apache/helix/metaclient/MetaClientTestUtil.java b/meta-client/src/test/java/org/apache/helix/metaclient/MetaClientTestUtil.java
@@ -4,7 +4,7 @@
 
 
 public class MetaClientTestUtil {
-  public static final long WAIT_DURATION =  TimeUnit.MINUTES.toMicros(1);
+  public static final long WAIT_DURATION =  TimeUnit.MINUTES.toMillis(1);
   public interface Verifier {
     boolean verify()
         throws Exception;

diff --git a/.../test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/CreatePuppy.java b/.../test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/CreatePuppy.java
@@ -64,8 +64,7 @@ protected void bark() {
 
   @Override
   protected void cleanup() {
-    // Implement the recovery logic by deleting the created documents
-    _metaclient.recursiveDelete(_parentPath);
+    // Cleanup logic in test case
   }
 
   private boolean shouldIntroduceError() {

diff --git a/.../test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/DeletePuppy.java b/.../test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/DeletePuppy.java
@@ -58,7 +58,7 @@ protected void bark() {
 
   @Override
   protected void cleanup() {
-    _metaclient.recursiveDelete(_parentPath);
+    // Do nothing
   }
 
   private boolean shouldIntroduceError() {

diff --git a/...src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java b/...src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java
@@ -59,7 +59,6 @@ protected void bark() {
 
   @Override
   protected void cleanup() {
-    _metaclient.recursiveDelete(_parentPath);
   }
 
   private boolean shouldIntroduceError() {

diff --git a/...src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/SetPuppy.java b/...src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/SetPuppy.java
@@ -61,7 +61,6 @@ protected void bark() {
 
   @Override
   protected void cleanup() {
-    _metaclient.recursiveDelete(_parentPath);
   }
 
   private boolean shouldIntroduceError() {

diff --git a/...che/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java b/...che/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java
@@ -52,7 +52,7 @@ private void setUp() {
   public void testCreatePuppy() {
     _zkMetaClient.create(zkParentKey, "test");
 
-    PuppySpec puppySpec = new org.apache.helix.metaclient.puppy.PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
     CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
     CreatePuppy createPuppy2 = new CreatePuppy(_zkMetaClient, puppySpec);
     CreatePuppy createPuppy3 = new CreatePuppy(_zkMetaClient, puppySpec);
@@ -189,7 +189,7 @@ public void testBasicParentListenerPuppy() {
     AtomicInteger globalChildChangeCounter = new AtomicInteger();
     ChildChangeListener childChangeListener = (changedPath, changeType) -> {
       globalChildChangeCounter.addAndGet(1);
-      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + globalChildChangeCounter.get());
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + ". Number of total changes: " + globalChildChangeCounter.get());
     };
 
     _zkMetaClient.subscribeChildChanges(zkParentKey, childChangeListener, false);
@@ -219,8 +219,6 @@ public void testComplexParentListenerPuppy() {
       globalChildChangeCounter.addAndGet(1);
       System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + globalChildChangeCounter.get());
     };
-
-
     _zkMetaClient.subscribeChildChanges(zkParentKey, childChangeListener, false);
 
     PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);

diff --git a/.../test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/UpdatePuppy.java b/.../test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/UpdatePuppy.java
@@ -64,7 +64,6 @@ protected void bark() throws Exception {
 
   @Override
   protected void cleanup() {
-    _metaclient.recursiveDelete(_parentPath);
   }
 
   private boolean shouldIntroduceError() {

diff --git a/meta-client/src/test/java/org/apache/helix/metaclient/puppy/AbstractPuppy.java b/meta-client/src/test/java/org/apache/helix/metaclient/puppy/AbstractPuppy.java
@@ -55,30 +55,23 @@ public AbstractPuppy(MetaClientInterface<String> metaclient, PuppySpec puppySpec
   @Override
   public void run() {
     try {
-      while (true) {
+      while (!Thread.currentThread().isInterrupted()) {
         try {
+          Thread.sleep(getPuppySpec().getExecDelay().getNextDelay());
           bark();
+        } catch (InterruptedException e) {
+          break;
         } catch (Exception e) {
           incrementUnhandledErrorCounter();
           e.printStackTrace();
         }
-
-        if (getPuppySpec().getMode() == PuppyMode.ONE_OFF) {
-          cleanup();
-          break;
-        } else {
-          try {
-            Thread.sleep(getPuppySpec().getExecDelay().getNextDelay());
-          } catch (InterruptedException e) {
-            cleanup();
-            break;
-            // Handle interruption if necessary
-          }
-        }
       }
     } catch (Exception e) {
       e.printStackTrace();
     }
+    finally {
+      cleanup();
+    }
   }
 
   public PuppySpec getPuppySpec() {