Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16814 )

Change subject: [tserver] KUDU-2612: participant op RPC endpoint
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16814/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16814/1//COMMIT_MSG@9
PS1, Line 9: This adds an RPC endpoint to the tablet servers that allows 
proxies to
           : interact with transaction participants.
Could you add more colors on this, adding an example of a scenario when and how 
this is going to be used.  Adding a reference to a particular piece (or step) 
in the txn design doc might be a good thing to add, I guess.


http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/integration-tests/txn_participant-itest.cc@521
PS1, Line 521: not RUNNING
BTW, what is the expected behavior what a tablet is bootstrapping?  Is it the 
same as in case of tombstoned tablet or it should be something else?


http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/integration-tests/txn_participant-itest.cc@529
PS1, Line 529: dummy-tablet-id
BTW, would we allow status tablet to participate in a transaction?  Not sure it 
would contradict to anything, but from the other side I'm not sure we should 
allow that.  Maybe you have already thought about that and know the answer :)


http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/tserver/tserver_admin.proto
File src/kudu/tserver/tserver_admin.proto:

http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/tserver/tserver_admin.proto@113
PS1, Line 113: message ParticipantRequestPB {
             :   optional bytes tablet_id = 1;
             :   optional ParticipantOpPB op = 2;
             : }
Just want to double-check it's safe to swap the fields.  I guess it's so 
because this message hasn't been sent over wire yet, even if the changelist 
that introduced ParticipantRequestPB is already in released bits:

$ git tag -l --contains 14d4117afa
1.13.0
1.13.0-RC1
1.13.0-RC2

Is it correct?


http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/tserver/tserver_admin.proto@256
PS1, Line 256: Have a tablet participate in a transaction
It would nice if the comment were a bit more clearer w.r.t. what is the 
semantics of this method from the callee side, i.e. what 'have' here means?



--
To view, visit http://gerrit.cloudera.org:8080/16814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b
Gerrit-Change-Number: 16814
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 07 Dec 2020 06:47:40 +0000
Gerrit-HasComments: Yes

Reply via email to