Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10612 )
Change subject: [rebalancer] 'auto' mode for RF=1 tablet movements ...................................................................... Patch Set 11: (6 comments) http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/kudu-admin-test.cc@1510 PS11, Line 1510: const auto& param = GetParam(); : const auto& move_single_replica = std::get<0>(param); : const auto is_343_scheme = (std::get<1>(param) == Kudu1097::Enable); : constexpr auto kRepFactor = 1; : constexpr auto kNumTservers = 2 * (kRepFactor + 1); : constexpr auto kNumTables = kNumTservers; : const string table_name_pattern = "rebalance_test_table_$0"; : constexpr auto kTserverUnresponsiveMs = 3000; : const vector<string> kMasterFlags = { : Substitute("--raft_prepare_replacement_before_eviction=$0", is_343_scheme), : Substitute("--tserver_unresponsive_timeout_ms=$0", kTserverUnresponsiveMs), : }; : const vector<string> kTserverFlags = { : Substitute("--raft_prepare_replacement_before_eviction=$0", is_343_scheme), : }; Nit: can you sort this a bit? 1. constexpr stuff 2. const foo kBar stuff 3. const auto under_score_stuff Maybe like that? http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/kudu-admin-test.cc@1560 PS11, Line 1560: (kRepFactor + 1) Nit: don't need parens. http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/kudu-tool-test.cc@3364 PS11, Line 3364: signle Shouldn't this be 'single'? http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/tool_replica_util.h File src/kudu/tools/tool_replica_util.h: http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/tool_replica_util.h@132 PS11, Line 132: no information available to find on the replica management scheme : // used "not enough information available to determine the replica management scheme" http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/tool_replica_util.cc File src/kudu/tools/tool_replica_util.cc: http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/tool_replica_util.cc@524 PS11, Line 524: signle typo here http://gerrit.cloudera.org:8080/#/c/10612/11/src/kudu/tools/tool_replica_util.cc@539 PS11, Line 539: if (tablet_id.empty()) { : return Status::Incomplete(Substitute( : "table '$0': empty tablet identifier for one of the tablets", : table_name)); : } Is this even possible? -- To view, visit http://gerrit.cloudera.org:8080/10612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id5a06b137cb34d9351f7e2fb819ed52790e2ee7e Gerrit-Change-Number: 10612 Gerrit-PatchSet: 11 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Tue, 03 Jul 2018 22:51:08 +0000 Gerrit-HasComments: Yes
