Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18029 )
Change subject: [tools] KUDU-3337 Add unsafe_create_cmeta tool ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/18029/6/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/18029/6/src/kudu/tools/kudu-tool-test.cc@18 PS6, Line 18: #include <limits.h> nit: replace with <climits> C++ header file http://gerrit.cloudera.org:8080/#/c/18029/3/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: http://gerrit.cloudera.org:8080/#/c/18029/3/src/kudu/tools/tool_action_local_replica.cc@675 PS3, Line 675: > If the tablet in question was a non-voter, it wouldn't affect consensus, an I guess it's possible to have a situation when there was still a majority of replicas when a non-voter replica had been added and tablet copying started, but during the catch-up phase the majority wasn't present anymore. This situation could persist for some time: such a non-voter replica would be evicted, but only if there is a majority to update Raft config. Even such a situation is of rare occurrence, I was curious whether we target this tool to help in such cases as well. Say, the only alive replicas out of three are: (1) a voter (2) a non-voter stuck in such limbo. And this particular former replica was a voter, but now its Raft metadata file is corrupted, and there is an attempt to fix the issue. It's totally fine with me if such a situation is out of scope for this patch, and it might be a follow-up one to address such a case separately. http://gerrit.cloudera.org:8080/#/c/18029/3/src/kudu/tools/tool_action_local_replica.cc@678 PS3, Line 678: Status UnsafeRecreateCmeta(const RunnerContext& context) { : const string& tablet_id = FindOrDie(context.required_args, kTabletIdArg); : int64_t term; : int64_t opid_index; : try { > Does it make sense to add an extra check for the arguments provided w.r.t. It seems this comment isn't addressed as of PS6. Or the idea is to address it in a follow-up PS or even in a separate changelist? -- To view, visit http://gerrit.cloudera.org:8080/18029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I136cc5b5797420a9ca9156f37c3e281da0c265d7 Gerrit-Change-Number: 18029 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Mon, 06 Nov 2023 20:50:46 +0000 Gerrit-HasComments: Yes
