Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16527 )
Change subject: KUDU-2612: initial implementation of TxnManager ...................................................................... Patch Set 5: (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: Whether to initialize TxnManager upon arrival of first request. Wondering how to decide if TxnManager should be initialized upon arrival of first request or the other? http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/master.cc@319 PS5, Line 319: KLOG_EVERY_N_SECS(WARNING, 60) << Substitute( : "$0: unable to init TxnManager, will retry in $1", : s.ToString(), retry_interval.ToString()); : SleepFor(retry_interval); Why do we want to retry the initialization, but not as what catalog manager is doing which don't retry? 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: static constexpr int64_t kTxnTableRangeStep = 1024; Can you add a comment to explain why 1024 is chosen? 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: If 's' is not OK and 'resp' has no application specific error set Do you know in which cases this will happen? http://gerrit.cloudera.org:8080/#/c/16527/5/src/kudu/master/txn_manager.cc@127 PS5, Line 127: // This method isn't supposed to be called concurrently, so there isn't any : // protection against concurrent calls. Should this be moved to the method declaration of the header. 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: was nit: was OK? -- 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: 5 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: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 13 Oct 2020 18:59:56 +0000 Gerrit-HasComments: Yes
