[ https://issues.apache.org/jira/browse/ZOOKEEPER-965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13032575#comment-13032575 ]
Stephen Tyree commented on ZOOKEEPER-965: ----------------------------------------- Great job all around! Some comments on the C library portion of the patch: - The zoo_acheck comment includes documentation on a member 'stat' that is not an argument - The zoo_check comment has the parameter stat listed as _stat - In create_completion_entry, you commented out the else case where you set c->clist to NULL. Did you mean to leave this in there? - In op_result_string_completion, you memcpy from the value onto data->value based on the size of value. Is data->value guaranteed to be larger than value? - You only assert result in op_result_string_completion, but none of the other op_result_* functions - Do you not need to set result->err in op_result_multi_completion? - In zoo_amulti, any reason "int index = 0;" isn't a few lines down right before it's used? - Slick zoo_amulti code, very nicely done. - In testCreate you compare results[*].err to 0, which will work, but shouldn't you compare it to ZOK? Or do I misunderstand op_result_t. - In testDelete, your comment should say "// '/multi2' should have been deleted" All-in-all the code looks great in the C client. Nicely done. > 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 > > > 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