You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2022/12/17 13:19:13 UTC
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19370
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
[tools] Support to dump rowset primary key bounds and in readable format
This patch adds features for CLI tool 'kudu local_replica dump rowset',
including:
1. Dump rowset primary keys in readable format by setting
flag --dump_readable_primary_key.
2. Dump rowset primary key bounds only instead of all rows
by setting flag --dump_primary_key_bounds_only.
Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 85 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/19370/1
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
Patch Set 2: Verified+1
unrelated test failure in KuduTxnsCliTest.TestShowTxnHybridTimestamps
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
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, 28 Dec 2022 03:16:24 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
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, 28 Dec 2022 03:16:28 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
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>
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
Patch Set 1:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/19370/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19370/1//COMMIT_MSG@14
PS1, Line 14: by setting flag --dump_primary_key_bounds_only.
> Could you explain in which situation it is need to dump primary key bound o
There may be hundreds and thousands of rows in the rowset, dump them all is heavy and hard to read. In the case we only want to know the bounds, it's helpful.
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2854
PS1, Line 2854: rowsets primary key
> rowsets' primary keys
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2856
PS1, Line 2856: --nodump_all_columns "
: "--nodump_metadata --dump_readable_primary_key=true
> nit: this looks like a strange mix of gflag notations. Could you update th
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2860
PS1, Line 2860: SCOPED_TRACE(stdout);
> If using SCOPED_TRACE(), then please enclose the sub-scenario in its own sc
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2869
PS1, Line 2869: rowsets primary key bounds
> rowsets' primary key bounds
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2871
PS1, Line 2871: --nodump_all_columns "
: "--nodump_metadata --dump_readable_primary_key=true "
: "--dump_primary_key_bounds_only=true
> nit: ditto for the gflag notations used.
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@107
PS1, Line 107: dump_readable_primary_key
> If that's about the format of the keys, maybe name this flag
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@201
PS1, Line 201: when set --dump_all_columns.
> when --dump_all_columns is enabled
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@207
PS1, Line 207: when set --dump_all_columns.
> when --dump_all_columns is enabled
Done
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@792
PS1, Line 792: string DumpRow(const Schema& key_proj, const RowBlockRow& row, faststring* key) {
> "faststring* key" is not need to be posted from outside. How about defining
Reuse it to reduce memory re-allocate.
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@1033
PS1, Line 1033: if (FLAGS_dump_primary_key_bounds_only) {
> Why this code outside of the loop: Line1018?
Because we will only print the min & max PK, so find them in the loop, and emplace them into the list we will print later.
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
Gerrit-PatchSet: 1
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: Sat, 24 Dec 2022 12:18:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19370/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19370/1//COMMIT_MSG@14
PS1, Line 14: by setting flag --dump_primary_key_bounds_only.
Could you explain in which situation it is need to dump primary key bound only?
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@792
PS1, Line 792: string DumpRow(const Schema& key_proj, const RowBlockRow& row, faststring* key) {
"faststring* key" is not need to be posted from outside. How about defining a local faststring?
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@1033
PS1, Line 1033: if (FLAGS_dump_primary_key_bounds_only) {
Why this code outside of the loop: Line1018?
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
Gerrit-PatchSet: 1
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: Tue, 20 Dec 2022 01:19:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2854
PS1, Line 2854: rowsets primary key
rowsets' primary keys
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2856
PS1, Line 2856: --nodump_all_columns "
: "--nodump_metadata --dump_readable_primary_key=true
nit: this looks like a strange mix of gflag notations. Could you update this to use either '--xxx=<true|false>' or '--[no]xxx' notation for all the specified flags?
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2860
PS1, Line 2860: SCOPED_TRACE(stdout);
If using SCOPED_TRACE(), then please enclose the sub-scenario in its own scope to avoid confusing print-outs of all the scoped traces in the current scope when any assertion in this scope fails (there are 4 of those already in the scope with this patch).
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2869
PS1, Line 2869: rowsets primary key bounds
rowsets' primary key bounds
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2871
PS1, Line 2871: --nodump_all_columns "
: "--nodump_metadata --dump_readable_primary_key=true "
: "--dump_primary_key_bounds_only=true
nit: ditto for the gflag notations used.
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@107
PS1, Line 107: dump_readable_primary_key
If that's about the format of the keys, maybe name this flag
use_readable_format
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@201
PS1, Line 201: when set --dump_all_columns.
when --dump_all_columns is enabled
http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@207
PS1, Line 207: when set --dump_all_columns.
when --dump_all_columns is enabled
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
Gerrit-PatchSet: 1
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: Tue, 20 Dec 2022 21:38:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yuqi Du, Yifan Zhang, Kudu Jenkins, KeDeng, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19370
to look at the new patch set (#2).
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
[tools] Support to dump rowset primary key bounds and in readable format
This patch adds features for CLI tool 'kudu local_replica dump rowset',
including:
1. Dump rowsets' primary keys in readable format by setting
flag --use_readable_format.
2. Dump rowsets' primary key bounds only instead of all rows
by setting flag --dump_primary_key_bounds_only.
Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 122 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/19370/2
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
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>
[kudu-CR] [tools] Support to dump rowset primary key bounds and in readable format
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format
......................................................................
[tools] Support to dump rowset primary key bounds and in readable format
This patch adds features for CLI tool 'kudu local_replica dump rowset',
including:
1. Dump rowsets' primary keys in readable format by setting
flag --use_readable_format.
2. Dump rowsets' primary key bounds only instead of all rows
by setting flag --dump_primary_key_bounds_only.
Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Reviewed-on: http://gerrit.cloudera.org:8080/19370
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
2 files changed, 122 insertions(+), 20 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a
Gerrit-Change-Number: 19370
Gerrit-PatchSet: 3
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>