Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18029 )

Change subject: [tools] KUDU-3337 Add unsafe_create_cmeta tool
......................................................................


Patch Set 2:

(5 comments)

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: ,
> nit: remove
Done


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 t
Done


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@389
PS1, Line 389:   int64_t term = stoi(FindOrDie(context.required_args, "term"));
             :   int64_t opid_index = stoi(FindOrDie(context.required_args, 
"index"));
> Since these both are integers which can be easily interchanged erroneously,
Index doesn't necessarily need to be higher or equal to term as elections don't 
bump the index.


http://gerrit.cloudera.org:8080/#/c/18029/1/src/kudu/tools/tool_action_local_replica.cc@400
PS1, Line 400:     string uuid;
> nit: seems this is never used as a pair, so why bother making it a pair, in
Done


http://gerrit.cloudera.org:8080/#/c/18029/1/src/kudu/tools/tool_action_local_replica.cc@416
PS1, Line 416: // Write the cmeta file.
> Can this be stack-allocated?
I don't think so, this is how it's used everywhere. If I try to stack-allocate 
it, it complains about Release() not being called when deleting.



--
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: 2
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[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: Fri, 07 Jan 2022 18:35:48 +0000
Gerrit-HasComments: Yes

Reply via email to