Will Berkeley 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 wel It's pretty clear it won't work if the source or destination servers are down, but it would be good to have a test that if a different replica is down then the move fails. 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 Done 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 Fet Yes. It would complicate that code quite a bit. Even though a filter would let us do less work by only going to the tablet servers the master thinks host a replica of the tablet, in the general case for ksck we want to check every tablet server every time since an important part of ksck is checking for inconsistencies between master and tablet servers, so we don't want to rely just on what the master tells us. -- 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 17:42:57 +0000 Gerrit-HasComments: Yes