virajjasani commented on a change in pull request #1945:
URL: https://github.com/apache/hbase/pull/1945#discussion_r443368727
##########
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);
+ try {
+ if (!latch.await(zkSyncTimeout, TimeUnit.MILLISECONDS)) {
+ LOG.error("sync() operation to ZK timed out. Configured timeout: {}ms.
This usually points "
Review comment:
IMHO `error` is fine but better to keep it `warn`. We anyways have
`warn` for `InterruptedException` also.
##########
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:
`RecoverableZooKeeper` is IA.Private. Can we remove last argument
`Object ctx` as part of this PR? Anyways we pass null context to actual API.
----------------------------------------------------------------
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:
[email protected]
