You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org> on 2022/07/08 06:13:31 UTC
[kudu-CR] [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18712
Change subject: [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
[WIP] KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master WebUI
to show custom hash schemas in the sections of
1. Partition Schema
The custom hash schema if present for a particular range is displayed right
beside the range schema. Different dimensions of the hash schema are comma separated
2. Detail
There is a new column to identify if a particular partition has custom or table wide
hash schema. Every hash bucket is displayed with its corresponding hash schema.
The Kudu tablet server WebUI's pages /tablets and /tablet?id=<tablet_id> are also
updated to reflect the custom hash schema or table wide hash schema accordingly
This is yet to be tested thoroughly. But below are the screenshots of the WebUI
after the changes
Master WebUI:
https://i.imgur.com/Nw4GmtI.png
Tablet server WebUI:
https://i.imgur.com/bpGHFs4.png
https://i.imgur.com/obJ8JSf.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master_path_handlers.cc
3 files changed, 123 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/1
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 6:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc
File src/kudu/client/flex_partitioning_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc@573
PS6, Line 573: DefaultAndCustomHashSchemas
nit: it seems this doesn't reflect the name of the scenario; is it intended?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc@626
PS6, Line 626: tables.front()->GetAllTablets(&tablets);
Should we add a check on the size of 'tables' before trying to access its first element?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc@627
PS6, Line 627:
Should we add a check on the size of 'tablets' before trying to access its elements?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1032
PS6, Line 1032:
nit: misaligned indent
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1448
PS6, Line 1448: *
style nit here and below: stick the asterisk to the type
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1477
PS6, Line 1477: tables.front()->GetAllTablets(&tablets);
Should we check for non-emptiness (or exact size) of 'tables' before accessing its first element?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1496
PS6, Line 1496: {
Should we add checks on the number of elements in 'tablets' before accessing its elements?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1584
PS6, Line 1584: tables.front()->GetAllTablets(&tablets);
Should we also add ASSERT_GT(tables.size(), 0) or ASSERT_FALSE(tables.empty()) before accessing tables.front()?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1585
PS6, Line 1585:
Should we add a check for the size of 'tablets' before trying to access its elements?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master_path_handlers.cc@476
PS6, Line 476:
nit: misaligned indent
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 13 Jul 2022 20:35:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#7).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/O4ra4JA.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
6 files changed, 417 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/7
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 7:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc
File src/kudu/client/flex_partitioning_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc@573
PS6, Line 573: TabletServerWebUI";
> nit: it seems this doesn't reflect the name of the scenario; is it intended
Fixed it.
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc@626
PS6, Line 626: vector<scoped_refptr<TabletInfo>> tablets;
> Should we add a check on the size of 'tables' before trying to access its f
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/client/flex_partitioning_client-test.cc@627
PS6, Line 627: tables.front()->GetAllTablets(&tablets);
> Should we add a check on the size of 'tablets' before trying to access its
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1032
PS6, Line 1032:
> nit: misaligned indent
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1060
PS6, Line 1060: upper
> Should this be 'upper' instead?
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1062
PS6, Line 1062: lower
> Should this be 'lower' instead?
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1069
PS6, Line 1069: hash
> hash
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1069
PS6, Line 1069: schema infor
> schema
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1195
PS6, Line 1195:
: if (idx_ptr) {
> Is it possible to use binary search instead of linear iteration here? The
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1448
PS6, Line 1448: *
> style nit here and below: stick the asterisk to the type
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1477
PS6, Line 1477:
> Should we check for non-emptiness (or exact size) of 'tables' before access
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1496
PS6, Line 1496: ")\"");
> Did you miss this one?
The check is done a few lines above in https://gerrit.cloudera.org/#/c/18712/7/src/kudu/master/master-test.cc@1480
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1496
PS6, Line 1496: ")\"");
> Should we add checks on the number of elements in 'tablets' before accessin
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1584
PS6, Line 1584: CatalogManager::ScopedLeaderSharedLock l(master_->catalog_manager());
> Should we also add ASSERT_GT(tables.size(), 0) or ASSERT_FALSE(tables.empty
Added ASSERT_EQ(tables.size, 1)
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1585
PS6, Line 1585: master_->catalog_manager()->GetAllTables(&tables);
> Should we add a check for the size of 'tablets' before trying to access its
Done
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master_path_handlers.cc@476
PS6, Line 476:
> nit: misaligned indent
Done
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 14:38:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 3:
(6 comments)
A quick top-level review.
http://gerrit.cloudera.org:8080/#/c/18712/3//COMMIT_MSG
Commit Message:
PS3:
Overall looks good.
There is some difference in terms: in master's webUI BUCKET is used for the partition/bucket index; in tablet server's webUI that's PARTITION. I'm not sure how to consolidate these differences.
Maybe, for the master's webUI, instead of mixing both the schema and the bucket index information into the same column 'Hash Schema', there should be two separate columns: 'Hash Schema' and 'Hash Partition'?
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.h@409
PS3, Line 409:
nit: misaligned indent
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.h@410
PS3, Line 410:
style nit: stick the ampersand to the type
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.cc@1087
PS3, Line 1087: // Get the custom hash schema information, if present, for this partition
It's possible to find hash schema for a range using the helper map hash_schema_idx_by_encoded_range_start_: you can see how to do so in PartitionSchema::GetHashSchemaForRange().
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.cc@1096
PS3, Line 1096: lower
Why to compare the upper_boundary with lower?
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/master/master-test.cc@290
PS3, Line 290: hash_schema->set_seed(hash_dimension.seed);
Could you separate this into its own changelist? I guess this change isn't related to the essence of this patch, right?
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 06:15:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/O4ra4JA.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Reviewed-on: http://gerrit.cloudera.org:8080/18712
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
6 files changed, 413 insertions(+), 16 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/master/master-test.cc@1496
PS6, Line 1496: ")\"");
> Should we add checks on the number of elements in 'tablets' before accessin
Did you miss this one?
http://gerrit.cloudera.org:8080/#/c/18712/7/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/7/src/kudu/master/master-test.cc@1476
PS7, Line 1476: tables.size(), 1
here and elsewhere: the expected value comes first in ASSERT_XXX -- that way it's easier to read error messages if assertion triggers
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 05:17:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#9).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/O4ra4JA.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
6 files changed, 415 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/9
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG
Commit Message:
PS1:
> style nit: please keep the lines of the description to be 72 characters or
Done
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 02:30:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1060
PS6, Line 1060: lower
Should this be 'upper' instead?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1062
PS6, Line 1062: upper
Should this be 'lower' instead?
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1069
PS6, Line 1069: Hash
hash
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1069
PS6, Line 1069: partitioning
schema
http://gerrit.cloudera.org:8080/#/c/18712/6/src/kudu/common/partition.cc@1195
PS6, Line 1195: for (const auto& range : ranges_with_custom_hash_schemas_) {
: if (range.lower == partition.begin_.range_key()) {
Is it possible to use binary search instead of linear iteration here? The `hash_schema_idx_by_encoded_range_start_` member field is a tree dictionary built to complement ranges_with_custom_hash_schemas_ to perform exactly that sort of lookup.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 13 Jul 2022 20:16:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#10).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/O4ra4JA.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
6 files changed, 413 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/10
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#8).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/O4ra4JA.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
6 files changed, 417 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/8
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 5: Code-Review+1
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.h@406
PS5, Line 406: a comma separated
BTW, impala-shell is going to output them as space separated: https://gerrit.cloudera.org/#/c/18720/
Should we use the same convention here as well? What do you think?
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1059
PS5, Line 1059: } else if (lower_unbounded) {
: partition = Substitute("VALUES < $0", RangeKeyDebugString(lower));
: } else if (upper_unbounded) {
: partition = Substitute("VALUES >= $0", RangeKeyDebugString(upper));
It seems this is messed up a bit.
Overall, using '$0 <= VALUES' and 'VALUES < $0' is easier to comprehend when reading.
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1064
PS5, Line 1064: <
Should this be '<='?
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1195
PS5, Line 1195: range_schema
nit: 'range_schema' usually means range partitioning schema, so it's a bit confusing for me to read trough this piece. Maybe, rename this variable into 'range'?
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1197
PS5, Line 1197: Hash Schema
Could we drop this part? The header of the column already provides the 'hash schema' context.
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1219
PS5, Line 1219: Hash Schema
Could we drop this part? The header of the column already provides the 'hash schema' context.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 13 Jul 2022 19:01:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1040
PS8, Line 1040: // Decode the lower and upper bounds
> My comment was about 'hash_schema'. Is it used anywhere in this scope befo
No it isn't, but the value of lower_bound is changed after line 1043, so calling GetHashSchemaForRange(lower_bound.ToString()) after line 1043 returns table wide hash schema always.
http://gerrit.cloudera.org:8080/#/c/18712/9/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/9/src/kudu/common/partition.cc@1214
PS9, Line 1214: }
:
: else {
> I guess LINT target isn't happy about 'else {' at a separate line: move it
Ah, yes. Fixed it now.
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1502
PS8, Line 1502: HASH
> nit: ah, OK -- I thought that using 2*i + j would make it simple enough. F
Done
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 20:15:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#3).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There is a new column to identify if a particular partition has
custom or table wide hash schema. Every hash bucket is displayed with
its corresponding hash schema.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also updated to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/oZcI7L9.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
5 files changed, 407 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/3
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 10: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1040
PS8, Line 1040: // Decode the lower and upper bounds
> No it isn't, but the value of lower_bound is changed after line 1043, so ca
Ah, OK -- it seems I missed the fact that DecodeRangeKey() modifies its parameter, sorry.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 20:53:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18712/3//COMMIT_MSG
Commit Message:
PS3:
> Overall looks good.
Done
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.h@409
PS3, Line 409:
> nit: misaligned indent
Done
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.h@410
PS3, Line 410:
> style nit: stick the ampersand to the type
Done
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.cc@1087
PS3, Line 1087: // Get the custom hash schema information, if present, for this partition
> It's possible to find hash schema for a range using the helper map hash_sch
Done
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/common/partition.cc@1096
PS3, Line 1096: lower
> Why to compare the upper_boundary with lower?
Good catch. That's incorrect. But we do not need this loop anyway as we can get the hash schema for a partition as suggested above
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/3/src/kudu/master/master-test.cc@290
PS3, Line 290: hash_schema->set_seed(hash_dimension.seed);
> Could you separate this into its own changelist? I guess this change isn't
Yes, will do
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 17:46:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG@13
PS1, Line 13: The custom hash schema if present for a particular range is displayed right
: beside the range schema
> I think you are referring to the in table under Detail section. The origina
Right, but where a user could get information on the number of hash buckets in range-specific hash schema? With current code in this PS, a user would need to go over all the output and find the maximum hash bucket index for all the tablets that back the range in question. That's not good from the usability standpoint.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 22:44:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG@13
PS1, Line 13: The custom hash schema if present for a particular range is displayed right
: beside the range schema
> I could not see the hash schema as-is there, something that one would expec
I think you are referring to the in table under Detail section. The original implementation had the column header as "HASH (column_name) PARTITION" and each column having the corresponding bucket id of the partition and I tried to keep the new implementation close to this.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 14:21:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#4).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/Xj8FeGW.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
5 files changed, 399 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/4
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1195
PS5, Line 1195: range_schema
> nit: 'range_schema' usually means range partitioning schema, so it's a bit
Done
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1197
PS5, Line 1197: Hash Schema
> Could we drop this part? The header of the column already provides the 'ha
Done
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1219
PS5, Line 1219: Hash Schema
> Could we drop this part? The header of the column already provides the 'ha
Done
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 13 Jul 2022 19:44:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#6).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/O4ra4JA.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
5 files changed, 402 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/6
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 9: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1040
PS8, Line 1040: // Decode the lower and upper bounds
> The DecodeRangeKey() below is setting this to remainder of the composite ke
My comment was about 'hash_schema'. Is it used anywhere in this scope before line 1069 in PS9?
http://gerrit.cloudera.org:8080/#/c/18712/9/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/9/src/kudu/common/partition.cc@1214
PS9, Line 1214: }
:
: else {
I guess LINT target isn't happy about 'else {' at a separate line: move it to be at line 1214 after the closing brace '}'
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1502
PS8, Line 1502: HASH
> We reset j but not k. We need k to get the index range [0-3]. Using j alone
nit: ah, OK -- I thought that using 2*i + j would make it simple enough. Feel free to keep 'k' if that's more natural. But could you move the increment of the 'k' to the line 1511 then? Something like
tablets[k++]->id(), ...
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 19:40:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 9:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1039
PS8, Line 1039:
> nit: move this closer to line 1048 where it's used?
Done
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1040
PS8, Line 1040: // Decode the lower and upper bounds
> nit: move this closer to line 1070 where it's used?
The DecodeRangeKey() below is setting this to remainder of the composite key after decoding, so had to use it before this is called.
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1071
PS8, Line 1071: .appen
> style nit: stick the ampersand to the type, not to the variable
Done
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1215
PS8, Line 1215:
: else {
: entry.append("<td>
> Semantically, this should always be if/else regardless of the 'entry' empti
Agree, it should have been updated after we revised the loop from linear structure. Fixed it now
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1453
PS8, Line 1453: {
> Could you please re-base this patch on the top of the master branch in the
Done
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1502
PS8, Line 1502: HASH
> I'm not sure I understand why 'k' is needed when there is 'j' already?
We reset j but not k. We need k to get the index range [0-3]. Using j alone or combined with i, I couldn't figure out a logic to get the index in each iteration.
i j index
0 0 0
0 1 1
1 0 2
1 1 3
I could theoretically do index=i+i+j to make it work but doesn't look correct logically.
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1529
PS8, Line 1529: ColumnSchemaPB*
> style nit here and below: stick the asterisk to the type
Done
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1540
PS8, Line 1540: KuduPartialRow upper(&kTabl
> style nit here and below: stick the asterisk to the type
Done
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1552
PS8, Line 1552:
> style nit here and below: stick the ampersand to the type, not to the varia
Done
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 19:17:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG@13
PS1, Line 13: The custom hash schema if present for a particular range is displayed right
: beside the range schema
> Right, but where a user could get information on the number of hash buckets
I see your point now. Let me push a new patch with the hash bucket information included.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Jul 2022 01:40:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.h@406
PS5, Line 406: a comma separated
> BTW, impala-shell is going to output them as space separated: https://gerri
I made it space separated here as well. Just forgot to update to update the comment. Will fix it
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1059
PS5, Line 1059: } else if (lower_unbounded) {
: partition = Substitute("VALUES < $0", RangeKeyDebugString(lower));
: } else if (upper_unbounded) {
: partition = Substitute("VALUES >= $0", RangeKeyDebugString(upper));
> It seems this is messed up a bit.
Okay, so that will need https://gerrit.cloudera.org/#/c/18712/5/src/kudu/common/partition.cc@1000
to be changed as well to maintain uniformity.
http://gerrit.cloudera.org:8080/#/c/18712/5/src/kudu/common/partition.cc@1064
PS5, Line 1064: <
> Should this be '<='?
Yes, good catch. Will fix it.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 13 Jul 2022 19:13:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#2).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master WebUI
to show custom hash schemas in the sections of
1. Partition Schema
The custom hash schema if present for a particular range is displayed right
beside the range schema. Different dimensions of the hash schema are comma separated
2. Detail
There is a new column to identify if a particular partition has custom or table wide
hash schema. Every hash bucket is displayed with its corresponding hash schema.
The Kudu tablet server WebUI's pages /tablets and /tablet?id=<tablet_id> are also
updated to reflect the custom hash schema or table wide hash schema accordingly
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/oZcI7L9.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
5 files changed, 407 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/2
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 8: Code-Review+1
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1039
PS8, Line 1039: KuduPartialRow upper(&schema);
nit: move this closer to line 1048 where it's used?
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1040
PS8, Line 1040: HashSchema hash_schema = GetHashSchemaForRange(lower_bound.ToString());
nit: move this closer to line 1070 where it's used?
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1071
PS8, Line 1071: auto &
style nit: stick the ampersand to the type, not to the variable
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/common/partition.cc@1215
PS8, Line 1215: }
:
: if (entry.empty()) {
Semantically, this should always be if/else regardless of the 'entry' emptiness, right? With that, it would be easier for understanding if replacing 'if (entry.empty())' with just 'else' (and it saves a few CPU instructions as well).
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1453
PS8, Line 1453: FLAGS_enable_per_range_hash_schemas = true;
Could you please re-base this patch on the top of the master branch in the git repo? This no longer needed since --enable_per_range_hash_schemas=true by default.
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1502
PS8, Line 1502: k++
I'm not sure I understand why 'k' is needed when there is 'j' already?
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1529
PS8, Line 1529: ColumnSchemaPB *
style nit here and below: stick the asterisk to the type
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1540
PS8, Line 1540: AlterTableRequestPB::Step *
style nit here and below: stick the asterisk to the type
http://gerrit.cloudera.org:8080/#/c/18712/8/src/kudu/master/master-test.cc@1552
PS8, Line 1552: auto &
style nit here and below: stick the ampersand to the type, not to the variable
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 14 Jul 2022 17:19:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18712
to look at the new patch set (#5).
Change subject: KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
KUDU-2671 Make WebUI compatible with custom hash schema
This patch updates the /table?id=<table_id> page in the Kudu master
WebUI to show custom hash schemas in the sections of:
1. Partition Schema
The custom hash schema if present for a particular range is displayed
right beside the range schema. Different dimensions of the hash
schema are comma separated.
2. Detail
There are new columns to identify if a particular partition has
custom or table wide hash schema, display the hash schema and the hash
partition id of the partition.
The Kudu tablet server WebUI's pages /tablets and
/tablet?id=<tablet_id> are also tested to reflect the custom hash
schema or table wide hash schema accordingly.
Below are the screenshots of the WebUI after the changes
Master WebUI:
https://i.imgur.com/Xj8FeGW.png
Tablet server WebUI:
https://i.imgur.com/BxdfsYt.png
https://i.imgur.com/l2wA08Q.png
Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
5 files changed, 401 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/18712/5
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18712 )
Change subject: [WIP] KUDU-2671 Make WebUI compatible with custom hash schema
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG
Commit Message:
PS1:
style nit: please keep the lines of the description to be 72 characters or less; see https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines as linked from https://kudu.apache.org/docs/contributing.html#_submitting_patches
http://gerrit.cloudera.org:8080/#/c/18712/1//COMMIT_MSG@13
PS1, Line 13: The custom hash schema if present for a particular range is displayed right
: beside the range schema
I could not see the hash schema as-is there, something that one would expect to be similar to the part of the 'Partition Schema' part in the top of the page. In particular, there was no information on the number of buckets in each hash dimension, just the index of the corresponding bucket.
--
To view, visit http://gerrit.cloudera.org:8080/18712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8b8d90f70c39f13b838e858c870e08dacbdfcd3
Gerrit-Change-Number: 18712
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jul 2022 17:53:59 +0000
Gerrit-HasComments: Yes