jacoblukose commented on code in PR #2930:
URL: https://github.com/apache/helix/pull/2930#discussion_r1847728448


##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java:
##########
@@ -423,6 +423,16 @@ public boolean update(String path, DataUpdater<T> updater, 
int options) {
    * sync update
    */
   public AccessResult doUpdate(String path, DataUpdater<T> updater, int 
options) {
+    return doUpdate(path, updater, options, ZkClient.TTL_NOT_SET);
+  }
+
+  /**
+   * sync update with ttl
+   *
+   * ttl is only used when creating new znode, hence if znode is already 
created with a ttl, further
+   * update operations will not update the znode ttl even if ttl is provided 
in the options
+   */
+  public AccessResult doUpdate(String path, DataUpdater<T> updater, int 
options, long ttl) {

Review Comment:
   thanks @xyuanlu the only reason why `doUpdate` need a ttl value to be passed 
in is because the current code is using a `doCreate` fn inside the `doUpdate`. 
If there is no `doCreate` call made within `doUpdate` then there is no need to 
pass in the ttl as well, reason being, `ttl` can only be enforced during 
creation of znode. 
   
   the related q is it correct behaviour on the existing `doUpdate` fn code to 
call a `doCreate` internally? if its correct, then challenge is how to let such 
a create call have the `ttl` supported. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to