Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24978 )
Change subject: KUDU-3811 [tools] add 'kudu cluster location check' ...................................................................... Patch Set 4: (11 comments) http://gerrit.cloudera.org:8080/#/c/24978/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24978/2//COMMIT_MSG@23 PS2, Line 23: . The action exits non-zero : if any location is unsafe or if --locati > This is quite superficial. If all tablets of such a RF<3 table aren't in t Removed both 1 and 2 as we could still take a location down if less than 3 locations are present or if a location doesn't have a single replica whose RF=1. http://gerrit.cloudera.org:8080/#/c/24978/2//COMMIT_MSG@49 PS2, Line 49: : > You mean it doesn't take into account any pending replica replacement/movem I meant it doesn't consider the distribution of Kudu master servers across locations. Updated it for clarity. If there are any tablet copies in progress they will now be noted in the output, something like: [NOTE] 2 tablet(s) have a Raft config change in progress: 4f1c..., 9a07... Replicas are being replaced or added (e.g. by the auto-rebalancer,'kudu cluster rebalance', or re-replication), so the placement checked below is about to change. Re-run once it settles. http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/kudu-tool-test.cc@1437 PS2, Line 1437: ksck", > It's OK for robots, but it is very long string for humans. Consider making Makes sense, it is now kudu cluster location check http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/location_maintenance.cc File src/kudu/tools/location_maintenance.cc: http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/location_maintenance.cc@54 PS2, Line 54: << "' referenced by tablet " << t.id << "; skipping"; > During 3-4-3 reconfig an RF=3 tablet has 4 voters (real majority 3); 2-on-t This is updated to having max(declared_rf, voters_in_config) / 2 + 1. http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc File src/kudu/tools/tool_action_cluster.cc: http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@22 PS2, Line 22: #include <iomanip> : #include <iostream > Please run and fix "make iwyu" on ubuntu 18. Done http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@252 PS2, Line 252: const vector<string> listed( : tablets_in_flux.begin(), : tablets_in_flux.begin() + std::min(kMaxListed, tablets_in_flux.size())); : cout << " [NOTE] " << tablets_in_flux.size() : << " tablet(s) have a Raft config change in progress: " : > Perhaps, we shouldn't fail the verification if there RF=1 tablets at locati Done http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@313 PS2, Line 313: // Without a location there's no telling whether a tablet server would go > If we have any unlocated TServers, does this whole "safe to turn off region Yes, makes sense. Done http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@313 PS2, Line 313: // Without a location there's no telling whether a tablet server would go > "" from un-located tservers is inserted into locations, so a cluster with r Based on the below comments, it would make sense to exit and report about the location unassigned servers. http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@313 PS2, Line 313: // Without a location there's no telling whether a tablet server would go > +1 Yes, done http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@334 PS2, Line 334: "tablet server needs one before a location can be > nit: please use map-util.h wrappers instead Done http://gerrit.cloudera.org:8080/#/c/24978/2/src/kudu/tools/tool_action_cluster.cc@337 PS2, Line 337: JoinStrings(unlocated_tservers, ", "))); : } > JoinStrings over locations renders "" as an empty slot ([, /L0, /L1]); use We now exit earlier above if there is an empty location -- To view, visit http://gerrit.cloudera.org:8080/24978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd4cbaa8dab835d4d27270c06b4b0028aab8285b Gerrit-Change-Number: 24978 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Tue, 13 Oct 2026 18:55:30 +0000 Gerrit-HasComments: Yes
