[GitHub] [helix] kaisun2000 commented on a change in pull request #1194: Fix flaky test testGetChildrenOnLargeNumChildren

[GitBox <gi...@apache.org>]

kaisun2000 commented on a change in pull request #1194:
URL: https://github.com/apache/helix/pull/1194#discussion_r464604562



##########
File path: zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
##########
@@ -868,59 +864,38 @@ public void testAsyncWriteOperations() {
    * Tests getChildren() when there are an excessive number of children and connection loss happens,
    * the operation should terminate and exit retry loop.
    */
-  @Test
+  @Test(timeOut = 30L)
   public void testGetChildrenOnLargeNumChildren() throws Exception {
-    // Default packetLen is 4M. It is static final and initialized
-    // when first zkClient is created.
-    // So we could not just set "jute.maxbuffer" to change the value.
-    // Reflection is needed to change the value.
-    // Remove "final" modifier
-    Field modifiersField = Field.class.getDeclaredField("modifiers");
-    boolean isModifierAccessible = modifiersField.isAccessible();
-    modifiersField.setAccessible(true);
-
-    Field packetLenField = ClientCnxn.class.getDeclaredField("packetLen");
-    Field childrenLimitField =
-        org.apache.helix.zookeeper.zkclient.ZkClient.class.getDeclaredField("NUM_CHILDREN_LIMIT");
-    modifiersField.setInt(packetLenField, packetLenField.getModifiers() & ~Modifier.FINAL);
-    modifiersField.setInt(childrenLimitField, childrenLimitField.getModifiers() & ~Modifier.FINAL);
-
-    boolean isPacketLenAccessible = packetLenField.isAccessible();
-    packetLenField.setAccessible(true);
-    int originPacketLen = packetLenField.getInt(null);
-    // Keep 150 bytes for successfully creating each child node.
-    packetLenField.set(null, 150);
-
-    boolean isChildrenLimitAccessible = childrenLimitField.isAccessible();
-    childrenLimitField.setAccessible(true);
-    int originChildrenLimit = childrenLimitField.getInt(null);
-    childrenLimitField.set(null, 2);
-
-    String path = "/" + TestHelper.getTestMethodName();
-    // Create 5 children to make packet length of children exceed 150 bytes
+    // Create 110K children to make packet length of children exceed 4 MB
     // and cause connection loss for getChildren() operation
-    for (int i = 0; i < 5; i++) {
-      _zkClient.createPersistent(path + "/" + UUID.randomUUID().toString(), true);
+    String path = "/" + TestHelper.getTestMethodName();
+    int toCreateChildrenCount = 110;
+
+    _zkClient.createPersistent(path);
+
+    for (int i = 0; i < toCreateChildrenCount; i++) {
+      List<Op> ops = new ArrayList<>(1000);
+      for (int j = 0; j < 1000; j++) {
+        String child = path + "/" + UUID.randomUUID().toString();
+        ops.add(Op.create(child, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.EPHEMERAL));
+      }
+      // Reduce total creation time by batch creating znodes
+      _zkClient.multi(ops);
     }
 
     try {
       _zkClient.getChildren(path);
-      Assert.fail("Should not successfully get children.");
+      Assert.fail("Should not successfully get children because of connection loss.");
     } catch (ZkException expected) {
       Assert.assertEquals(expected.getMessage(),
           "org.apache.zookeeper.KeeperException$MarshallingErrorException: "
               + "KeeperErrorCode = MarshallingError");
     } finally {
-      packetLenField.set(null, originPacketLen);
-      packetLenField.setAccessible(isPacketLenAccessible);
-
-      childrenLimitField.set(null, originChildrenLimit);
-      childrenLimitField.setAccessible(isChildrenLimitAccessible);
-
-      modifiersField.setAccessible(isModifierAccessible);
+      _zkClient.close();
+      _zkClient = new ZkClient(ZkTestBase.ZK_ADDR);
 
       Assert.assertTrue(TestHelper.verify(() -> {
-        _zkClient.deleteRecursively(path);
+        _zkClient.delete(path);

Review comment:
       I see. Did not notice this ephemeral node. So closing the _zkClient delete the nodes. The second delete is just to delete the parent path. This looks good. 




----------------------------------------------------------------
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.

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