----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20448/#review40954 -----------------------------------------------------------
The patch looks good overall. Does this change affect the C client? ./src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/20448/#comment74276> trailing space ./src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/20448/#comment74280> Could you refactor the code to avoid duplicating the logic in sync and async versions? ./src/zookeeper.jute <https://reviews.apache.org/r/20448/#comment74275> trailing space - michim On April 17, 2014, 5:45 a.m., Rakesh R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20448/ > ----------------------------------------------------------- > > (Updated April 17, 2014, 5:45 a.m.) > > > Review request for zookeeper, michim and Raul Gutierrez Segales. > > > Bugs: ZOOKEEPER-1910 > https://issues.apache.org/jira/browse/ZOOKEEPER-1910 > > > Repository: zookeeper > > > Description > ------- > > RemoveWatches wrongly removes the watcher if multiple watches exists on a > path. As per the comments, we have splitted the removeWatches api into: > - removeWatches(path, watcher, watcherType, local); > - removeAllWatches(path, watcherType, local); > > > Diffs > ----- > > ./src/java/main/org/apache/zookeeper/ZooKeeper.java 1588229 > ./src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java 1588229 > ./src/java/main/org/apache/zookeeper/server/DataTree.java 1588229 > ./src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java > 1588229 > ./src/java/main/org/apache/zookeeper/server/WatchManager.java 1588229 > ./src/java/main/org/apache/zookeeper/server/ZKDatabase.java 1588229 > ./src/java/test/org/apache/zookeeper/RemoveWatchesTest.java 1588229 > ./src/zookeeper.jute 1588229 > > Diff: https://reviews.apache.org/r/20448/diff/ > > > Testing > ------- > > included tests > > > Thanks, > > Rakesh R > >
