----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/739/#review750 -----------------------------------------------------------
this looks really great! one thing to think about (you don't need to think about it too long, but it would be great if you could "fix it"): we generally only use generated jute Record instances. i think this makes it easier if we ever switch serialization libraries, but MultiResponse is not generated. i understand why, but it would be cool if we could use a generated class. this is not blocking the patch. the cleanup of pending ops was not in the uploaded diffs, but i looked at the code on git hub. i think this is almost right, but removeChangeRecord has a bug: we do the checking based on the Map not the list, so when we remove the ChangeRecord we need to check to see if there was something there before that we replaced and put it back. i think this is the last blocker that i can see. - Benjamin On 2011-05-29 23:54:13, Ted Dunning wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/739/ > ----------------------------------------------------------- > > (Updated 2011-05-29 23:54:13) > > > Review request for zookeeper and Benjamin Reed. > > > Summary > ------- > > This mega-patch adds the multi-op capability to ZK. This allows a batch of > create, delete, update or version-check operations to > succeed or fail together. Both C and java bindings are provided. > > > This addresses bug ZOOKEEPER-965. > https://issues.apache.org/jira/browse/ZOOKEEPER-965 > > > Diffs > ----- > > src/c/Makefile.am 65830fe > src/c/include/proto.h 843032f > src/c/include/zookeeper.h c055edb > src/c/src/zookeeper.c db715b0 > src/c/tests/TestMulti.cc PRE-CREATION > src/c/tests/TestOperations.cc f9441ea > src/c/tests/ZKMocks.cc a75dce6 > src/java/main/org/apache/zookeeper/MultiResponse.java PRE-CREATION > src/java/main/org/apache/zookeeper/MultiTransactionRecord.java PRE-CREATION > src/java/main/org/apache/zookeeper/Op.java PRE-CREATION > src/java/main/org/apache/zookeeper/OpResult.java PRE-CREATION > src/java/main/org/apache/zookeeper/Transaction.java PRE-CREATION > src/java/main/org/apache/zookeeper/ZooDefs.java 832976f > src/java/main/org/apache/zookeeper/ZooKeeper.java 00b4012 > src/java/main/org/apache/zookeeper/server/DataTree.java d16537e > src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java > 2538cf7 > src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 50f208d > src/java/main/org/apache/zookeeper/server/Request.java a5c57e2 > src/java/main/org/apache/zookeeper/server/RequestProcessor.java 5c3e8ff > src/java/main/org/apache/zookeeper/server/TraceFormatter.java 8ece929 > src/java/main/org/apache/zookeeper/server/package.html 3ec7656 > src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java > 49affd5 > src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 0ad4dd6 > src/java/test/org/apache/zookeeper/MultiResponseTest.java PRE-CREATION > src/java/test/org/apache/zookeeper/MultiTransactionRecordTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/MultiTransactionTest.java > PRE-CREATION > src/zookeeper.jute 7d96f32 > > Diff: https://reviews.apache.org/r/739/diff > > > Testing > ------- > > A number of unit tests have been implemented. Test coverage is very good. > > A sample application has been written to do simple operations on a graph of > nodes. > > > Thanks, > > Ted > >
