This is an automated email from the ASF dual-hosted git repository. jxue pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/helix.git
commit c3afa7f2714c44e73fe8f0210a624f88b382a270 Author: Marcos Rico Peng <[email protected]> AuthorDate: Wed Aug 2 18:46:26 2023 +0200 Lattice Puppy Stress Test Race Condition Fix and Code Cleanup (#2579) --------- Co-authored-by: mapeng <[email protected]> --- .../apache/helix/metaclient/MetaClientTestUtil.java | 2 +- .../zk/TestMultiThreadStressTest/CreatePuppy.java | 3 +-- .../zk/TestMultiThreadStressTest/DeletePuppy.java | 2 +- .../impl/zk/TestMultiThreadStressTest/GetPuppy.java | 1 - .../impl/zk/TestMultiThreadStressTest/SetPuppy.java | 1 - .../TestMultiThreadStressZKClient.java | 6 ++---- .../zk/TestMultiThreadStressTest/UpdatePuppy.java | 1 - .../helix/metaclient/puppy/AbstractPuppy.java | 21 +++++++-------------- 8 files changed, 12 insertions(+), 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 index 2c7543e31..f0a14c4a4 100644 --- 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 @@ import java.util.concurrent.TimeUnit; 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/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/CreatePuppy.java b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/CreatePuppy.java index 3e28df06b..3940f79ab 100644 --- a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/CreatePuppy.java +++ b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/CreatePuppy.java @@ -64,8 +64,7 @@ public class CreatePuppy extends AbstractPuppy { @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/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/DeletePuppy.java b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/DeletePuppy.java index e0e1b7b5c..1aa9d4d72 100644 --- a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/DeletePuppy.java +++ b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/DeletePuppy.java @@ -58,7 +58,7 @@ public class DeletePuppy extends AbstractPuppy { @Override protected void cleanup() { - _metaclient.recursiveDelete(_parentPath); + // Do nothing } private boolean shouldIntroduceError() { diff --git a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java index fe24b2bd3..4af1c4df3 100644 --- a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java +++ b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java @@ -59,7 +59,6 @@ public class GetPuppy extends AbstractPuppy { @Override protected void cleanup() { - _metaclient.recursiveDelete(_parentPath); } private boolean shouldIntroduceError() { diff --git a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/SetPuppy.java b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/SetPuppy.java index c0de4ece7..3385b8673 100644 --- a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/SetPuppy.java +++ b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/SetPuppy.java @@ -61,7 +61,6 @@ public class SetPuppy extends AbstractPuppy { @Override protected void cleanup() { - _metaclient.recursiveDelete(_parentPath); } private boolean shouldIntroduceError() { diff --git a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java index 5ee026bc1..6a01fffa7 100644 --- a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java +++ b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java @@ -52,7 +52,7 @@ public class TestMultiThreadStressZKClient extends ZkMetaClientTestBase { 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 class TestMultiThreadStressZKClient extends ZkMetaClientTestBase { 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 class TestMultiThreadStressZKClient extends ZkMetaClientTestBase { 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/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/UpdatePuppy.java b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/UpdatePuppy.java index 721d507eb..5af055f74 100644 --- a/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/UpdatePuppy.java +++ b/meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/UpdatePuppy.java @@ -64,7 +64,6 @@ public class UpdatePuppy extends AbstractPuppy { @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 index bfbbb915d..9ce21fc15 100644 --- 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 abstract class AbstractPuppy implements Runnable { @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() {
