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


##########
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:
   @xyuanlu thanks for reponding and on the ack. It will be helpful if you can 
also callout alternative approach here to improve the craft if you thinks thats 
seen comprised. Happy to correct it. 
   
   @junkaixue thanks for responding, would you be kind enough to highlight the 
code part where  the said concern: `I am not buying this. As I said, when a 
node is created, we should not update its property and change it from a 
persistent node to TTL node.` is. `doUpdate` fn is not going to update ttl for 
an existing node, only when underlying `create` is called, it leverage `ttl` 
passed if any. And also, if you see, public facing `doUpdate` doesnt have `ttl` 
option exposed. It would be great if you can suggest an alternative on how this 
need to handled as well. 
   
   Also I think there is some misunderstanding, the usecase is not  to change 
znodes created with persistent mode to migrate to use ttl mode. Idea is to just 
expand the scope of `ZkBucketDataAccessor` to leverage zk ttl feature when the 
same is being initialized.



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