Alexey Serbin has posted comments on this change.

Change subject: tool: add a 'local-replica cmeta set-term' tool
......................................................................


Patch Set 2:

(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 since 
the target term is supposed to be X+1, where X >= 0?


Line 305:         "specified term $0 must be higher than current term $1",
nit: than _the_ current term ?


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 from 
the backup file?  Or, maybe, add an extra  verification step into 
BackupConsensusMetadata() just to verify the backup file can be loaded after 
the backup is finished.

The concern is that if the source file is being updated, the generated backup 
might be inconsistent.


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

Reply via email to