Remove watcher only when it registered watcher (reduce the annoying logging from zookeeper client about "Failed to find watcher!"
RB_ID=837073 Project: http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/commit/98dc9ab2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/tree/98dc9ab2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/diff/98dc9ab2 Branch: refs/heads/merge/DL-98 Commit: 98dc9ab2df8af7af26ca68dbe93be3d420cda417 Parents: b571d3b Author: Sijie Guo <sij...@twitter.com> Authored: Thu May 26 17:06:31 2016 -0700 Committer: Sijie Guo <sij...@twitter.com> Committed: Mon Dec 12 16:37:27 2016 -0800 ---------------------------------------------------------------------- .../main/java/com/twitter/distributedlog/BKLogHandler.java | 7 ------- .../java/com/twitter/distributedlog/BKLogReadHandler.java | 7 ++++++- .../java/com/twitter/distributedlog/BKLogWriteHandler.java | 6 +++++- .../com/twitter/distributedlog/readahead/ReadAheadWorker.java | 2 +- .../java/com/twitter/distributedlog/zk/ZKWatcherManager.java | 4 ++-- .../com/twitter/distributedlog/zk/TestZKWatcherManager.java | 2 +- 6 files changed, 15 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/blob/98dc9ab2/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogHandler.java ---------------------------------------------------------------------- diff --git a/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogHandler.java b/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogHandler.java index 9aa3465..a6ec318 100644 --- a/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogHandler.java +++ b/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogHandler.java @@ -702,13 +702,6 @@ public abstract class BKLogHandler implements Watcher, AsyncCloseable, AsyncAbor } @Override - public Future<Void> asyncClose() { - // No-op - this.zooKeeperClient.getWatcherManager().unregisterChildWatcher(logMetadata.getLogSegmentsPath(), this); - return Future.Void(); - } - - @Override public Future<Void> asyncAbort() { return asyncClose(); } http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/blob/98dc9ab2/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogReadHandler.java ---------------------------------------------------------------------- diff --git a/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogReadHandler.java b/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogReadHandler.java index 0bf6b84..6a8f90e 100644 --- a/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogReadHandler.java +++ b/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogReadHandler.java @@ -311,7 +311,12 @@ class BKLogReadHandler extends BKLogHandler { if (null != handleCache) { handleCache.clear(); } - return BKLogReadHandler.super.asyncClose(); + // No-op + zooKeeperClient.getWatcherManager().unregisterChildWatcher( + logMetadata.getLogSegmentsPath(), + BKLogReadHandler.this, + true); + return Future.Void(); } }); } http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/blob/98dc9ab2/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogWriteHandler.java ---------------------------------------------------------------------- diff --git a/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogWriteHandler.java b/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogWriteHandler.java index 573679a..4665ed5 100644 --- a/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogWriteHandler.java +++ b/distributedlog-core/src/main/java/com/twitter/distributedlog/BKLogWriteHandler.java @@ -1263,7 +1263,11 @@ class BKLogWriteHandler extends BKLogHandler { ).flatMap(new AbstractFunction1<Void, Future<Void>>() { @Override public Future<Void> apply(Void result) { - return BKLogWriteHandler.super.asyncClose(); + zooKeeperClient.getWatcherManager().unregisterChildWatcher( + logMetadata.getLogSegmentsPath(), + BKLogWriteHandler.this, + false); + return Future.Void(); } }); } http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/blob/98dc9ab2/distributedlog-core/src/main/java/com/twitter/distributedlog/readahead/ReadAheadWorker.java ---------------------------------------------------------------------- diff --git a/distributedlog-core/src/main/java/com/twitter/distributedlog/readahead/ReadAheadWorker.java b/distributedlog-core/src/main/java/com/twitter/distributedlog/readahead/ReadAheadWorker.java index a3fd239..5c15009 100644 --- a/distributedlog-core/src/main/java/com/twitter/distributedlog/readahead/ReadAheadWorker.java +++ b/distributedlog-core/src/main/java/com/twitter/distributedlog/readahead/ReadAheadWorker.java @@ -359,7 +359,7 @@ public class ReadAheadWorker implements ReadAheadCallback, Runnable, Watcher, As running = false; this.zkc.getWatcherManager() - .unregisterChildWatcher(this.logMetadata.getLogSegmentsPath(), this); + .unregisterChildWatcher(this.logMetadata.getLogSegmentsPath(), this, true); // Aside from unfortunate naming of variables, this allows // the currently active long poll to be interrupted and completed http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/blob/98dc9ab2/distributedlog-core/src/main/java/com/twitter/distributedlog/zk/ZKWatcherManager.java ---------------------------------------------------------------------- diff --git a/distributedlog-core/src/main/java/com/twitter/distributedlog/zk/ZKWatcherManager.java b/distributedlog-core/src/main/java/com/twitter/distributedlog/zk/ZKWatcherManager.java index a24b560..03b2841 100644 --- a/distributedlog-core/src/main/java/com/twitter/distributedlog/zk/ZKWatcherManager.java +++ b/distributedlog-core/src/main/java/com/twitter/distributedlog/zk/ZKWatcherManager.java @@ -139,7 +139,7 @@ public class ZKWatcherManager implements Watcher { return this; } - public void unregisterChildWatcher(String path, Watcher watcher) { + public void unregisterChildWatcher(String path, Watcher watcher, boolean removeFromServer) { Set<Watcher> watchers = childWatches.get(path); if (null == watchers) { logger.warn("No watchers found on path {} while unregistering child watcher {}.", @@ -155,7 +155,7 @@ public class ZKWatcherManager implements Watcher { if (watchers.isEmpty()) { // best-efforts to remove watches try { - if (null != zkc) { + if (null != zkc && removeFromServer) { zkc.get().removeWatches(path, this, WatcherType.Children, true, new AsyncCallback.VoidCallback() { @Override public void processResult(int rc, String path, Object ctx) { http://git-wip-us.apache.org/repos/asf/incubator-distributedlog/blob/98dc9ab2/distributedlog-core/src/test/java/com/twitter/distributedlog/zk/TestZKWatcherManager.java ---------------------------------------------------------------------- diff --git a/distributedlog-core/src/test/java/com/twitter/distributedlog/zk/TestZKWatcherManager.java b/distributedlog-core/src/test/java/com/twitter/distributedlog/zk/TestZKWatcherManager.java index 6f269c3..3ad181d 100644 --- a/distributedlog-core/src/test/java/com/twitter/distributedlog/zk/TestZKWatcherManager.java +++ b/distributedlog-core/src/test/java/com/twitter/distributedlog/zk/TestZKWatcherManager.java @@ -72,7 +72,7 @@ public class TestZKWatcherManager { assertEquals(event2, events.get(1)); // unregister watcher - watcherManager.unregisterChildWatcher(path, watcher); + watcherManager.unregisterChildWatcher(path, watcher, true); assertEquals(0, watcherManager.childWatches.size()); }