----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3364/#review4185 -----------------------------------------------------------
src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9424> This class is getting rather large, it should be refactored into it's own file. src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9426> all watchers _registered by this session_, correct? src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9427> this is confusing, what does "leak" mean? It might be good to move this detail from here to the programmer guide. src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9428> @throws? src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9425> Update all the @since with expected release - looks like 3.5.0 src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9429> why type vs having explicit methods? removeChildWatches removeDataWatches we're unlikely to add more watch types. On second thought though, shouldn't we have a "both" (or "alltypes", etc...) option as well? If that's the case perhaps 3 is enough to have a type... src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9432> since is incorrect src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9430> async operations don't throw keeperexception, see other async ops such as getData src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/3364/#comment9431> see my comment on method signature - your intuition was correct. - Patrick On 2012-01-04 09:41:43, Daniel Gómez Ferro wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3364/ > ----------------------------------------------------------- > > (Updated 2012-01-04 09:41:43) > > > Review request for zookeeper and Benjamin Reed. > > > Summary > ------- > > Added APIs to remove watches that are not needed anymore. If the removal > completes successfully it is guaranteed that the watcher won't be notified. > > With the current semantics if two clients remove watches on a znode at the > same time the watch is triggered, one could remove it successfully while the > other could receive the notification first. > > > This addresses bug ZOOKEEPER-442. > https://issues.apache.org/jira/browse/ZOOKEEPER-442 > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/ClientCnxn.java 6c25e40 > src/java/main/org/apache/zookeeper/KeeperException.java 7c10d2c > src/java/main/org/apache/zookeeper/Watcher.java 36c7b5b > src/java/main/org/apache/zookeeper/ZooDefs.java c7f1b20 > src/java/main/org/apache/zookeeper/ZooKeeper.java e82eaa0 > src/java/main/org/apache/zookeeper/server/DataTree.java 757a572 > src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java > 336827a > src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1a80d74 > src/java/main/org/apache/zookeeper/server/Request.java c6a2249 > src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 > src/java/main/org/apache/zookeeper/server/ZKDatabase.java 2842803 > src/java/test/org/apache/zookeeper/test/RemoveWatchesTest.java PRE-CREATION > src/zookeeper.jute d24e145 > > Diff: https://reviews.apache.org/r/3364/diff > > > Testing > ------- > > Added unit test that checks client side semantics. > > > Thanks, > > Daniel > >
