Adar Dembo has posted comments on this change.

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

Patch Set 11:

File src/kudu/tools/

Line 70: DEFINE_int64(rowset_index, INT64_MAX,
> Err... updated to INT64_MAX.
Why is INT64_MAX a better choice than -1? The advantage of -1 is that it's 
actually printed in the help as "-1", whereas INT64_MAX looks like some huge 
number that a user might think has meaning as a real number.

Line 384:            nullptr, // MetricRegistry
> I can take it out, but it looked like it was dumping some in-memory content
The MRS will be empty if the tablet isn't bootstrapped. So in effect, it's 
still doing the same thing as "dump rowset". Plus it's doing it in a poor way: 
it loads _all_ the data into an in-memory vector, and the data is loaded as 
debug strings, which aren't as useful as how "dump rowset" prints.

PS9, Line 521: }
> Done, doesn't the 4-space indent next-line apply here ?
Generally yes, but sometimes people deviate from that style to "pretty print" 
some stuff (like this), and when they do, I think it's better code etiquette to 
preserve that style, at least on a file-by-file basis.

PS9, Line 607: // Rowset index no
> Heh, somewhere came across in Strostrup book few weeks ago, this way of ini
Primitive types don't have constructors/destructors the way that classes and 
structs do. It's all just raw memory.

PS9, Line 679: 
> Done
Nope, not changed.

PS9, Line 721: 
> Should be 'replica' now.
Looks like you missed this one.
File src/kudu/tools/

PS11, Line 637: tablet

PS11, Line 645: tablet

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Dinesh Bhat <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to