[GitHub] [hbase] sguggilam commented on a change in pull request #1945: HBASE-24603: Make Zookeeper sync() call synchronous

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

sguggilam commented on a change in pull request #1945:
URL: https://github.com/apache/hbase/pull/1945#discussion_r443775794



##########
File path: hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java
##########
@@ -595,9 +604,28 @@ private void connectionEvent(WatchedEvent event) {
    * data of an existing node and delete or transition that node, utilizing the
    * previously read version and data.  We want to ensure that the version read
    * is up-to-date from when we begin the operation.
+   * <p>
    */
-  public void sync(String path) throws KeeperException {
-    this.recoverableZooKeeper.sync(path, null, null);
+  public void syncOrTimeout(String path) throws KeeperException {
+    final CountDownLatch latch = new CountDownLatch(1);
+    long startTime = EnvironmentEdgeManager.currentTime();
+    this.recoverableZooKeeper.sync(path, (i, s, o) -> latch.countDown(), null);

Review comment:
       Why can't we instead pass ctx as the last argument instead of NULL during the actual API call ? Just in case we want to use this field somewhere else in future to get the context in callback.




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