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 8c8f393a4dc772a3dae2c14e59952ab1569884ec Author: Alexey Serbin <[email protected]> AuthorDate: Fri Jul 8 16:07:57 2022 -0700 KUDU-2671 fix double-free in new scenario of scan_token-test Changelist b746978c7 introduced a bug in the updated CountRowsSeq() function -- the mistake was calling delete twice on KuduScanToken pointers. That lead to reading garbage data, so the newly introduced test scenario ScanTokensWithCustomHashSchemasPerRange was failing if enabled. I didn't pay enough attention to the updated test code when reviewing it since the new test scenario was disabled in the original patch b746978c7. This patch fixes the mistake. Change-Id: I227f1dadf9d5b4d3b209570716cde7bda74c6b25 Reviewed-on: http://gerrit.cloudera.org:8080/18714 Tested-by: Alexey Serbin <[email protected]> Reviewed-by: Khazar Mammadli <[email protected]> Reviewed-by: Mahesh Reddy <[email protected]> Reviewed-by: Abhishek Chennaka <[email protected]> Reviewed-by: Attila Bukor <[email protected]> --- src/kudu/client/scan_token-test.cc | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/kudu/client/scan_token-test.cc b/src/kudu/client/scan_token-test.cc index 11e9697ee..f38dae6ae 100644 --- a/src/kudu/client/scan_token-test.cc +++ b/src/kudu/client/scan_token-test.cc @@ -137,13 +137,12 @@ class ScanTokenTest : public KuduTest { // Similar to CountRows() above, but use the specified client handle // and run all the scanners sequentially, one by one. static Status CountRowsSeq(KuduClient* client, - const vector<KuduScanToken*>& tokens, - int64_t* row_count) { + const vector<KuduScanToken*>& tokens, + int64_t* row_count) { int64_t count = 0; for (auto* t : tokens) { - unique_ptr<KuduScanToken> token(t); unique_ptr<KuduScanner> scanner; - RETURN_NOT_OK(IntoUniqueScanner(client, *token, &scanner)); + RETURN_NOT_OK(IntoUniqueScanner(client, *t, &scanner)); RETURN_NOT_OK(scanner->Open()); while (scanner->HasMoreRows()) { KuduScanBatch batch; @@ -583,9 +582,7 @@ TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) { } } -// TODO(mreddy) : Enable test once there is support for scan tokens with -// custom hash schema per range. -TEST_F(ScanTokenTest, DISABLED_TestScanTokensWithCustomHashSchemasPerRange) { +TEST_F(ScanTokenTest, ScanTokensWithCustomHashSchemasPerRange) { FLAGS_enable_per_range_hash_schemas = true; KuduSchema schema; // Create schema @@ -610,8 +607,9 @@ TEST_F(ScanTokenTest, DISABLED_TestScanTokensWithCustomHashSchemasPerRange) { { ASSERT_OK(lower_bound->SetInt64("col", 0)); ASSERT_OK(upper_bound->SetInt64("col", 100)); - unique_ptr<KuduTableCreator::KuduRangePartition> range_partition - (new KuduTableCreator::KuduRangePartition(lower_bound.release(), upper_bound.release())); + unique_ptr<KuduTableCreator::KuduRangePartition> range_partition( + new KuduTableCreator::KuduRangePartition(lower_bound.release(), + upper_bound.release())); range_partition->add_hash_partitions({ "col" }, 4); table_creator->add_custom_range_partition(range_partition.release()); } @@ -621,8 +619,9 @@ TEST_F(ScanTokenTest, DISABLED_TestScanTokensWithCustomHashSchemasPerRange) { upper_bound.reset(schema.NewRow()); ASSERT_OK(lower_bound->SetInt64("col", 100)); ASSERT_OK(upper_bound->SetInt64("col", 200)); - unique_ptr<KuduTableCreator::KuduRangePartition> range_partition - (new KuduTableCreator::KuduRangePartition(lower_bound.release(), upper_bound.release())); + unique_ptr<KuduTableCreator::KuduRangePartition> range_partition( + new KuduTableCreator::KuduRangePartition(lower_bound.release(), + upper_bound.release())); range_partition->add_hash_partitions({ "col"}, 2); table_creator->add_custom_range_partition(range_partition.release()); } @@ -1116,6 +1115,7 @@ TEST_P(StaleScanTokensParamTest, DroppingFirstRange) { // Prepare two sets of scan tokens. vector<KuduScanToken*> tokens_a; + ElementDeleter deleter_a(&tokens_a); { KuduScanTokenBuilder builder(table.get()); ASSERT_OK(builder.IncludeTableMetadata(true)); @@ -1125,6 +1125,7 @@ TEST_P(StaleScanTokensParamTest, DroppingFirstRange) { ASSERT_EQ(2, tokens_a.size()); vector<KuduScanToken*> tokens_b; + ElementDeleter deleter_b(&tokens_b); { KuduScanTokenBuilder builder(table.get()); ASSERT_OK(builder.IncludeTableMetadata(true));
