Mike Percy has posted comments on this change. Change subject: tool: add a 'local-replica cmeta set-term' tool ......................................................................
Patch Set 2: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/7049/2/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: PS2, Line 291: = > I would expect term 0 were allowed. Or it's some sort of optimization sinc Yeah, the lowest term is 0 and you can't set the term <= the current term, so 0 shouldn't be allowed in any case. Line 305: "specified term $0 must be higher than current term $1", > nit: than _the_ current term ? Interesting. It sounds ok to me as-is in the context of a computer error message but in spoken language (or even in a comment) it would sound wrong. I think in this context things like definite articles are sometimes clipped in order to make the message shorter. That said, I could go either way on this. PS2, Line 309: // Make a copy of the old file before rewriting it. : RETURN_NOT_OK(BackupConsensusMetadata(&fs_manager, tablet_id)); > Would it be safer to first backup the existing metadata and load the cmeta We have an FsManager lock from L297-L298 so nobody else should be messing with the files. If we didn't have that lock, this would definitely not be safe to do. -- To view, visit http://gerrit.cloudera.org:8080/7049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7525ffbe772f214e0972a6b450f3f1609109ca05 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
