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
The following commit(s) were added to refs/heads/master by this push:
new e3c6259d0 [client] fix result status handling
e3c6259d0 is described below
commit e3c6259d02b06e86470c03289ddbc99ddf7b9ce3
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Feb 27 18:16:00 2025 -0800
[client] fix result status handling
This patch addresses sloppy result handling of functions/methods
that return Status under the src/kudu/client directory.
Change-Id: If12f237b254b041cb6df1b037d4bc42482849699
Reviewed-on: http://gerrit.cloudera.org:8080/22672
Reviewed-by: Yifan Zhang <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
src/kudu/client/client-test.cc | 45 +++++++++++++++++-----------------
src/kudu/client/client-unittest.cc | 2 +-
src/kudu/client/scan_token-internal.cc | 14 +++++------
src/kudu/client/scan_token-test.cc | 16 ++++++------
src/kudu/client/schema.cc | 23 ++++++++---------
5 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index e2ce4aebc..e8e8ad434 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -461,7 +461,7 @@ class ClientTest : public KuduTest {
unique_ptr<KuduScanner> scanner(scanner_ptr);
// Try to avoid flakiness when running at busy nodes.
- scanner->SetTimeoutMillis(60 * 1000);
+ THR_RET_NOT_OK(scanner->SetTimeoutMillis(60 * 1000), thread_status);
// Make sure to read the most recent data if the test table has
// multiple replicas.
THR_RET_NOT_OK(scanner->SetSelection(KuduClient::LEADER_ONLY),
thread_status);
@@ -766,7 +766,7 @@ class ClientTest : public KuduTest {
client_table_->NewComparisonPredicate("string_val",
KuduPredicate::LESS_EQUAL,
KuduValue::CopyString("hello 3"))));
if (!query_id.empty()) {
- scanner.SetQueryId(query_id);
+ ASSERT_OK(scanner.SetQueryId(query_id));
}
LOG_TIMING(INFO, "Scanning with string predicate") {
ASSERT_OK(scanner.Open());
@@ -2009,7 +2009,7 @@ TEST_F(ClientTest, TestNonCoveringRangePartitions) {
ASSERT_OK(split->SetInt32("key", 50));
splits.push_back(std::move(split));
- CreateTable(kTableName, 1, std::move(splits), std::move(bounds), &table);
+ ASSERT_OK(CreateTable(kTableName, 1, std::move(splits), std::move(bounds),
&table));
// Aggresively clear the meta cache between insert batches so that the meta
// cache will execute GetTableLocation RPCs at different partition keys.
@@ -2141,7 +2141,7 @@ TEST_F(ClientTest,
TestOpenTableClearsNonCoveringRangePartitions) {
ASSERT_OK(upper_bound->SetInt32("key", 1));
bounds.emplace_back(std::move(lower_bound), std::move(upper_bound));
- CreateTable(kTableName, 1, {}, std::move(bounds), &table);
+ ASSERT_OK(CreateTable(kTableName, 1, {}, std::move(bounds), &table));
// Attempt to insert into the non-covered range, priming the meta cache.
shared_ptr<KuduSession> session = client_->NewSession();
@@ -3018,7 +3018,7 @@ class KeepAlivePeriodicallyTest :
InternalMiniClusterOptions options;
options.num_tablet_servers = 3;
cluster_->Shutdown();
- env_->DeleteRecursively(test_dir_);
+ ASSERT_OK(env_->DeleteRecursively(test_dir_));
cluster_.reset(new InternalMiniCluster(env_, options));
ASSERT_OK(cluster_->Start());
ASSERT_OK(KuduClientBuilder()
@@ -3115,14 +3115,14 @@ TEST_P(KeepAlivePeriodicallyTest,
TestScannerKeepAlivePeriodicallyScannerTolerat
// Do a first scan.
ASSERT_TRUE(scanner.HasMoreRows());
KuduScanBatch batch;
- scanner.NextBatch(&batch);
+ ASSERT_OK(scanner.NextBatch(&batch));
ASSERT_TRUE(scanner.HasMoreRows());
// Stop the current tablet server.
KuduTabletServer* kts_ptr;
ASSERT_OK(scanner.GetCurrentServer(&kts_ptr));
unique_ptr<KuduTabletServer> kts(kts_ptr);
- RestartTServerAndWait(kts->uuid());
+ ASSERT_OK(RestartTServerAndWait(kts->uuid()));
bool has_expired_scan = false;
while (scanner.HasMoreRows()) {
@@ -5396,7 +5396,7 @@ TEST_F(ClientTest, TestSoftDeleteAndReserveTable) {
// Insert a few rows, and scan them back. This is to populate the MetaCache.
NO_FATALS(InsertTestRows(client_.get(), client_table_.get(), 10));
vector<string> rows;
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
// Remove the table.
@@ -5472,7 +5472,7 @@ TEST_F(ClientTest, TestSoftDeleteAndReserveTable) {
{
// Read and write are allowed for soft_deleted table.
NO_FATALS(InsertTestRows(client_.get(), client_table_.get(), 20, 10));
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(30, rows.size());
}
@@ -5510,7 +5510,7 @@ TEST_F(ClientTest, TestSoftDeleteAndRecallTable) {
// Insert a few rows, and scan them back. This is to populate the MetaCache.
NO_FATALS(InsertTestRows(client_.get(), client_table_.get(), 10));
vector<string> rows;
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
// Remove the table
@@ -5525,7 +5525,7 @@ TEST_F(ClientTest, TestSoftDeleteAndRecallTable) {
ASSERT_OK(client_->RecallTable(client_table_->id()));
// Check the data in the table.
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
}
@@ -5537,7 +5537,7 @@ TEST_F(ClientTest,
TestSoftDeleteAndRecallTableWithNewTableName) {
// Insert a few rows, and scan them back. This is to populate the MetaCache.
NO_FATALS(InsertTestRows(client_.get(), client_table_.get(), 10));
vector<string> rows;
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
// Remove the table
@@ -5556,7 +5556,7 @@ TEST_F(ClientTest,
TestSoftDeleteAndRecallTableWithNewTableName) {
ASSERT_EQ(old_table_id, client_table_->id());
// Check the data in the table.
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
}
@@ -5567,7 +5567,7 @@ TEST_F(ClientTest,
TestSoftDeleteAndRecallAfterReserveTimeTable) {
// Insert a few rows, and scan them back. This is to populate the MetaCache.
NO_FATALS(InsertTestRows(client_.get(), client_table_.get(), 10));
vector<string> rows;
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
// Remove the table
@@ -5660,7 +5660,7 @@ TEST_F(ClientTest,
TestDeleteWithDeletedTableReserveSeconds) {
// Insert a few rows, and scan them back. This is to populate the MetaCache.
NO_FATALS(InsertTestRows(client_.get(), client_table_.get(), 10));
vector<string> rows;
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
ASSERT_EQ(0, FLAGS_default_deleted_table_reserve_seconds);
@@ -5690,7 +5690,7 @@ TEST_F(ClientTest,
TestDeleteWithDeletedTableReserveSeconds) {
ASSERT_OK(client_->RecallTable(client_table_->id()));
// Check the data in the table.
- ScanTableToStrings(client_table_.get(), &rows);
+ ASSERT_OK(ScanTableToStrings(client_table_.get(), &rows));
ASSERT_EQ(10, rows.size());
// Force to delete the table with
FLAGS_default_deleted_table_reserve_seconds set to 0.
@@ -5926,7 +5926,7 @@ TEST_F(ClientTest,
TestReplicatedTabletWritesWithLeaderElection) {
namespace {
void CheckCorrectness(KuduScanner* scanner, int expected[], int nrows) {
- scanner->Open();
+ ASSERT_OK(scanner->Open());
int readrows = 0;
KuduScanBatch batch;
if (nrows) {
@@ -6110,7 +6110,7 @@ class DLSCallback : public KuduStatusCallback {
int32_t ReadFirstRowKeyFirstCol(const shared_ptr<KuduTable>& tbl) {
KuduScanner scanner(tbl.get());
- scanner.Open();
+ CHECK_OK(scanner.Open());
KuduScanBatch batch;
CHECK(scanner.HasMoreRows());
CHECK_OK(scanner.NextBatch(&batch));
@@ -6123,7 +6123,7 @@ int32_t ReadFirstRowKeyFirstCol(const
shared_ptr<KuduTable>& tbl) {
// Checks that all rows have value equal to expected, return number of rows.
int CheckRowsEqual(const shared_ptr<KuduTable>& tbl, int32_t expected) {
KuduScanner scanner(tbl.get());
- scanner.Open();
+ CHECK_OK(scanner.Open());
KuduScanBatch batch;
int cnt = 0;
while (scanner.HasMoreRows()) {
@@ -6893,7 +6893,7 @@ TEST_F(ClientTest, TestAlterTableChangeOwner) {
.set_owner(kOriginalOwner).Create());
{
shared_ptr<KuduTable> table;
- client_->OpenTable(kTableName, &table);
+ ASSERT_OK(client_->OpenTable(kTableName, &table));
ASSERT_EQ(kOriginalOwner, table->owner());
}
@@ -6902,7 +6902,7 @@ TEST_F(ClientTest, TestAlterTableChangeOwner) {
ASSERT_OK(table_alterer->Alter());
{
shared_ptr<KuduTable> table;
- client_->OpenTable(kTableName, &table);
+ ASSERT_OK(client_->OpenTable(kTableName, &table));
ASSERT_EQ(kNewOwner, table->owner());
}
}
@@ -7918,7 +7918,8 @@ TEST_F(ClientTest, TestRetrieveAuthzTokenInParallel) {
});
}
for (int i = 0 ; i < kThreads; i++) {
- syncs[i].Wait();
+ SCOPED_TRACE(Substitute("thread idx $0", i));
+ ASSERT_OK(syncs[i].Wait());
threads[i].join();
}
SignedTokenPB token;
diff --git a/src/kudu/client/client-unittest.cc
b/src/kudu/client/client-unittest.cc
index d9fb6aa2f..63352c3ce 100644
--- a/src/kudu/client/client-unittest.cc
+++ b/src/kudu/client/client-unittest.cc
@@ -564,7 +564,7 @@ TEST(ClientUnitTest, TestKuduSchemaToStringWithColumnIds) {
// Build a KuduSchema from a Schema, so that the KuduSchema's internal Schema
// has column ids.
SchemaBuilder builder;
- builder.AddKeyColumn("key", DataType::INT32);
+ ASSERT_OK(builder.AddKeyColumn("key", DataType::INT32));
const auto schema = builder.Build();
const auto kudu_schema = KuduSchema::FromSchema(schema);
diff --git a/src/kudu/client/scan_token-internal.cc
b/src/kudu/client/scan_token-internal.cc
index 9e0a86e7c..10465f31d 100644
--- a/src/kudu/client/scan_token-internal.cc
+++ b/src/kudu/client/scan_token-internal.cc
@@ -28,8 +28,6 @@
#include <vector>
#include <glog/logging.h>
-#include <google/protobuf/stubs/common.h>
-#include <google/protobuf/stubs/port.h>
#include "kudu/client/client-internal.h"
#include "kudu/client/client.h"
@@ -189,8 +187,8 @@ Status KuduScanToken::Data::PBIntoScanner(KuduClient*
client,
*mock_resp.add_ts_infos() = std::move(server_pb);
}
- client->data_->meta_cache_->ProcessGetTableLocationsResponse(
- table.get(), partition.begin(), true, mock_resp, &entry, 1);
+
RETURN_NOT_OK(client->data_->meta_cache_->ProcessGetTableLocationsResponse(
+ table.get(), partition.begin(), true, mock_resp, &entry, 1));
}
}
@@ -327,11 +325,11 @@ Status KuduScanToken::Data::PBIntoScanner(KuduClient*
client,
}
if (message.has_scan_request_timeout_ms()) {
- scan_builder->SetTimeoutMillis(message.scan_request_timeout_ms());
+
RETURN_NOT_OK(scan_builder->SetTimeoutMillis(message.scan_request_timeout_ms()));
}
- if (message.has_query_id()) {
- scan_builder->SetQueryId(message.query_id());
+ if (message.has_query_id() && !message.query_id().empty()) {
+ RETURN_NOT_OK(scan_builder->SetQueryId(message.query_id()));
}
*scanner = scan_builder.release();
@@ -391,7 +389,7 @@ Status
KuduScanTokenBuilder::Data::Build(vector<KuduScanToken*>* tokens) {
if (include_table_metadata_) {
for (const ColumnSchema& col : configuration_.projection()->columns()) {
int column_idx;
- table->schema().schema_->FindColumn(col.name(), &column_idx);
+ RETURN_NOT_OK(table->schema().schema_->FindColumn(col.name(),
&column_idx));
pb.mutable_projected_column_idx()->Add(column_idx);
}
} else {
diff --git a/src/kudu/client/scan_token-test.cc
b/src/kudu/client/scan_token-test.cc
index 2275e6595..22c4ea2bd 100644
--- a/src/kudu/client/scan_token-test.cc
+++ b/src/kudu/client/scan_token-test.cc
@@ -320,7 +320,7 @@ class ScanTokenTest : public KuduTest {
Synchronizer sync;
client_->data_->meta_cache_->LookupTabletById(
client_.get(), tablet_id, MonoTime::Max(), &rt,
sync.AsStatusCallback());
- sync.Wait();
+ ASSERT_OK(sync.Wait());
vector<internal::RemoteTabletServer*> tservers;
rt->GetRemoteTabletServers(&tservers);
ASSERT_EQ(3, tservers.size());
@@ -955,7 +955,7 @@ TEST_F(ScanTokenTest,
ScanTokensWithCustomHashSchemasPerRange) {
ASSERT_OK(upper_bound->SetInt64("col", 100));
unique_ptr<KuduRangePartition> range_partition(
new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
- range_partition->add_hash_partitions({ "col" }, 4);
+ ASSERT_OK(range_partition->add_hash_partitions({ "col" }, 4));
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -966,7 +966,7 @@ TEST_F(ScanTokenTest,
ScanTokensWithCustomHashSchemasPerRange) {
ASSERT_OK(upper_bound->SetInt64("col", 200));
unique_ptr<KuduRangePartition> range_partition(
new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
- range_partition->add_hash_partitions({ "col" }, 2);
+ ASSERT_OK(range_partition->add_hash_partitions({ "col" }, 2));
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -988,7 +988,7 @@ TEST_F(ScanTokenTest,
ScanTokensWithCustomHashSchemasPerRange) {
ASSERT_OK(session->Flush());
uint64_t expected_count = 0;
- CheckLiveRowCount("table", &expected_count);
+ ASSERT_OK(CheckLiveRowCount("table", &expected_count));
ASSERT_EQ(expected_count, 200);
{ // no predicates
@@ -1101,7 +1101,7 @@ TEST_F(ScanTokenTest,
TestScanTokensWithCustomHashSchemasPerNonCoveringRange) {
ASSERT_OK(upper_bound->SetInt64("col", 100));
unique_ptr<KuduRangePartition> range_partition(
new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
- range_partition->add_hash_partitions({ "col" }, 4);
+ ASSERT_OK(range_partition->add_hash_partitions({ "col" }, 4));
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -1112,7 +1112,7 @@ TEST_F(ScanTokenTest,
TestScanTokensWithCustomHashSchemasPerNonCoveringRange) {
ASSERT_OK(upper_bound->SetInt64("col", 300));
unique_ptr<KuduRangePartition> range_partition(
new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
- range_partition->add_hash_partitions({ "col" }, 2);
+ ASSERT_OK(range_partition->add_hash_partitions({ "col" }, 2));
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -1139,7 +1139,7 @@ TEST_F(ScanTokenTest,
TestScanTokensWithCustomHashSchemasPerNonCoveringRange) {
ASSERT_OK(session->Flush());
uint64_t expected_count = 0;
- CheckLiveRowCount("table", &expected_count);
+ ASSERT_OK(CheckLiveRowCount("table", &expected_count));
ASSERT_EQ(expected_count, 200);
{ // no predicates
@@ -2021,7 +2021,7 @@ TEST_P(ReplicaSelectionTest, ReplicaSelection) {
// Launch scan requests.
ASSERT_OK(KuduScanTokenBuilder(table.get()).Build(&tokens));
int64_t row_count = 0;
- CountRowsSeq(client_.get(), tokens, &row_count, replica_selection);
+ ASSERT_OK(CountRowsSeq(client_.get(), tokens, &row_count,
replica_selection));
int result = 0;
map<string, int32_t> now_scanner_count_by_ts_uuid;
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index 2ceff0242..dbf59d1d8 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -674,7 +674,7 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) {
num_key_cols = 1;
if (!data_->specs[single_key_col_idx]->data_->primary_key_unique) {
- data_->AddAutoIncrementingColumn(&num_key_cols, &cols);
+ RETURN_NOT_OK(data_->AddAutoIncrementingColumn(&num_key_cols, &cols));
}
} else {
// Build a map from name to index of all of the columns.
@@ -723,7 +723,7 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) {
num_key_cols = key_col_indexes.size();
if (!data_->key_cols_unique) {
- data_->AddAutoIncrementingColumn(&num_key_cols, &cols);
+ RETURN_NOT_OK(data_->AddAutoIncrementingColumn(&num_key_cols, &cols));
}
}
@@ -902,19 +902,20 @@ KuduColumnTypeAttributes
KuduColumnSchema::type_attributes() const {
}
KuduColumnStorageAttributes KuduColumnSchema::storage_attributes() const {
- ColumnStorageAttributes storage_attributes =
DCHECK_NOTNULL(col_)->attributes();
+ const ColumnStorageAttributes sa = DCHECK_NOTNULL(col_)->attributes();
+ // The code below is to convert from the values of PB-based EncodingType and
+ // and CompressionType enums to the corresponding values of enums publicly
+ // exposed via Kudu client API.
KuduColumnStorageAttributes::EncodingType encoding_type;
- KuduColumnStorageAttributes::StringToEncodingType(
- kudu::EncodingType_Name(storage_attributes.encoding),
- &encoding_type);
+ DCHECK_OK(KuduColumnStorageAttributes::StringToEncodingType(
+ kudu::EncodingType_Name(sa.encoding), &encoding_type));
KuduColumnStorageAttributes::CompressionType compression_type;
- KuduColumnStorageAttributes::StringToCompressionType(
- kudu::CompressionType_Name(storage_attributes.compression),
- &compression_type);
+ DCHECK_OK(KuduColumnStorageAttributes::StringToCompressionType(
+ kudu::CompressionType_Name(sa.compression), &compression_type));
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
- return KuduColumnStorageAttributes(encoding_type, compression_type,
- storage_attributes.cfile_block_size);
+ return KuduColumnStorageAttributes(
+ encoding_type, compression_type, sa.cfile_block_size);
#pragma GCC diagnostic pop
}
