You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Wang Xixu (Code Review)" <ge...@cloudera.org> on 2022/06/09 03:21:53 UTC
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Wang Xixu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18604
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copy a table to another cluster, we can create the same table
in the destination cluster. But we can not config the number of hash bucket
in the new table. Why we need to config hash bucket number? the old table
may be configured with small number of hash buckets, but stored with large
of data in it. When copy the old table to new cluster, we want to add more
hash buckets to store large of data. And there no method to change the number
of hash bucket when the table has already been created.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 20 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/1
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@9
PS1, Line 9: copy
copying
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@11
PS1, Line 11: the
The
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@13
PS1, Line 13: copy
copying
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@16
PS1, Line 16: Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
nit: add an empty line to separate the Change-Id from the commit description
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc@95
PS1, Line 95: The hash bucket factor of the destination table
What about multiple hash dimensions in case of multi-level partitioning (https://kudu.apache.org/docs/schema_design.html#multilevel-partitioning)? See https://kudu.apache.org/docs/schema_design.html#hash-hash-partitioning-example for an example.
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/tool_action_table.cc@136
PS1, Line 136: FLAGS_create_table_hash_bucket_factor
The declaration should use the name of the flag, i.e. 'create_table_hash_bucket_factor' in this case. FLAGS_xxx is a way to reference to the flag in the code.
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 10 Jun 2022 15:00:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#10).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 131 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/10
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#12).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 137 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/12
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 13:
(1 comment)
> Patch Set 12:
>
> (1 comment)
>
> Thank you for revving this patch.
>
> One more question regarding error handling, but otherwise the patch looks good enough to me.
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@435
PS10, Line 435: if (!partition_schema.hash_schema().empty())
> Probably, I should have articulated my question better. I'll try one more
Ok, I got it. It is done, see Line 466
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Jul 2022 06:59:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 9:
(1 comment)
> Patch Set 6:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@450
PS6, Line 450: for (int i = 0; i < hash_bucket_nums_str.size(); i++) {
: int bucket_num = 0;
: bool is_number = safe_strto32(hash_bucket_nums_str[i], &bucket_num);
: if (!is_number) {
: return Status::InvalidArgument("Hash bucket number must be digit!");
: }
: if (bucket_num < 2) {
: return Status::InvalidArgument("Hash bucket number must be >= 2!");
: }
:
> +1
Done
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 9
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 12:13:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 13: Verified+1
(1 comment)
unrelated test failures in log-rolling-itest (ASAN)
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@435
PS10, Line 435: if (!partition_schema.hash_schema().empty())
> Ok, I got it. It is done, see Line 466
Great, thanks a lot for addressing this!
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Jul 2022 18:14:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#11).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 136 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/11
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#9).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 131 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/9
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 9
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 6:
(3 comments)
> Patch Set 5:
>
> (3 comments)
http://gerrit.cloudera.org:8080/#/c/18604/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18604/5//COMMIT_MSG@9
PS5, Line 9: table,
> nit: should be 'table', we can copy table to the same cluster.
Done
http://gerrit.cloudera.org:8080/#/c/18604/5//COMMIT_MSG@9
PS5, Line 9: e table
: with the same schema. But
> nit: the table with the same schema.
Done
http://gerrit.cloudera.org:8080/#/c/18604/5/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/5/src/kudu/tools/table_scanner.cc@443
PS5, Line 443: if (!is_number) {
> nit: better to add brackets even if there is only one line.
Done
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 03:01:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/kudu-tool-test.cc@817
PS1, Line 817: args.create_table_hash_bucket_factor = 10;
should test both -1 and non -1
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc@94
PS1, Line 94: factor
nit: how about use 'number' instead of 'factor' to describe hash_dimension?
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 09 Jun 2022 07:06:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@450
PS6, Line 450: }
: }
: }
: int i = 0;
: for (const auto& hash_dimension : partition_schema.hash_schema()) {
: int num_buckets = hash_bucket_nums[i] != -1 ? hash_bucket_nums[i] :
: hash_dimension.num_buckets;
: auto hash_columns = convert_column_ids_to_names(hash_dimension.column_ids);
: table_creator->add_hash_partitions(hash_columns,
:
> The size of vector<int> hash_bucket_nums will exactly be equal to the size
Would it better to specify all the hash dimensions? The number of hash dimensions would not be too large in the real world, may be 5 hash dimensions is large enough. Ignore redundant numbers or add default -1 may introduce mistaken errors.
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 02:49:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@450
PS6, Line 450: int i = 0;
: for (const auto& hash_dimension : partition_schema.hash_schema()) {
: int num_buckets = hash_bucket_nums[i] != -1 ? hash_bucket_nums[i] :
: hash_dimension.num_buckets;
: auto hash_columns = convert_column_ids_to_names(hash_dimension.column_ids);
: table_creator->add_hash_partitions(hash_columns,
: num_buckets,
: hash_dimension.seed);
: i++;
: }
> Would it better to specify all the hash dimensions? The number of hash dime
+1
From the usability standpoint, I'd prefer the tool to notify me on non-expected number of dimensions (i.e. don't ignore the extra dimensions, but report an error instead). Also, I agree with Yingchun that once somebody specified the flag, it's not hard to specify the number of hash buckets for each dimension. That way it's more predictable and less error prone, IMO.
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 04:25:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Reviewed-on: http://gerrit.cloudera.org:8080/18604
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/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 155 insertions(+), 9 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 14
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 7:
(9 comments)
> Patch Set 6:
>
> (9 comments)
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@10
PS6, Line 10: config
> configure
Done
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@12
PS6, Line 12: mig
> might
Done
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@12
PS6, Line 12: t contained a lot
: of data in it
> contained a lot of data
Done
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@13
PS6, Line 13: the table to
> the table
Done
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@14
PS6, Line 14: to store. And there is
> drop this part
Done
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@14
PS6, Line 14: t a way to change the number of hash
: buckets in the partition schema of an already existing
> And there isn't a way to change the number of hush buckets in the partition
Done
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@433
PS6, Line 433: if (!partition_schema.hash_schema().empty()) {
: vector<string> hash_
> nit: consider moving these variables where they belong -- inside the 'for()
Done
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@437
PS6, Line 437: int bucket_num = 0;
> nit: misaligned indent
Done
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@450
PS6, Line 450: }
: int i = 0;
: for (const auto& hash_dimension : partition_schema.hash_schema()) {
: int num_buckets = hash_bucket_nums[i] != -1 ? hash_bucket_nums[i] :
: hash_dimension.num_buckets;
: auto hash_columns = convert_column_ids_to_names(hash_dimension.column_ids);
: table_creator->add_hash_partitions(hash_columns,
: num_buckets,
: hash_dimension.seed);
:
> What if the number of hash buckets specified in the command line doesn't ma
The size of vector<int> hash_bucket_nums will exactly be equal to the size of partition_schema.hash_schema(). If the number of hash buckets in the command line is less than the number of hash dimensions, it will be set to -1 default. If it is larger than the number of hash dimension in the table, it will be ignored.
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 08:28:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 9: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18604/9/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/9/src/kudu/tools/table_scanner.cc@98
PS9, Line 98: "The number of hash buckets for every hash keys seperated by comma");
nit: alignment
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 9
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 14:48:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#13).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 155 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/13
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 12:
(1 comment)
Thank you for revving this patch.
One more question regarding error handling, but otherwise the patch looks good enough to me.
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@435
PS10, Line 435: if (!partition_schema.hash_schema().empty())
> --create_table_hash_bucket_nums is "" default, if it is not specified. See
Probably, I should have articulated my question better. I'll try one more time :)
My point here is: what if --create_table_hash_bucket_nums=10 specified when running the tool, but the table has no hash schema defined (i.e. partition_schema.hash_schema().empty() is true). I'd expect the tool to report on this: the user indicated that they thought the table had some hash schema and expected the hash bucket number would change accordingly. IIUC, the current code would ignore the setting and does nothing with that, but the user would be under impression that their setting had some effect, but that's not the case.
What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 12
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Jul 2022 04:16:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18604/4/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/4/src/kudu/tools/table_scanner.cc@434
PS4, Line 434: if (partition_schema.hash_schema().size() > 0) {
nit: use !xxx.empty()
http://gerrit.cloudera.org:8080/#/c/18604/4/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/18604/4/src/kudu/tools/tool_action_common.cc@924
PS4, Line 924: bool isNumber(const string& str) {
Is there any function in src/kudu/gutil/strings/numbers.h would help?
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 17 Jun 2022 09:33:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#6).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not config the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
may be configured with small number of hash buckets, but stored with large
of data in it. When copying the old table to new cluster, we want to add more
hash buckets to store large of data. And there no method to change the number
of hash bucket when the table has already been created.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 84 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/6
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 03:07:45 +0000
Gerrit-HasComments: No
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#2).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another cluster, we can create the same table
in the destination cluster. But we can not config the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
may be configured with small number of hash buckets, but stored with large
of data in it. When copying the old table to new cluster, we want to add more
hash buckets to store large of data. And there no method to change the number
of hash bucket when the table has already been created.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
5 files changed, 61 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/2
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 2
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 13
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Jul 2022 18:14:38 +0000
Gerrit-HasComments: No
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#7).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another table, we can create the table
with the same schema. But we can not configure the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
might be configured with small number of hash buckets, but contained a lot
of data in it. When copying the table to new cluster, we want to add more
hash buckets to store. And there isn't a way to change the number of hash
buckets in the partition schema of an already existing table.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 85 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/7
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 7
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 6:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@10
PS6, Line 10: config
configure
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@12
PS6, Line 12: may
might
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@13
PS6, Line 13: the old table
the table
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@12
PS6, Line 12: stored with large
: of data in it
contained a lot of data
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@14
PS6, Line 14: to store large of data
drop this part
http://gerrit.cloudera.org:8080/#/c/18604/6//COMMIT_MSG@14
PS6, Line 14: And there no method to change the number
: of hash bucket when the table has already been created
And there isn't a way to change the number of hush buckets in the partition schema of an already existing table.
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@433
PS6, Line 433: int bucket_num = 0;
: bool is_number = true;
nit: consider moving these variables where they belong -- inside the 'for()' cycle below.
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@437
PS6, Line 437:
nit: misaligned indent
http://gerrit.cloudera.org:8080/#/c/18604/6/src/kudu/tools/table_scanner.cc@450
PS6, Line 450: int i = 0;
: for (const auto& hash_dimension : partition_schema.hash_schema()) {
: int num_buckets = hash_bucket_nums[i] != -1 ? hash_bucket_nums[i] :
: hash_dimension.num_buckets;
: auto hash_columns = convert_column_ids_to_names(hash_dimension.column_ids);
: table_creator->add_hash_partitions(hash_columns,
: num_buckets,
: hash_dimension.seed);
: i++;
: }
What if the number of hash buckets specified in the command line doesn't match the number of hash dimensions in the table? Say, the number of the specified dimensions in the command line is greater than the actual number of hash buckets? Should there be an error reported?
Also, it would be great to add check for the number of hash buckets specified: it must be greater or equal than 2.
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 14:24:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 11:
(8 comments)
> Patch Set 10:
>
> (8 comments)
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@98
PS10, Line 98: mens
> dimension
Done
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@98
PS10, Line 98: in each h
> in each
Done
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@435
PS10, Line 435: if (!partition_schema.hash_schema().empty())
> What if the --create_table_hash_bucket_nums flag is specified for table wit
--create_table_hash_bucket_nums is "" default, if it is not specified. See Line 97. It is difficult to distinguish with the case of specifying --create_table_hash_bucket_nums == "".
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@444
PS10, Line 444: // If the --create_table_hash_bucket_nums flag is set, the number
: // of comma-separated elements must be equal to t
> nit: how about
Done
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@447
PS10, Line 447: The count of hash bucket numbers must be equal to the "
: "number of hash schema di
> how about: count of hash bucket numbers must be equal to the number of hash
Done
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@454
PS10, Line 454: se th
> Well, it seems safe_strto32 can accept numbers if hexadecimal notation as w
Done
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@454
PS10, Line 454: e
> nit here and everywhere: there no need for exclamation marks in error messa
Done
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@457
PS10, Line 457:
> how about:
Done
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 11
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Jul 2022 02:49:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#4).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another cluster, we can create the same table
in the destination cluster. But we can not config the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
may be configured with small number of hash buckets, but stored with large
of data in it. When copying the old table to new cluster, we want to add more
hash buckets to store large of data. And there no method to change the number
of hash bucket when the table has already been created.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
5 files changed, 94 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/4
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@9
PS1, Line 9: copy
> copying
Done
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@11
PS1, Line 11: The
> The
Done
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@13
PS1, Line 13: copy
> copying
Done
http://gerrit.cloudera.org:8080/#/c/18604/1//COMMIT_MSG@16
PS1, Line 16:
> nit: add an empty line to separate the Change-Id from the commit descriptio
Done
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc@94
PS1, Line 94: f the
> nit: how about use 'number' instead of 'factor' to describe hash_dimension?
Done
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/table_scanner.cc@95
PS1, Line 95: By default, the replication factor of source ta
> What about multiple hash dimensions in case of multi-level partitioning (ht
Use FLAG_create_table_hash_bucket_nums = "10,20" in stead. One integer represents one dimension of hash partition
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/18604/1/src/kudu/tools/tool_action_table.cc@136
PS1, Line 136: ow_count_only);
> The declaration should use the name of the flag, i.e. 'create_table_hash_bu
Done
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 4
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 14 Jun 2022 07:57:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#5).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another cluster, we can create the same table
in the destination cluster. But we can not config the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
may be configured with small number of hash buckets, but stored with large
of data in it. When copying the old table to new cluster, we want to add more
hash buckets to store large of data. And there no method to change the number
of hash bucket when the table has already been created.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_table.cc
3 files changed, 83 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/5
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 10:
(1 comment)
> Patch Set 9: Code-Review+1
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/18604/9/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/9/src/kudu/tools/table_scanner.cc@98
PS9, Line 98: "The number of hash buckets for every hash keys seperated by comma");
> nit: alignment
Done
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 01:38:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 10: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 02:28:01 +0000
Gerrit-HasComments: No
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18604/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18604/5//COMMIT_MSG@9
PS5, Line 9: cluster
nit: should be 'table', we can copy table to the same cluster.
http://gerrit.cloudera.org:8080/#/c/18604/5//COMMIT_MSG@9
PS5, Line 9: the same table
: in the destination cluster
nit: the table with the same schema.
http://gerrit.cloudera.org:8080/#/c/18604/5/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/5/src/kudu/tools/table_scanner.cc@443
PS5, Line 443: if (!is_number)
nit: better to add brackets even if there is only one line.
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 24 Jun 2022 10:31:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18604 )
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
Patch Set 10:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@98
PS10, Line 98: keys
dimension
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@98
PS10, Line 98: for every
in each
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@435
PS10, Line 435: if (!partition_schema.hash_schema().empty())
What if the --create_table_hash_bucket_nums flag is specified for table with no hash partitioning? Should the tool report an error in that case instead of quietly ignoring that?
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@444
PS10, Line 444: // FLAGS_create_table_hash_bucket_nums is defined, then it's size
: // must be equal to the number of hash dimension.
nit: how about
If the --create_table_hash_bucket_nums flag is set, the number of comma-separated elements must be equal to the number of hash schema dimensions.
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@447
PS10, Line 447: Specified hash bucket numbers "
: "are not equal the number
how about: count of hash bucket numbers must be equal to the number of hash schema dimensions
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@454
PS10, Line 454: digit
Well, it seems safe_strto32 can accept numbers if hexadecimal notation as well, so this code will accept 0xff as well, if I'm not mistaken.
Anyways, more actionable error would be:
return Status::InvalidArgument(Substitute("'$0': cannot parse the number of hash buckets", hash_bucket_nums_str[i]));
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@454
PS10, Line 454: !
nit here and everywhere: there no need for exclamation marks in error messages. The exclamation marks don't add any actionable information but make error messages longer and harder to read.
http://gerrit.cloudera.org:8080/#/c/18604/10/src/kudu/tools/table_scanner.cc@457
PS10, Line 457: Hash bucket number must be >= 2
how about:
number of hash buckets must not be less than 2
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 19:33:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Tools] Support to config hash bucket numbers when copy a table
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18604
to look at the new patch set (#3).
Change subject: [Tools] Support to config hash bucket numbers when copy a table
......................................................................
[Tools] Support to config hash bucket numbers when copy a table
When copying a table to another cluster, we can create the same table
in the destination cluster. But we can not config the number of hash bucket
in the new table. Why we need to config hash bucket number? The old table
may be configured with small number of hash buckets, but stored with large
of data in it. When copying the old table to new cluster, we want to add more
hash buckets to store large of data. And there no method to change the number
of hash bucket when the table has already been created.
Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
5 files changed, 96 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/18604/3
--
To view, visit http://gerrit.cloudera.org:8080/18604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cec38e5ea09c66bfed20622b85033602da60d41
Gerrit-Change-Number: 18604
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>