[GitHub] [helix] mgao0 commented on a diff in pull request #2090: Add TTL and Container modes to sync create API in ZkClient and ZkConnection

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

mgao0 commented on code in PR #2090:
URL: https://github.com/apache/helix/pull/2090#discussion_r875250187


##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java:
##########
@@ -438,6 +438,41 @@ public void createPersistent(String path)
     createPersistent(path, false);
   }
 
+  /**
+   * Create a persistent node with TTL.
+   * @param path the path where you want the node to be created
+   * @param ttl TTL of the node in milliseconds
+   * @throws ZkInterruptedException
+   *           if operation was interrupted, or a required reconnection got interrupted
+   * @throws IllegalArgumentException
+   *           if called from anything except the ZooKeeper event thread
+   * @throws ZkException
+   *           if any ZooKeeper exception occurred
+   * @throws RuntimeException
+   *           if any other exception occurs
+   */
+  public void createPersistentWithTTL(String path, long ttl)

Review Comment:
   Do we need `public void createPersistentSequentialWithTTL(String path, long ttl)` and `public void createPersistentSequentialWithTTL(String path, boolean createParents, long ttl)`?



##########
zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java:
##########
@@ -89,6 +89,65 @@ public void afterClass() {
     _zkClient.close();
   }
 
+  @Test
+  void testUnimplementedTypes() {
+    System.clearProperty("zookeeper.extendedTypesEnabled");
+    String parentPath = "/tmp";
+    String path = "/tmp/unimplemented";
+    long ttl = 1L;
+    _zkClient.deleteRecursively(parentPath);
+    try {
+      _zkClient.createPersistentWithTTL(path, true, ttl);
+    } catch (ZkException e) {
+      AssertJUnit.assertTrue(e.getCause() instanceof KeeperException.UnimplementedException);
+      return;
+    }
+    _zkClient.deleteRecursively(parentPath);
+    AssertJUnit.fail();
+  }
+
+  @Test
+  void testCreatePersistentWithTTL() {
+    System.setProperty("zookeeper.extendedTypesEnabled", "true");
+    String parentPath = "/tmp";
+    String path = "/tmp/createTTL";
+    _zkClient.deleteRecursively(parentPath);
+    AssertJUnit.assertFalse(_zkClient.exists(path));
+    long ttl = 1L;
+    _zkClient.createPersistentWithTTL(path, true, ttl);
+    AssertJUnit.assertTrue(_zkClient.exists(path));
+    _zkClient.deleteRecursively(parentPath);
+    System.clearProperty("zookeeper.extendedTypesEnabled");
+  }
+
+  @Test
+  void testCreatePersistentSequentialWithTTL() {
+    System.setProperty("zookeeper.extendedTypesEnabled", "true");
+    String parentPath = "/tmp";
+    String path = "/tmp/createSequentialTTL";
+    _zkClient.deleteRecursively(parentPath);
+    AssertJUnit.assertFalse(_zkClient.exists(path + "0000000000"));
+    long ttl = 1L;
+    _zkClient.createPersistent(parentPath);
+    _zkClient.createPersistentSequentialWithTTL(path, null, ttl);
+    AssertJUnit.assertTrue(_zkClient.exists(path + "0000000000"));
+    _zkClient.deleteRecursively(parentPath);
+    System.clearProperty("zookeeper.extendedTypesEnabled");
+  }
+
+  @Test
+  void testCreateContainer() {
+    System.setProperty("zookeeper.extendedTypesEnabled", "true");
+    String parentPath = "/tmp";
+    String path = "/tmp/createContainer";
+    _zkClient.deleteRecursively(parentPath);
+    AssertJUnit.assertFalse(_zkClient.exists(path));
+    _zkClient.createContainer(path, true);

Review Comment:
   +1



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