Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9510 )
Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/9510/3/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/9510/3/src/kudu/tools/kudu-admin-test.cc@327 PS3, Line 327: void AdminCliTest::DoTestMoveTabletKUDU2331(EnableKudu1097 enable_kudu_1097) { It's a nice test. Does it make sense to add the 'negative' scenario as well (i.e. when a tablet server hosting one of the replicas is down)? http://gerrit.cloudera.org:8080/#/c/9510/3/src/kudu/tools/kudu-admin-test.cc@386 PS3, Line 386: // Skip the ClusterVerifier since one tablet server is down. maybe, add NO_FATALS(cluster_->AssertNoCrashes()); http://gerrit.cloudera.org:8080/#/c/9510/3/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: http://gerrit.cloudera.org:8080/#/c/9510/3/src/kudu/tools/tool_action_tablet.cc@154 PS3, Line 154: ignore_result(ksck.FetchInfoFromTabletServers()); Did you consider the approach of 'applying' the 'tablet_id_filters' for FetchInfoFromTabletServers()? -- To view, visit http://gerrit.cloudera.org:8080/9510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Gerrit-Change-Number: 9510 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Wed, 07 Mar 2018 15:39:01 +0000 Gerrit-HasComments: Yes
