apurtell commented on a change in pull request #1945:
URL: https://github.com/apache/hbase/pull/1945#discussion_r444414654
##########
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:
WARN implies to me as an operator that something bad happened that is
noteworthy but not an emergency. ERROR implies a serious problem that probably
should cause someone to be paged.
sync is going to occasionally time out. We should be handling such without
correctness problems. ERROR logging doesn't substitute for handling the timeout
properly. So, assuming we are handling timeouts correctly, WARN level logging
is most appropriate.
----------------------------------------------------------------
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]
