Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16710 )

Change subject: WIP KUDU-2612 C++ client transaction API
......................................................................


Patch Set 5:

(3 comments)

Overall looks good -- I agree with the interface so far, and the proxy bits 
make sense (minus the pending TODOs).

http://gerrit.cloudera.org:8080/#/c/16710/5/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

http://gerrit.cloudera.org:8080/#/c/16710/5/src/kudu/client/client-internal.h@311
PS5, Line 311: swith
nit: switch


http://gerrit.cloudera.org:8080/#/c/16710/5/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/16710/5/src/kudu/client/client.h@402
PS5, Line 402:   // TODO(aserbin): do we need GetStatus()?
I recall briefly discussing maybe some sort of IsCommitComplete(), or similar. 
I think I'd prefer more a more pointed API like that, over defining a 
client-facing transaction status. Don't feel too strongly about it yet though.


http://gerrit.cloudera.org:8080/#/c/16710/5/src/kudu/common/txn_id.h
File src/kudu/common/txn_id.h:

http://gerrit.cloudera.org:8080/#/c/16710/5/src/kudu/common/txn_id.h@51
PS5, Line 51: int64_t
I might be missing something -- why this change?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic48233f72873012ea36ff4a05d16c58a0ba9b407
Gerrit-Change-Number: 16710
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 13 Nov 2020 06:16:42 +0000
Gerrit-HasComments: Yes

Reply via email to