zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Reed" <br...@yahoo-inc.com>
Subject Re: Review Request: ZOOKEEPER-965 - Add multi operation
Date Fri, 20 May 2011 04:15:05 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/739/#review689
-----------------------------------------------------------



src/c/include/zookeeper.h
<https://reviews.apache.org/r/739/#comment1380>

    shouldn't this be a union



src/c/include/zookeeper.h
<https://reviews.apache.org/r/739/#comment1381>

    it isn't clear to me that we should expose check outside of a multitransaction.



src/java/main/org/apache/zookeeper/Transaction.java
<https://reviews.apache.org/r/739/#comment1382>

    should we also have an asynchronous version?



src/java/main/org/apache/zookeeper/ZooKeeper.java
<https://reviews.apache.org/r/739/#comment1383>

    i think we also need an asynchronous version.



src/java/main/org/apache/zookeeper/server/DataTree.java
<https://reviews.apache.org/r/739/#comment1384>

    if stat didn't return anything, we could skip this altogether. if we are going to return
something, should we return more than the version?



src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/739/#comment1385>

    i don't think we want to log this at all do we?



src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/739/#comment1386>

    don't use tabs. spaces!



src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/739/#comment1392>

    i think we are only half atomic here. if the multi txn fails half way through, it will
not be applied to the database, but we will still have recorded it as pending, so we may fail
later changes.
    
    for example, if there is a create for a znode that would otherwise succeed, but fails
do to a later (in the multi txn) check failing. there will be a pending change record created,
so even though the create failed, later creates to that znode will result in a node exists
error.



src/zookeeper.jute
<https://reviews.apache.org/r/739/#comment1379>

    i don't think we should use TxnHeader since all we need is the type.


- Benjamin


On 2011-05-19 02:13:14, Ted Dunning wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/739/
> -----------------------------------------------------------
> 
> (Updated 2011-05-19 02:13:14)
> 
> 
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message