Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18029 )
Change subject: [tools] KUDU-3337 Add unsafe_create_cmeta tool ...................................................................... Patch Set 1: (4 comments) Not sure if the goal is to merge this or just stage it here for posterity, but gave it a quick look http://gerrit.cloudera.org:8080/#/c/18029/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18029/1//COMMIT_MSG@21 PS1, Line 21: m nit: remove http://gerrit.cloudera.org:8080/#/c/18029/1//COMMIT_MSG@23 PS1, Line 23: : I manually tested this tool by using it to recover a tablet with three : empty cmeta files. If you intend on merging this, it'd be good to at least add couple simple tests to ensure we get the expected behavior in both happy and non-happy cases. http://gerrit.cloudera.org:8080/#/c/18029/1/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: http://gerrit.cloudera.org:8080/#/c/18029/1/src/kudu/tools/tool_action_local_replica.cc@400 PS1, Line 400: pair<string, HostPort> parsed_peer; nit: seems this is never used as a pair, so why bother making it a pair, instead of having separate string and HostPort variables? http://gerrit.cloudera.org:8080/#/c/18029/1/src/kudu/tools/tool_action_local_replica.cc@416 PS1, Line 416: scoped_refptr<ConsensusMetadataManager> cmeta_manager( Can this be stack-allocated? -- 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: 1 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 23 Nov 2021 18:53:25 +0000 Gerrit-HasComments: Yes
