Yingchun Lai has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19357 )

Change subject: [tools] Add 'kudu local_replica tmeta delete_rowsets' to delete 
rowsets from tablet
......................................................................


Patch Set 3:

(9 comments)

> Patch Set 3:
>
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/19357/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19357/1//COMMIT_MSG@7
PS1, Line 7: edit
> Done
Renamed to tmeta at last, thanks.


http://gerrit.cloudera.org:8080/#/c/19357/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19357/3//COMMIT_MSG@7
PS3, Line 7: edit_metadata
> what about  'edit_tmeta'
Renamed to tmeta, thanks.


http://gerrit.cloudera.org:8080/#/c/19357/3//COMMIT_MSG@11
PS3, Line 11: corruption
> corrupted
Done


http://gerrit.cloudera.org:8080/#/c/19357/3//COMMIT_MSG@11
PS3, Line 11: a single point cluster
> What's 'a single point cluster'?  Probably, you meant 'a single node cluste
Yes, thanks for your correction.


http://gerrit.cloudera.org:8080/#/c/19357/3//COMMIT_MSG@19
PS3, Line 19: introduce
> introduces
Done


http://gerrit.cloudera.org:8080/#/c/19357/3/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/19357/3/src/kudu/tablet/tablet_metadata.cc@76
PS3, Line 76:             "tablet-metadata to roll back. Note: This is only 
used for CLI "
> It seems a little complex.
Good advice. I moved most description to the tool description.


http://gerrit.cloudera.org:8080/#/c/19357/3/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

http://gerrit.cloudera.org:8080/#/c/19357/3/src/kudu/tools/tool_action_local_replica.cc@105
PS3, Line 105: DEFINE_bool(backup_metadata, false,
> It's safer using default 'true' and use 'enable_adding_orphaned_blocks' false 
> ?

--enable_adding_orphaned_blocks should be true by default, it's the safer value 
for servers. But it's conflict when using CLI tools when enable 
--backup_metadata. So set --backup_metadata default value as false. I think 
it's reasonable because backup is just an optional manner.


http://gerrit.cloudera.org:8080/#/c/19357/3/src/kudu/tools/tool_action_local_replica.cc@1250
PS3, Line 1250: edit_matadata
> Could we have a sub-command 'delete_rowsets' for a new 'metadata' or 'tmeta
Renamed to tmeta, thanks.


http://gerrit.cloudera.org:8080/#/c/19357/3/src/kudu/tools/tool_action_local_replica.cc@1250
PS3, Line 1250:       ModeBuilder("edit_matadata")
> What about renaming ’metadata‘ to 'tmeta'
Renamed to tmeta, thanks.



--
To view, visit http://gerrit.cloudera.org:8080/19357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If2cf9035babf4c3af4c238cebe8dcecd2c65848f
Gerrit-Change-Number: 19357
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Wed, 28 Dec 2022 16:18:37 +0000
Gerrit-HasComments: Yes

Reply via email to