Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16527 )

Change subject: KUDU-2612: initial implementation of TxnManager
......................................................................


Patch Set 7: Code-Review+2

(6 comments)

http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc
File src/kudu/master/master.cc:

http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc@103
PS5, Line 103: ecurity::TokenSigner;
> I guess for real world scenarios in clusters which use transactions we shou
Ack


http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc@319
PS5, Line 319: //                wake up a long-awaiting task and retry 
initialization
             :     //                right away when TxnManager receives a call.
             :     static const MonoDelta kRetryInterval = 
MonoDelta::FromSeconds(1);
             :     KLOG_EVERY_N_SECS(WARNING
> TxnManager cannot complete initialization without tablet servers which host
Ack


http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.h
File src/kudu/master/txn_manager.h:

http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.h@102
PS5, Line 102: // Whether or not this instance is lazily initializ
> There is nothing specific about this, it's just a random number so far.  I
Ack


http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc
File src/kudu/master/txn_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc@49
PS5, Line 49: For Kudu clusters which use transactions we should prefer non-laz
> It happens in case of RPC-level errors.
Ack


http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc@127
PS5, Line 127:                                      const string& username,
             :                                      co
> The header has similar comment as well.  I put this comment here as well fo
Ack


http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.proto
File src/kudu/master/txn_manager.proto:

http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.proto@94
PS5, Line 94: ime
> I replaced the whole sentence with 'The transaction state at the time of pr
Ack



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie952977a3ae5f625d1283389f0be8afb79df7d8c
Gerrit-Change-Number: 16527
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <aser...@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: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 14 Oct 2020 18:13:20 +0000
Gerrit-HasComments: Yes

Reply via email to