You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2022/06/21 05:28:42 UTC
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18642
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch. I also added unit test scenarios to cover the updated
functionality of the PartitionSchema class.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
5 files changed, 329 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/1
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/master/catalog_manager.cc@2699
PS1, Line 2699: } else if (step.type() == AlterTableRequestPB::DROP_RANGE_PARTITION) {
> To summarize, for adding a range you just updated the partition schema prot
Yes, that's right: removing one of the repeated fields in a protobuf structure would be not very performant.
Probably, as a next step we can refactor the code and apply a similar approach for the ADD_RANGE_PARTITION control path, adding PartitionSchema::AddRange() method, and then we could remove PartitionSchema::CreatePartitionsForRange() method and use PartitionSchema::CreatePartitions() instead. I guess we can do that in a separate changelist.
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 19:03:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#7).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch.
In addition, I updated the PartitionSchema class to store only the
information on ranges with custom hash schemas in the
ranges_with_custom_hash_schemas_ field. I also added unit test
scenarios to cover the updated functionality of the PartitionSchema
class.
A follow-up patch is needed to refresh PartitionPruner's code since
the logic in PartitionPruner::Init() is expecting that the whole list
of table's ranges would returned by the
PartitionSchema::ranges_with_hash_schemas() method.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
8 files changed, 857 insertions(+), 174 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/7
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#3).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch. I also added unit test scenarios to cover the updated
functionality of the PartitionSchema class.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
6 files changed, 485 insertions(+), 138 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/3
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18642/3/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/3/src/kudu/common/partition-test.cc@1902
PS3, Line 1902: RowOperationsPBEncoder encoder(range->mutable_range_bounds());
> nit: one more time or once more
Done
http://gerrit.cloudera.org:8080/#/c/18642/5/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/5/src/kudu/master/catalog_manager.cc@2645
PS5, Line 2645: step.type() == AlterTableRequestPB::ADD_RANGE_PARTITION
> what is the purpose of this?
Ah, that's to make sure we don't store information on other than ranges with custom hash schemas in PartitionSchemaPB persisted in the system catalog.
I can remove this now since I refactored what information is stored in PartitionSchema (the counterpart for PartitionSchemaPB).
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Jun 2022 00:37:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has uploaded a new patch set (#5) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch. I also added unit test scenarios to cover the updated
functionality of the PartitionSchema class.
In addition, I updated the PartitionSchema class:
* removed ranges_with_hash_schemas() method
* updated HasCustomHashSchemas() method's implementation
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
7 files changed, 837 insertions(+), 186 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/5
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#6).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch.
In addition, I updated the PartitionSchema class to store only the
information on ranges with custom hash schemas in the
ranges_with_custom_hash_schemas_ field. I also added unit test
scenarios to cover the updated functionality of the PartitionSchema
class.
A follow-up patch is needed to refresh PartitionPruner's code since
the logic in PartitionPruner::Init() is expecting that the whole list
of table's ranges would returned by the
PartitionSchema::ranges_with_hash_schemas() method.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
8 files changed, 860 insertions(+), 174 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/6
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc@1746
PS2, Line 1746: for (size_t idx = 0; idx < updated_array.size(); ++idx) {
> Ack
After some consideration, I followed your suggestion. The updated code doesn't look too bad to me (using size of the array instead of computing the shift in indices).
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 22:19:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 8: Verified+1
unrelated test failure in log-rolling-itest (ASAN)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 06:25:29 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 7: Verified+1
unrelated test failure (ASAN): log-rolling-itest.0
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 02:46:23 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 8:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1492
PS8, Line 1492: RangeSchemaPB,
> nit: RangeWithHashSchemaPB
actually, that's about PartitionSchemaPB and its field custom_hash_schema_ranges
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1494
PS8, Line 1494: tablet
> nit: table
Done
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1497
PS8, Line 1497: RangeSchemaPB
> nit: RangeWithHashSchemaPB
Done
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@2163
PS8, Line 2163: truly
> nit: remove 'truly'
Done
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition.h@310
PS8, Line 310: with
> nit: remove extra with
Done
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition_pruner-test.cc
File src/kudu/common/partition_pruner-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition_pruner-test.cc@1124
PS8, Line 1124: PartitionSchema::RangesWithHashSchemas ranges;
: ASSERT_OK(PartitionSchema::FromPB(pb, schema, &partition_schema, &ranges));
:
: vector<Partition> partitions;
: ASSERT_OK(partition_schema.CreatePartitions(ranges, schema, &partitions));
> So to understand the usage of 'ranges'. Now since we're only storing the ra
Yes, that's right: I found this way it would be cleaner.
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/master/catalog_manager.cc@1939
PS8, Line 1939: based on specified ranges with custom hash schemas.
> Doesn't this method create partitions for each range, regardless of custom
Right. As you can see, there have been too many iterations on this relatively small feature, and comments are falling behind :)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 02:56:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 9: Verified+1
unrelated test failures (ASAN): ReplicatedAlterTableTest.AlterReplicationFactorWhileWriting
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 15:12:53 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18642/3/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/3/src/kudu/common/partition-test.cc@1902
PS3, Line 1902: hash_dimension->add_columns()->set_name("a");
nit: one more time or once more
http://gerrit.cloudera.org:8080/#/c/18642/5/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/5/src/kudu/master/catalog_manager.cc@2645
PS5, Line 2645: #if defined(NDEBUG)
what is the purpose of this?
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Jun 2022 16:55:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc@1746
PS2, Line 1746: for (size_t idx = 0; idx < updated_array.size(); ++idx) {
> We could, but that code would be a bit messy -- indices change after the dr
Ack
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 20:45:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#8).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch.
In addition, I updated the PartitionSchema class to store only the
information on ranges with custom hash schemas in the
ranges_with_custom_hash_schemas_ field. I also added unit test
scenarios to cover the updated functionality of the PartitionSchema
class.
A follow-up patch is needed to refresh PartitionPruner's code since
the logic in PartitionPruner::Init() is expecting that the whole list
of table's ranges would returned by the
PartitionSchema::ranges_with_hash_schemas() method.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
8 files changed, 858 insertions(+), 175 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/8
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 8: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 14:01:45 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 8:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1492
PS8, Line 1492: RangeSchemaPB,
nit: RangeWithHashSchemaPB
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1494
PS8, Line 1494: tablet
nit: table
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1497
PS8, Line 1497: RangeSchemaPB
nit: RangeWithHashSchemaPB
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@2163
PS8, Line 2163: truly
nit: remove 'truly'
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition.h
File src/kudu/common/partition.h:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition.h@310
PS8, Line 310: with
nit: remove extra with
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition_pruner-test.cc
File src/kudu/common/partition_pruner-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition_pruner-test.cc@1124
PS8, Line 1124: PartitionSchema::RangesWithHashSchemas ranges;
: ASSERT_OK(PartitionSchema::FromPB(pb, schema, &partition_schema, &ranges));
:
: vector<Partition> partitions;
: ASSERT_OK(partition_schema.CreatePartitions(ranges, schema, &partitions));
So to understand the usage of 'ranges'. Now since we're only storing the ranges with custom hash schema in PartitionSchema, we need to return all the ranges with their hash schema within the 'ranges' parameter in PartitionSchema::FromPB(). Then, that's being propagated to CreatePartitions so all the proper partitions can be created.
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/master/catalog_manager.cc@1939
PS8, Line 1939: based on specified ranges with custom hash schemas.
Doesn't this method create partitions for each range, regardless of custom or table wide hash schema.
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 19:45:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Jun 2022 00:07:41 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#2).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch. I also added unit test scenarios to cover the updated
functionality of the PartitionSchema class.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
5 files changed, 340 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/2
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch.
In addition, I updated the PartitionSchema class to store only the
information on ranges with custom hash schemas in the
ranges_with_custom_hash_schemas_ field. I also added unit test
scenarios to cover the updated functionality of the PartitionSchema
class.
A follow-up patch is needed to refresh PartitionPruner's code since
the logic in PartitionPruner::Init() is expecting that the whole list
of table's ranges would returned by the
PartitionSchema::ranges_with_hash_schemas() method.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Reviewed-on: http://gerrit.cloudera.org:8080/18642
Reviewed-by: Mahesh Reddy <mr...@cloudera.com>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Attila Bukor <ab...@apache.org>
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
8 files changed, 859 insertions(+), 176 deletions(-)
Approvals:
Mahesh Reddy: Looks good to me, but someone else must approve
Abhishek Chennaka: Looks good to me, but someone else must approve
Alexey Serbin: Verified
Attila Bukor: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 10
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/common/partition.cc@1770
PS6, Line 1770: if (updated_array.back().hash_schema != hash_schema_) {
: updated_array.emplace_back(updated_array.back());
: }
As discussed, we seem to be string the same element twice in the vector here.
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 23:34:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition-test.cc@1898
PS1, Line 1898: ASSERT_EQ(0, ps.ranges_with_hash_schemas().size());
> nit: check helper map size before and after as well.
Done
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition.cc@1738
PS1, Line 1738: continue;
> I know there's a check in catalog manager but do you think it's worth addin
Done
I added DCHECK_EQ() to compare the size of the original and the updated array.
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition.cc@1744
PS1, Line 1744: updated_array.size()
> Shouldn't this be size as well? updated_array.size() will be (size - 1) ass
Good catch -- this seems to be a bug introduced while I worked on a couple revisions of this code. There should be no 'continue' condition while iterating through the 'updated_array' container, of course.
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 18:53:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 15:32:47 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#9).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch.
In addition, I updated the PartitionSchema class to store only the
information on ranges with custom hash schemas in the
ranges_with_custom_hash_schemas_ field. I also added unit test
scenarios to cover the updated functionality of the PartitionSchema
class.
A follow-up patch is needed to refresh PartitionPruner's code since
the logic in PartitionPruner::Init() is expecting that the whole list
of table's ranges would returned by the
PartitionSchema::ranges_with_hash_schemas() method.
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
8 files changed, 859 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/9
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 03:43:19 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jun 2022 02:59:43 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 1: Verified+1
unrelated failure (timeout waiting for the build and tests pass):
22:44:50 369.2s 458/459 tests complete
23:34:50 Build timed out (after 50 minutes). Marking the build as failed.
23:34:50 build-support/jenkins/build-and-test.sh: line 782: 357218 Terminated
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Jun 2022 16:26:59 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition-test.cc@1898
PS1, Line 1898: ASSERT_EQ(0, ps.ranges_with_hash_schemas().size());
nit: check helper map size before and after as well.
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition.cc@1738
PS1, Line 1738: continue;
I know there's a check in catalog manager but do you think it's worth adding a check in these for loops to ensure that this if condition actually gets triggered?
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/common/partition.cc@1744
PS1, Line 1744: updated_array.size()
Shouldn't this be size as well? updated_array.size() will be (size - 1) assuming the above for loop operates correctly. When the if condition gets triggered here, then the size of updated_mapping will be (size - 2) which doesn't seem right. Maybe we could add some checks on the size of the respective fields before and after these for loops.
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/master/catalog_manager.cc@2699
PS1, Line 2699: } else if (step.type() == AlterTableRequestPB::DROP_RANGE_PARTITION) {
To summarize, for adding a range you just updated the partition schema protobuf directly in the catalog but for dropping a range you updated the partition schema object then serialized the object into the protobuf and updated it in the catalog. Is the difference in approaches due to the lack of a remove/drop method for the repeated field of protobuf?
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Jun 2022 21:55:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc@1746
PS2, Line 1746: for (size_t idx = 0; idx < updated_array.size(); ++idx) {
> Can we combine the two for loops since this is just essentially copying ove
We could, but that code would be a bit messy -- indices change after the dropping one element. I prefer to keep this as is for the clarity, and we can switch to more optimized version if there any performance issues observed (if any).
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 20:07:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 6: Verified+1
unrelated test failures in log-rolling-itest
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sun, 26 Jun 2022 06:10:17 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/master/master-test.cc@1030
PS6, Line 1030: in
nit: on*
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 04:16:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/master/master-test.cc@1030
PS6, Line 1030: in
> nit: on*
Done
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 05:23:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/6/src/kudu/common/partition.cc@1770
PS6, Line 1770: if (updated_array.back().hash_schema != hash_schema_) {
: updated_array.emplace_back(updated_array.back());
: }
> As discussed, we seem to be string the same element twice in the vector her
Good catch! I'm not sure how it ended up like this.
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 00:55:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Jun 2022 16:41:03 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18642
to look at the new patch set (#4).
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
KUDU-2671 update partition schema in catalog when dropping range
When dropping a range with custom hash schema from a table, it's
necessary to update the partition schema information stored in the
system catalog correspondingly. That was missing in one of the previous
patches and this patch fixes the deficiency.
This patch also adds a test scenario to spot regressions, if any. The
scenario was failing before the update in CatalogManager introduced
in this patch. I also added unit test scenarios to cover the updated
functionality of the PartitionSchema class.
In addition, I updated the PartitionSchema class:
* removed ranges_with_hash_schemas() method
* updated HasCustomHashSchemas() method's implementation
This is a follow-up to 250eb90bc0e1f4f472f44de8a23ce213595d5ee7.
Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
---
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
7 files changed, 837 insertions(+), 186 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/18642/4
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671 update partition schema in catalog when dropping range
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18642 )
Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc
File src/kudu/common/partition.cc:
http://gerrit.cloudera.org:8080/#/c/18642/2/src/kudu/common/partition.cc@1746
PS2, Line 1746: for (size_t idx = 0; idx < updated_array.size(); ++idx) {
Can we combine the two for loops since this is just essentially copying over the contents of updated_array to updated_mapping?
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18642/1/src/kudu/master/catalog_manager.cc@2699
PS1, Line 2699: } else if (step.type() == AlterTableRequestPB::DROP_RANGE_PARTITION) {
> Yes, that's right: removing one of the repeated fields in a protobuf struct
Ack
--
To view, visit http://gerrit.cloudera.org:8080/18642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 19:48:04 +0000
Gerrit-HasComments: Yes