This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 23908bdbe753c25aa2a33796559ecf64f0197949
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Fri Jul 8 17:01:34 2022 -0700
KUDU-2671 fix consistency check in KuduScanner::NextBatch()
KuduScanner::Data::OpenTablet() can return earlier when the metacache
returns a tablet covering the partition key range past the partition key
provided. In such case, the 'proxy_' member of KuduScanner::Data isn't
set, and that's expected since the upper level code should continue with
the next tablet. The new test sub-scenarios introduced in [1] triggered
the consistency check on the 'data_->proxy_' member in
KuduScanner::NextBatch(), so this patch addresses the issue.
Also, this patch re-enables a few test sub-scenarios introduced in
[1] and [2].
[1]
https://github.com/apache/kudu/commit/b746978c71ce4a95b69d49c43d0ac852909a8b4e
[2]
https://github.com/apache/kudu/commit/1889d4c44385fec5efeeb2d287d9ab7a3544dcfe
Change-Id: Icfbdfac46f35d4d143f37845d158795bf1793da7
Reviewed-on: http://gerrit.cloudera.org:8080/18715
Reviewed-by: Khazar Mammadli <mammadli.kha...@cloudera.com>
Tested-by: Alexey Serbin <ale...@apache.org>
Reviewed-by: Mahesh Reddy <mre...@cloudera.com>
Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
Reviewed-by: Attila Bukor <abu...@apache.org>
---
src/kudu/client/client.cc | 3 +-
src/kudu/client/flex_partitioning_client-test.cc | 37 +++++++++---------------
src/kudu/client/scanner-internal.cc | 1 +
3 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 46b9a53da..7adc33800 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -2027,7 +2027,6 @@ Status
KuduScanner::NextBatch(internal::ScanBatchDataInterface* batch_data) {
// need to do some swapping of the response objects around to avoid
// stomping on the memory the user is looking at.
CHECK(data_->open_);
- CHECK(data_->proxy_);
batch_data->Clear();
@@ -2037,6 +2036,7 @@ Status
KuduScanner::NextBatch(internal::ScanBatchDataInterface* batch_data) {
if (data_->data_in_open_) {
// We have data from a previous scan.
+ CHECK(data_->proxy_);
VLOG(2) << "Extracting data from " << data_->DebugString();
data_->data_in_open_ = false;
return batch_data->Reset(&data_->controller_,
@@ -2048,6 +2048,7 @@ Status
KuduScanner::NextBatch(internal::ScanBatchDataInterface* batch_data) {
if (data_->last_response_.has_more_results()) {
// More data is available in this tablet.
+ CHECK(data_->proxy_);
VLOG(2) << "Continuing " << data_->DebugString();
MonoTime batch_deadline = MonoTime::Now() +
data_->configuration().timeout();
diff --git a/src/kudu/client/flex_partitioning_client-test.cc
b/src/kudu/client/flex_partitioning_client-test.cc
index 35a10a54f..17ed4baa0 100644
--- a/src/kudu/client/flex_partitioning_client-test.cc
+++ b/src/kudu/client/flex_partitioning_client-test.cc
@@ -253,9 +253,9 @@ class FlexPartitioningTest : public KuduTest {
}
ASSERT_OK(scanner.Open());
- KuduScanBatch batch;
int64_t count = 0;
while (scanner.HasMoreRows()) {
+ KuduScanBatch batch;
ASSERT_OK(scanner.NextBatch(&batch));
count += batch.NumRows();
}
@@ -300,7 +300,7 @@ class FlexPartitioningTest : public KuduTest {
ASSERT_OK(scanner.AddConjunctPredicate(table->NewComparisonPredicate(
col_name, KuduPredicate::GREATER_EQUAL, KuduValue::FromInt(lower))));
ASSERT_OK(scanner.AddConjunctPredicate(table->NewComparisonPredicate(
- col_name, KuduPredicate::LESS_EQUAL, KuduValue::FromInt(upper))));
+ col_name, KuduPredicate::LESS, KuduValue::FromInt(upper))));
ASSERT_OK(scanner.Open());
KuduScanBatch batch;
@@ -444,19 +444,13 @@ TEST_F(FlexPartitioningCreateTableTest,
DISABLED_SingleCustomRangeEmptyHashSchem
// the partitions: first check the range of table-wide schema, then check
// the ranges with custom hash schemas.
ASSERT_OK(InsertTestRows(kTableName, -111, 0));
- //NO_FATALS(CheckTableRowsNum(kTableName, 111));
+ NO_FATALS(CheckTableRowsNum(kTableName, 111));
ASSERT_OK(InsertTestRows(kTableName, 111, 222));
- // TODO(aserbin): uncomment the line below once PartitionPruner handles such
- // cases properly
- //NO_FATALS(CheckTableRowsNum(kTableName, 222));
+ NO_FATALS(CheckTableRowsNum(kTableName, 222));
}
// Create a table with mixed set of range partitions, using both table-wide and
// custom hash bucket schemas.
-//
-// TODO(aserbin): add verification based on PartitionSchema provided by
-// KuduTable::partition_schema() once PartitionPruner
-// recognizes custom hash bucket schema per range
TEST_F(FlexPartitioningCreateTableTest, DefaultAndCustomHashSchemas) {
// Create a table with the following partitions:
//
@@ -991,8 +985,8 @@ TEST_F(FlexPartitioningCreateTableTest,
ScansWithRangePredicates) {
CheckScanWithColumnPredicate(kTableName, "key", 83, 3, 250, 333);
CheckScanWithColumnPredicate(kTableName, "key", 1, 1, 0, 1);
CheckScanWithColumnPredicate(kTableName, "key", 2, 6, 110, 112);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, -10, -5);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 350, 400);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, -10, -5);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 350, 400);
CheckScanWithColumnPredicate(kTableName, "key", 1, 1, 332, 333);
CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 333, 334);
CheckScanWithColumnPredicate(kTableName, "key", 100, 2, lower_filler, 100);
@@ -1003,7 +997,7 @@ TEST_F(FlexPartitioningCreateTableTest,
ScansWithRangePredicates) {
CheckScanWithColumnPredicate(kTableName, "key", 233, 9, 100, upper_filler);
CheckScanWithColumnPredicate(kTableName, "key", 133, 7, 200, upper_filler);
CheckScanWithColumnPredicate(kTableName, "key", 83, 3, 250, upper_filler);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 333, upper_filler);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 333, upper_filler);
// Meanwhile, inserting into non-covered ranges should result in a proper
// error status return to the client attempting such an operation.
@@ -1095,8 +1089,8 @@ TEST_F(FlexPartitioningCreateTableTest,
ScansWithRangePredicatesWithSameHashBuck
CheckScanWithColumnPredicate(kTableName, "key", 3, 6, 110, 113);
CheckScanWithColumnPredicate(kTableName, "key", 1, 1, 332, 333);
CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 333, 334);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, -10, -5);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 350, 400);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, -10, -5);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 350, 400);
CheckScanWithColumnPredicate(kTableName, "key", 100, 3, lower_filler, 100);
CheckScanWithColumnPredicate(kTableName, "key", 250, 9, lower_filler, 250);
CheckScanWithColumnPredicate(kTableName, "key", 333, 9, lower_filler, 333);
@@ -1105,7 +1099,7 @@ TEST_F(FlexPartitioningCreateTableTest,
ScansWithRangePredicatesWithSameHashBuck
CheckScanWithColumnPredicate(kTableName, "key", 233, 9, 100, upper_filler);
CheckScanWithColumnPredicate(kTableName, "key", 133, 6, 200, upper_filler);
CheckScanWithColumnPredicate(kTableName, "key", 83, 3, 250, upper_filler);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 333, upper_filler);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 333, upper_filler);
// Meanwhile, inserting into non-covered ranges should result in a proper
// error status return to the client attempting such an operation.
@@ -1190,8 +1184,8 @@ TEST_F(FlexPartitioningCreateTableTest,
ScansWithNonCoveringRanges) {
CheckScanWithColumnPredicate(kTableName, "key", 333, 9, -50, 600);
CheckScanWithColumnPredicate(kTableName, "key", 222, 5, 150, 600);
CheckScanWithColumnPredicate(kTableName, "key", 1, 1, 0, 1);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, -10, -5);
- //CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 600, 650);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, -10, -5);
+ CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 600, 650);
CheckScanWithColumnPredicate(kTableName, "key", 1, 1, 554, 555);
CheckScanWithColumnPredicate(kTableName, "key", 0, 0, 555, 556);
CheckScanWithColumnPredicate(kTableName, "key", 111, 4, lower_filler, 150);
@@ -1538,10 +1532,8 @@ TEST_F(FlexPartitioningAlterTableTest,
ReadAndWriteToCustomRangePartition) {
ASSERT_OK(InsertTestRows(kTableName, 111, 444));
NO_FATALS(CheckTableRowsNum(kTableName, 444));
- // WIP: Scan the data present after rebasing on pruning patches
- /*
- NO_FATALS(CheckTableRowsNum(kTableName, kKeyColumn, 111, 222, 111));
- NO_FATALS(CheckTableRowsNum(kTableName, kKeyColumn, 0, 444, 444));
+ NO_FATALS(CheckTableRowsNum(kTableName, kKeyColumn, 111, 222, 111));
+ NO_FATALS(CheckTableRowsNum(kTableName, kKeyColumn, 0, 444, 444));
// Drop a partition and re-scan
{
unique_ptr<KuduTableAlterer>
table_alterer_drop(client_->NewTableAlterer(kTableName));
@@ -1553,7 +1545,6 @@ TEST_F(FlexPartitioningAlterTableTest,
ReadAndWriteToCustomRangePartition) {
ASSERT_OK(table_alterer_drop->Alter());
}
NO_FATALS(CheckTableRowsNum(kTableName, kKeyColumn, 0, 444, 333));
- */
// Meanwhile, inserting into non-covered ranges should result in a proper
// error status return to the client attempting such an operation.
diff --git a/src/kudu/client/scanner-internal.cc
b/src/kudu/client/scanner-internal.cc
index 18a436fba..a5ad65bb4 100644
--- a/src/kudu/client/scanner-internal.cc
+++ b/src/kudu/client/scanner-internal.cc
@@ -511,6 +511,7 @@ Status KuduScanner::Data::OpenTablet(const PartitionKey&
partition_key,
if (s.IsNotFound()) {
// No more tablets in the table.
partition_pruner_.RemovePartitionKeyRange({});
+ DCHECK(!partition_pruner_.HasMorePartitionKeyRanges());
return Status::OK();
}
RETURN_NOT_OK(s);
