Dinesh Bhat has posted comments on this change.

Change subject: tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool
......................................................................


Patch Set 11:

(6 comments)

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

Line 70: DEFINE_int64(rowset_index, INT64_MAX,
> Why is INT64_MAX a better choice than -1? The advantage of -1 is that it's 
Absence of this flags on cmd-line will mean -1, which means even if we type 
'--rowset_index=-1' by mistake we end up printing everything. I preferred 
INT64_MAX over that. Given that this is meant for adv users, INT64_MAX kinda 
felt intuitive. I changed it back to (-1) anyways because I wasn't against 
either approach.


Line 384:            nullptr, // MetricRegistry
> The MRS will be empty if the tablet isn't bootstrapped. So in effect, it's 
I see, ok removed now. removed fomr test too.


PS9, Line 679: 
> Nope, not changed.
yikes !! sorry, actually it's kinda odd that all our required params are 
'tablet_id' and help strings are not referring to tablet anymore :). But, 
laters may be. Also do we not want to honor 80 char wrap-ups ?


PS9, Line 721: 
> Looks like you missed this one.
Done


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

PS11, Line 637: tablet
> replica
Done


PS11, Line 645: tablet
> replica
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to