You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by "Marcosrico (via GitHub)" <gi...@apache.org> on 2023/06/28 15:33:41 UTC

[GitHub] [helix] Marcosrico opened a new pull request, #2548: MultiThreading Puppy Stress Testing

Marcosrico opened a new pull request, #2548:
URL: https://github.com/apache/helix/pull/2548

   ### Issues
   
   - [ ] My PR addresses the following Helix issues and references them in the PR description:
   
   (#200 - Link your issue number here: You can write "Fixes #XXX". Please use the proper keyword so that the issue gets closed automatically. See https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue
   Any of the following keywords can be used: close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved)
   
   ### Description
   
   - [ ] Here are some details about my PR, including screenshots of any UI changes:
   
   (Write a concise description including what, why, how)
   
   ### Tests
   
   - [ ] The following tests are written for this issue:
   
   (List the names of added unit/integration tests)
   
   - The following is the result of the "mvn test" command on the appropriate module:
   
   (If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)
   
   ### Changes that Break Backward Compatibility (Optional)
   
   - My PR contains changes that break backward compatibility or previous assumptions for certain methods or API. They include:
   
   (Consider including all behavior changes for public methods or API. Also include these changes in merge description so that other developers are aware of these changes. This allows them to make relevant code changes in feature branches accounting for the new method/API behavior.)
   
   ### Documentation (Optional)
   
   - In case of new functionality, my PR adds documentation in the following wiki page:
   
   (Link the GitHub wiki you added)
   
   ### Commits
   
   - My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
     1. Subject is separated from body by a blank line
     1. Subject is limited to 50 characters (not including Jira issue reference)
     1. Subject does not end with a period
     1. Subject uses the imperative mood ("add", not "adding")
     1. Body wraps at 72 characters
     1. Body explains "what" and "why", not "how"
   
   ### Code Quality
   
   - My diff has been formatted using helix-style.xml 
   (helix-style-intellij.xml if IntelliJ IDE is used)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] Marcosrico commented on a diff in pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2548:
URL: https://github.com/apache/helix/pull/2548#discussion_r1268357824


##########
meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java:
##########
@@ -0,0 +1,368 @@
+package org.apache.helix.metaclient.impl.zk.TestMultiThreadStressTest;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.helix.metaclient.api.ChildChangeListener;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClient;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClientTestBase;
+import org.apache.helix.metaclient.puppy.ExecDelay;
+import org.apache.helix.metaclient.puppy.PuppyManager;
+import org.apache.helix.metaclient.puppy.PuppyMode;
+import org.apache.helix.metaclient.puppy.PuppySpec;
+import org.apache.helix.metaclient.puppy.AbstractPuppy;
+import org.testng.Assert;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class TestMultiThreadStressZKClient extends ZkMetaClientTestBase {
+
+  private ZkMetaClient<String> _zkMetaClient;
+  private final String zkParentKey = "/test";
+
+  @BeforeTest
+  private void setUp() {
+    this._zkMetaClient = createZkMetaClient();
+    this._zkMetaClient.connect();
+  }
+
+  @Test
+  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);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy2 = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy3 = new CreatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(createPuppy2);
+    puppyManager.addPuppy(createPuppy3);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testDeletePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(deletePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testGetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testSetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(setPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testUpdatePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testCrudPuppies() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+
+  @Test
+  public void testBasicParentListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    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());
+    };
+
+    _zkMetaClient.subscribeChildChanges(zkParentKey, childChangeListener, false);
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+
+    long timeoutInSeconds = 20; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, globalChildChangeCounter);
+
+    // cleanup
+    _zkMetaClient.unsubscribeChildChanges(zkParentKey, childChangeListener);
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testComplexParentListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    // Global counter for all child changes
+    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());
+    };
+
+
+    _zkMetaClient.subscribeChildChanges(zkParentKey, childChangeListener, false);
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, globalChildChangeCounter);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+    _zkMetaClient.unsubscribeChildChanges(zkParentKey, childChangeListener);
+    _zkMetaClient.delete(zkParentKey);
+  }
+
+
+  @Test
+  public void testChildListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    // Setting num diff paths to 3 until we find a better way of scaling listeners.
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 3);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    // Create a child listener for each child defined in number diff paths in puppyspec.
+    // TODO: Make this a parameter for a loop.
+    AtomicInteger childChangeCounter0 = new AtomicInteger();
+    ChildChangeListener childChangeListener0 = (changedPath, changeType) -> {
+      childChangeCounter0.addAndGet(1);
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + childChangeCounter0.get());
+    };
+    _zkMetaClient.subscribeChildChanges("/test/0", childChangeListener0, false);
+
+    AtomicInteger childChangeCounter1 = new AtomicInteger();
+    ChildChangeListener childChangeListener1 = (changedPath, changeType) -> {
+      childChangeCounter1.addAndGet(1);
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + childChangeCounter1.get());
+    };
+    _zkMetaClient.subscribeChildChanges("/test/1", childChangeListener1, false);
+
+    AtomicInteger childChangeCounter2 = new AtomicInteger();
+    ChildChangeListener childChangeListener2 = (changedPath, changeType) -> {
+      childChangeCounter2.addAndGet(1);
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + childChangeCounter2.get());
+    };
+    _zkMetaClient.subscribeChildChanges("/test/2", childChangeListener2, false);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration

Review Comment:
   Since it's the same for all tests, I'll change it to a global var



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] xyuanlu commented on a diff in pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu commented on code in PR #2548:
URL: https://github.com/apache/helix/pull/2548#discussion_r1267351268


##########
meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java:
##########
@@ -0,0 +1,68 @@
+package org.apache.helix.metaclient.impl.zk.TestMultiThreadStressTest;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+import org.apache.helix.metaclient.puppy.AbstractPuppy;
+import org.apache.helix.metaclient.puppy.PuppySpec;
+
+import java.util.Objects;
+import java.util.Random;
+
+public class GetPuppy extends AbstractPuppy {
+
+  private final Random _random;
+  private final String _parentPath = "/test";
+
+  public GetPuppy(MetaClientInterface<String> metaclient, PuppySpec puppySpec) {
+    super(metaclient, puppySpec);
+    _random = new Random();
+  }
+
+  @Override
+  protected void bark() {
+    int randomNumber = _random.nextInt(_puppySpec.getNumberDiffPaths());
+    if (shouldIntroduceError()) {
+      try {
+        _metaclient.get("invalid");
+        _unhandledErrorCounter++;
+      } catch (IllegalArgumentException e) {
+        System.out.println(Thread.currentThread().getName() + " intentionally tried to read an invalid path.");
+      }
+    } else {
+      System.out.println(Thread.currentThread().getName() + " is attempting to read node: " + randomNumber);
+      String nodeValue = _metaclient.get(_parentPath + "/" + randomNumber);
+      if (Objects.equals(nodeValue, null)) {
+        System.out.println(Thread.currentThread().getName() + " failed to read node " + randomNumber + ", it does not exist");
+      } else {
+        System.out.println(Thread.currentThread().getName() + " successfully read node " + randomNumber + " at time: " + System.currentTimeMillis());

Review Comment:
   QQ: why we dont get current time in prev 2 prints? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] xyuanlu commented on a diff in pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu commented on code in PR #2548:
URL: https://github.com/apache/helix/pull/2548#discussion_r1267352684


##########
meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java:
##########
@@ -0,0 +1,368 @@
+package org.apache.helix.metaclient.impl.zk.TestMultiThreadStressTest;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.helix.metaclient.api.ChildChangeListener;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClient;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClientTestBase;
+import org.apache.helix.metaclient.puppy.ExecDelay;
+import org.apache.helix.metaclient.puppy.PuppyManager;
+import org.apache.helix.metaclient.puppy.PuppyMode;
+import org.apache.helix.metaclient.puppy.PuppySpec;
+import org.apache.helix.metaclient.puppy.AbstractPuppy;
+import org.testng.Assert;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class TestMultiThreadStressZKClient extends ZkMetaClientTestBase {
+
+  private ZkMetaClient<String> _zkMetaClient;
+  private final String zkParentKey = "/test";
+
+  @BeforeTest
+  private void setUp() {
+    this._zkMetaClient = createZkMetaClient();
+    this._zkMetaClient.connect();
+  }
+
+  @Test
+  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);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy2 = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy3 = new CreatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(createPuppy2);
+    puppyManager.addPuppy(createPuppy3);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testDeletePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(deletePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testGetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testSetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(setPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testUpdatePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testCrudPuppies() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+
+  @Test
+  public void testBasicParentListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    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());
+    };
+
+    _zkMetaClient.subscribeChildChanges(zkParentKey, childChangeListener, false);
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+
+    long timeoutInSeconds = 20; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, globalChildChangeCounter);
+
+    // cleanup
+    _zkMetaClient.unsubscribeChildChanges(zkParentKey, childChangeListener);
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testComplexParentListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    // Global counter for all child changes
+    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());
+    };
+
+
+    _zkMetaClient.subscribeChildChanges(zkParentKey, childChangeListener, false);
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, globalChildChangeCounter);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+    _zkMetaClient.unsubscribeChildChanges(zkParentKey, childChangeListener);
+    _zkMetaClient.delete(zkParentKey);
+  }
+
+
+  @Test
+  public void testChildListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    // Setting num diff paths to 3 until we find a better way of scaling listeners.
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 3);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    // Create a child listener for each child defined in number diff paths in puppyspec.
+    // TODO: Make this a parameter for a loop.
+    AtomicInteger childChangeCounter0 = new AtomicInteger();
+    ChildChangeListener childChangeListener0 = (changedPath, changeType) -> {
+      childChangeCounter0.addAndGet(1);
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + childChangeCounter0.get());
+    };
+    _zkMetaClient.subscribeChildChanges("/test/0", childChangeListener0, false);
+
+    AtomicInteger childChangeCounter1 = new AtomicInteger();
+    ChildChangeListener childChangeListener1 = (changedPath, changeType) -> {
+      childChangeCounter1.addAndGet(1);
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + childChangeCounter1.get());
+    };
+    _zkMetaClient.subscribeChildChanges("/test/1", childChangeListener1, false);
+
+    AtomicInteger childChangeCounter2 = new AtomicInteger();
+    ChildChangeListener childChangeListener2 = (changedPath, changeType) -> {
+      childChangeCounter2.addAndGet(1);
+      System.out.println("-------------- Child change detected: " + changeType + " at path: " + changedPath + " number of changes: " + childChangeCounter2.get());
+    };
+    _zkMetaClient.subscribeChildChanges("/test/2", childChangeListener2, false);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration

Review Comment:
   nit: static var



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] xyuanlu merged pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu merged PR #2548:
URL: https://github.com/apache/helix/pull/2548


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] Marcosrico commented on pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on PR #2548:
URL: https://github.com/apache/helix/pull/2548#issuecomment-1644035486

   PR approved by @xyuanlu 
   Commit Message: MultiThreading Stress Test Lattice - CRUD puppies and Listener and Tests (Part 2)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] Marcosrico commented on a diff in pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2548:
URL: https://github.com/apache/helix/pull/2548#discussion_r1268377795


##########
meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java:
##########
@@ -0,0 +1,368 @@
+package org.apache.helix.metaclient.impl.zk.TestMultiThreadStressTest;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.helix.metaclient.api.ChildChangeListener;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClient;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClientTestBase;
+import org.apache.helix.metaclient.puppy.ExecDelay;
+import org.apache.helix.metaclient.puppy.PuppyManager;
+import org.apache.helix.metaclient.puppy.PuppyMode;
+import org.apache.helix.metaclient.puppy.PuppySpec;
+import org.apache.helix.metaclient.puppy.AbstractPuppy;
+import org.testng.Assert;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class TestMultiThreadStressZKClient extends ZkMetaClientTestBase {
+
+  private ZkMetaClient<String> _zkMetaClient;
+  private final String zkParentKey = "/test";
+
+  @BeforeTest
+  private void setUp() {
+    this._zkMetaClient = createZkMetaClient();
+    this._zkMetaClient.connect();
+  }
+
+  @Test
+  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);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy2 = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy3 = new CreatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(createPuppy2);
+    puppyManager.addPuppy(createPuppy3);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testDeletePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(deletePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testGetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testSetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(setPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testUpdatePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testCrudPuppies() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+
+  @Test
+  public void testBasicParentListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    AtomicInteger globalChildChangeCounter = new AtomicInteger();

Review Comment:
   For lambda expressions we need the variable to be final and atomicIntegers works. While it is not necessary to have a lambda expression, the overhead of using an atomicinteger over a regular int is minimal in this scenario and might not be worth the additional lines of code (that make it slightly messier). Let me know if this makes sense, thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] Marcosrico commented on pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on PR #2548:
URL: https://github.com/apache/helix/pull/2548#issuecomment-1644280712

   PR approved by @xyuanlu 
   Commit message: Multithreading stress test lattice - CRUD puppies and Listener Tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] Marcosrico commented on a diff in pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2548:
URL: https://github.com/apache/helix/pull/2548#discussion_r1267418183


##########
meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/GetPuppy.java:
##########
@@ -0,0 +1,68 @@
+package org.apache.helix.metaclient.impl.zk.TestMultiThreadStressTest;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+import org.apache.helix.metaclient.puppy.AbstractPuppy;
+import org.apache.helix.metaclient.puppy.PuppySpec;
+
+import java.util.Objects;
+import java.util.Random;
+
+public class GetPuppy extends AbstractPuppy {
+
+  private final Random _random;
+  private final String _parentPath = "/test";
+
+  public GetPuppy(MetaClientInterface<String> metaclient, PuppySpec puppySpec) {
+    super(metaclient, puppySpec);
+    _random = new Random();
+  }
+
+  @Override
+  protected void bark() {
+    int randomNumber = _random.nextInt(_puppySpec.getNumberDiffPaths());
+    if (shouldIntroduceError()) {
+      try {
+        _metaclient.get("invalid");
+        _unhandledErrorCounter++;
+      } catch (IllegalArgumentException e) {
+        System.out.println(Thread.currentThread().getName() + " intentionally tried to read an invalid path.");
+      }
+    } else {
+      System.out.println(Thread.currentThread().getName() + " is attempting to read node: " + randomNumber);
+      String nodeValue = _metaclient.get(_parentPath + "/" + randomNumber);
+      if (Objects.equals(nodeValue, null)) {
+        System.out.println(Thread.currentThread().getName() + " failed to read node " + randomNumber + ", it does not exist");
+      } else {
+        System.out.println(Thread.currentThread().getName() + " successfully read node " + randomNumber + " at time: " + System.currentTimeMillis());

Review Comment:
   No particular reason. I can add timestamps to all print statements. Thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] xyuanlu commented on pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu commented on PR #2548:
URL: https://github.com/apache/helix/pull/2548#issuecomment-1641073406

   Great PR! Some nits. Thanks for working on this!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] xyuanlu commented on a diff in pull request #2548: MultiThreading Stress Test Lattice - CRUD and Listener Puppies and Tests (Part 2)

Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu commented on code in PR #2548:
URL: https://github.com/apache/helix/pull/2548#discussion_r1267358112


##########
meta-client/src/test/java/org/apache/helix/metaclient/impl/zk/TestMultiThreadStressTest/TestMultiThreadStressZKClient.java:
##########
@@ -0,0 +1,368 @@
+package org.apache.helix.metaclient.impl.zk.TestMultiThreadStressTest;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.helix.metaclient.api.ChildChangeListener;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClient;
+import org.apache.helix.metaclient.impl.zk.ZkMetaClientTestBase;
+import org.apache.helix.metaclient.puppy.ExecDelay;
+import org.apache.helix.metaclient.puppy.PuppyManager;
+import org.apache.helix.metaclient.puppy.PuppyMode;
+import org.apache.helix.metaclient.puppy.PuppySpec;
+import org.apache.helix.metaclient.puppy.AbstractPuppy;
+import org.testng.Assert;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class TestMultiThreadStressZKClient extends ZkMetaClientTestBase {
+
+  private ZkMetaClient<String> _zkMetaClient;
+  private final String zkParentKey = "/test";
+
+  @BeforeTest
+  private void setUp() {
+    this._zkMetaClient = createZkMetaClient();
+    this._zkMetaClient.connect();
+  }
+
+  @Test
+  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);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy2 = new CreatePuppy(_zkMetaClient, puppySpec);
+    CreatePuppy createPuppy3 = new CreatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(createPuppy2);
+    puppyManager.addPuppy(createPuppy3);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testDeletePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(deletePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testGetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testSetPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(setPuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testUpdatePuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+  @Test
+  public void testCrudPuppies() {
+    _zkMetaClient.create(zkParentKey, "test");
+
+    PuppySpec puppySpec = new PuppySpec(PuppyMode.REPEAT, 0.2f, new ExecDelay(5000, 0.1f), 5);
+    CreatePuppy createPuppy = new CreatePuppy(_zkMetaClient, puppySpec);
+    GetPuppy getPuppy = new GetPuppy(_zkMetaClient, puppySpec);
+    DeletePuppy deletePuppy = new DeletePuppy(_zkMetaClient, puppySpec);
+    SetPuppy setPuppy = new SetPuppy(_zkMetaClient, puppySpec);
+    UpdatePuppy updatePuppy = new UpdatePuppy(_zkMetaClient, puppySpec);
+
+    PuppyManager puppyManager = new PuppyManager();
+    puppyManager.addPuppy(createPuppy);
+    puppyManager.addPuppy(getPuppy);
+    puppyManager.addPuppy(deletePuppy);
+    puppyManager.addPuppy(setPuppy);
+    puppyManager.addPuppy(updatePuppy);
+
+    long timeoutInSeconds = 60; // Set the desired timeout duration
+
+    puppyManager.start(timeoutInSeconds);
+
+    assertNoExceptions(puppyManager, null);
+
+    // cleanup
+    _zkMetaClient.recursiveDelete(zkParentKey);
+    Assert.assertEquals(_zkMetaClient.countDirectChildren(zkParentKey), 0);
+  }
+
+
+  @Test
+  public void testBasicParentListenerPuppy() {
+    _zkMetaClient.create(zkParentKey, "test");
+    AtomicInteger globalChildChangeCounter = new AtomicInteger();

Review Comment:
   Since we are not actually reading and doing incremental at the same time, why we need AtomicInteger?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] junkaixue commented on pull request #2548: MultiThreading Puppy Stress Testing

Posted by "junkaixue (via GitHub)" <gi...@apache.org>.
junkaixue commented on PR #2548:
URL: https://github.com/apache/helix/pull/2548#issuecomment-1613843751

   This is good to see chaos engineering in Helix! But one thing I would suggest: if you would like to introduce this Puppy, let's have general Puppy skeleton and implementation in one PR.  Put the Puppy with concrete testing in a different PR. That can keep the logic clean.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org