Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16321 )
Change subject: [master] KUDU-2181 Raft ChangeConfig request to add a master ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/dynamic_multi_master-test.cc@258 PS5, Line 258: *req.mutabl > Done Thanks! Much clearer this time around http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@162 PS6, Line 162: } Nit: Would it make sense to define a deadline instead and keep retrying until then? http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@266 PS6, Line 266: return RunLeaderMasterRPC(add_master).AndThen([&] { Nit: AndThen is useful when we need to do something extra with the result of the first error we see (eg check for specific errors). If we're just returning the error, it's simpler to just use RETURN_NOT_OK() and return. http://gerrit.cloudera.org:8080/#/c/16321/6/src/kudu/master/dynamic_multi_master-test.cc@399 PS6, Line 399: } The log_gc_duration histogram metric has a TotalCount() method that might be useful. I use it here: https://gerrit.cloudera.org/c/16492/3/src/kudu/integration-tests/txn_participant-itest.cc http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc File src/kudu/master/master.cc: http://gerrit.cloudera.org:8080/#/c/16321/5/src/kudu/master/master.cc@424 PS5, Line 424: auto > nit: could be a const ref Missed this? -- To view, visit http://gerrit.cloudera.org:8080/16321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ac7e6e55220bcb01cad0fa386daaf656258088c Gerrit-Change-Number: 16321 Gerrit-PatchSet: 6 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Sep 2020 19:46:56 +0000 Gerrit-HasComments: Yes
