[
https://issues.apache.org/jira/browse/ZOOKEEPER-965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13038944#comment-13038944
]
[email protected] commented on ZOOKEEPER-965:
---------------------------------------------------------
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/c/include/zookeeper.h, line 1119
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19416#file19416line1119>
bq. >
bq. > it isn't clear to me that we should expose check outside of a
multitransaction.
That's a valid point. I did it for completeness, but there's really no need to
have a check op outside a multi op. It would clean up the code as well if I
removed it. I'll try to get that done tonight as well.
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/java/main/org/apache/zookeeper/Transaction.java, line 57
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19425#file19425line57>
bq. >
bq. > should we also have an asynchronous version?
Agreed. Ted's created a new jira case for this.
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 801
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19427#file19427line801>
bq. >
bq. > i think we also need an asynchronous version.
Agreed. Ted's created a new jira case for this.
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/java/main/org/apache/zookeeper/server/DataTree.java, line 817
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19428#file19428line817>
bq. >
bq. > 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?
I'm not sure I understand... can you elaborate ?
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java,
line 483
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19430#file19430line483>
bq. >
bq. > i don't think we want to log this at all do we?
Agreed. I'll clean that up tonight.
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java,
line 484
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19430#file19430line484>
bq. >
bq. > don't use tabs. spaces!
Sorry... at my day job we use tabs (I know) and I forgot to switch my settings
around -- I'm usually very careful about that ;-).
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java,
line 489
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19430#file19430line489>
bq. >
bq. > 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.
bq. >
bq. > 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.
I thought not applying it to the database was sufficient, but perhaps I'm
wrong. I thought the pending change records would get discarded if the multi op
failed. Do I need to do that explicitly somehow? What are your thoughts on how
to deal with this?
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/zookeeper.jute, line 267
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19440#file19440line267>
bq. >
bq. > i don't think we should use TxnHeader since all we need is the type.
Agreed. I'll see about refactoring that.
bq. On 2011-05-20 04:15:05, Benjamin Reed wrote:
bq. > src/c/include/zookeeper.h, line 273
bq. > <https://reviews.apache.org/r/739/diff/4/?file=19416#file19416line273>
bq. >
bq. > shouldn't this be a union
It originally was a union but I ran into some annoyances when trying to use the
C API from C++ code as C++ won't let you use designated initializers unless you
compile with a newer --std flag (which, when enabled, caused other zookeeper
code not to compile from C++). e.g. you get warnings like:
tests/TestMulti.cc:689: warning: extended initializer lists only available with
-std=c++0x or -std=gnu++0x
Anyhow, I think the current implementation is a bit of a hack, and would prefer
a union as well. Let me take another look at this tonight.
- Marshall
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/739/#review689
-----------------------------------------------------------
On 2011-05-19 02:13:14, Ted Dunning wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/739/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-05-19 02:13:14)
bq.
bq.
bq. Review request for zookeeper and Benjamin Reed.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This mega-patch adds the multi-op capability to ZK. This allows a batch
of create, delete, update or version-check operations to
bq. succeed or fail together. Both C and java bindings are provided.
bq.
bq.
bq. This addresses bug ZOOKEEPER-965.
bq. https://issues.apache.org/jira/browse/ZOOKEEPER-965
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/c/Makefile.am 65830fe
bq. src/c/include/proto.h 843032f
bq. src/c/include/zookeeper.h c055edb
bq. src/c/src/zookeeper.c db715b0
bq. src/c/tests/TestMulti.cc PRE-CREATION
bq. src/c/tests/TestOperations.cc f9441ea
bq. src/c/tests/ZKMocks.cc a75dce6
bq. src/java/main/org/apache/zookeeper/MultiResponse.java PRE-CREATION
bq. src/java/main/org/apache/zookeeper/MultiTransactionRecord.java
PRE-CREATION
bq. src/java/main/org/apache/zookeeper/Op.java PRE-CREATION
bq. src/java/main/org/apache/zookeeper/OpResult.java PRE-CREATION
bq. src/java/main/org/apache/zookeeper/Transaction.java PRE-CREATION
bq. src/java/main/org/apache/zookeeper/ZooDefs.java 832976f
bq. src/java/main/org/apache/zookeeper/ZooKeeper.java 00b4012
bq. src/java/main/org/apache/zookeeper/server/DataTree.java d16537e
bq. src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
2538cf7
bq. src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
50f208d
bq. src/java/main/org/apache/zookeeper/server/Request.java a5c57e2
bq. src/java/main/org/apache/zookeeper/server/RequestProcessor.java 5c3e8ff
bq. src/java/main/org/apache/zookeeper/server/TraceFormatter.java 8ece929
bq. src/java/main/org/apache/zookeeper/server/package.html 3ec7656
bq. src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java
49affd5
bq. src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java
0ad4dd6
bq. src/java/test/org/apache/zookeeper/MultiResponseTest.java PRE-CREATION
bq. src/java/test/org/apache/zookeeper/MultiTransactionRecordTest.java
PRE-CREATION
bq. src/java/test/org/apache/zookeeper/test/MultiTransactionTest.java
PRE-CREATION
bq. src/zookeeper.jute 7d96f32
bq.
bq. Diff: https://reviews.apache.org/r/739/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. A number of unit tests have been implemented. Test coverage is very good.
bq.
bq. A sample application has been written to do simple operations on a graph
of nodes.
bq.
bq.
bq. Thanks,
bq.
bq. Ted
bq.
bq.
> Need a multi-update command to allow multiple znodes to be updated safely
> -------------------------------------------------------------------------
>
> Key: ZOOKEEPER-965
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-965
> Project: ZooKeeper
> Issue Type: Bug
> Affects Versions: 3.3.3
> Reporter: Ted Dunning
> Assignee: Ted Dunning
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-965.patch, ZOOKEEPER-965.patch,
> ZOOKEEPER-965.patch, ZOOKEEPER-965.patch, ZOOKEEPER-965.patch,
> ZOOKEEPER-965.patch, ZOOKEEPER-965.patch, ZOOKEEPER-965.patch,
> ZOOKEEPER-965.patch, ZOOKEEPER-965.patch, ZOOKEEPER-965.patch,
> ZOOKEEPER-965.patch, ZOOKEEPER-965.patch, ZOOKEEPER-965.patch
>
>
> The basic idea is to have a single method called "multi" that will accept a
> list of create, delete, update or check objects each of which has a desired
> version or file state in the case of create. If all of the version and
> existence constraints can be satisfied, then all updates will be done
> atomically.
> Two API styles have been suggested. One has a list as above and the other
> style has a "Transaction" that allows builder-like methods to build a set of
> updates and a commit method to finalize the transaction. This can trivially
> be reduced to the first kind of API so the list based API style should be
> considered the primitive and the builder style should be implemented as
> syntactic sugar.
> The total size of all the data in all updates and creates in a single
> transaction should be limited to 1MB.
> Implementation-wise this capability can be done using standard ZK internals.
> The changes include:
> - update to ZK clients to all the new call
> - additional wire level request
> - on the server, in the code that converts transactions to idempotent form,
> the code should be slightly extended to convert a list of operations to
> idempotent form.
> - on the client, a down-rev server that rejects the multi-update should be
> detected gracefully and an informative exception should be thrown.
> To facilitate shared development, I have established a github repository at
> https://github.com/tdunning/zookeeper and am happy to extend committer
> status to anyone who agrees to donate their code back to Apache. The final
> patch will be attached to this bug as normal.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
