[kudu-CR] [tools] Limit to count of rows to dump accurately

["Alexey Serbin (Code Review)" <ge...@cloudera.org>]

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19369 )

Change subject: [tools] Limit to count of rows to dump accurately
......................................................................


Patch Set 2:

(11 comments)

Thank you for the fix!

Just nits, overall looks good.

http://gerrit.cloudera.org:8080/#/c/19369/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19369/2//COMMIT_MSG@7
PS2, Line 7: Limit to count of rows to dump accurately
limit number of rows when dumping a rowset


http://gerrit.cloudera.org:8080/#/c/19369/2//COMMIT_MSG@10
PS2, Line 10: if not set --dump_all_columns
if --dump_all_columns is not enabled


http://gerrit.cloudera.org:8080/#/c/19369/2//COMMIT_MSG@12
PS2, Line 12: This patch fix the issue and early break when row count
            : reach --nrows.
This patch fixes the issue, so now the 'local_replica dump rowset' CLI tool dumps no more than the specified number of rows.


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/compaction.h
File src/kudu/tablet/compaction.h:

http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/compaction.h@20
PS2, Line 20: 
nit: remove this empty extra line?


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/compaction.h@261
PS2, Line 261: This consumes partial or all of the input in the compaction input, depends
             : // on 'rows_left'.
This consumes no more rows from the compaction input than specified by the 'rows_left' parameter.


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/compaction.h@263
PS2, Line 263: rows_left
Is negative value has the 'no limit' semantics in here?  It would be great to document this in the comment above.


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/memrowset.h
File src/kudu/tablet/memrowset.h:

http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/memrowset.h@381
PS2, Line 381:   // Dump the contents of the memrowset to the given vector.
             :   // If 'lines' is NULL, dumps to LOG(INFO).
nit: update the comment to document the newly added parameter


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/rowset.h
File src/kudu/tablet/rowset.h:

http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/rowset.h@a180
PS2, Line 180: 
Just curious why not to keep this parameter by default?


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tablet/rowset.h@180
PS2, Line 180: int64_t* rows_left
Would be great to document this parameter.  Does a negative value for *nrows_left has the 'no limit' semantics here?


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

http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tools/tool_action_local_replica.cc@987
PS2, Line 987: print
printed


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tools/tool_action_local_replica.cc@987
PS2, Line 987: print
printed



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia758ba910fccbbc06ac6c59a795574fb86d4e279
Gerrit-Change-Number: 19369
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 06:34:29 +0000
Gerrit-HasComments: Yes