You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2025/03/26 15:01:23 UTC
(kudu) 04/04: [tserver] fix result status handling
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 00619af3070a41b8eaba50290807e378a0285152
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Feb 27 18:45:37 2025 -0800
[tserver] fix result status handling
This patch addresses sloppy result handling of functions/methods
that return Status under the src/kudu/tserver directory.
Change-Id: If40eb0a273b13dc3b91817fc74a95a0dd99569a1
Reviewed-on: http://gerrit.cloudera.org:8080/22660
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Marton Greber <gr...@gmail.com>
---
src/kudu/tserver/heartbeater.cc | 4 +-
src/kudu/tserver/tablet_copy_client-test.cc | 12 ++--
.../tserver/tablet_copy_source_session-test.cc | 4 +-
src/kudu/tserver/tablet_server-test-base.cc | 21 +++++-
src/kudu/tserver/tablet_server-test-base.h | 1 +
src/kudu/tserver/tablet_server-test.cc | 82 ++++++++++++----------
.../tserver/tablet_server_authorization-test.cc | 6 +-
src/kudu/tserver/ts_tablet_manager.cc | 3 +-
8 files changed, 80 insertions(+), 53 deletions(-)
diff --git a/src/kudu/tserver/heartbeater.cc b/src/kudu/tserver/heartbeater.cc
index 5a671daf6..d9bd2e9e6 100644
--- a/src/kudu/tserver/heartbeater.cc
+++ b/src/kudu/tserver/heartbeater.cc
@@ -34,8 +34,6 @@
#include <gflags/gflags.h>
#include <glog/logging.h>
-#include <google/protobuf/stubs/common.h>
-#include <google/protobuf/stubs/port.h>
#include "kudu/common/common.pb.h"
#include "kudu/common/wire_protocol.h"
@@ -255,7 +253,7 @@ Status Heartbeater::Start() {
for (int j = 0; j < i; j++) {
// Ignore failures; we should try to stop every thread, and
// 'first_failure' is the most interesting status anyway.
- threads_[j]->Stop();
+ WARN_NOT_OK(threads_[j]->Stop(), "Unable to stop heartbeater thread");
}
return first_failure;
}
diff --git a/src/kudu/tserver/tablet_copy_client-test.cc b/src/kudu/tserver/tablet_copy_client-test.cc
index fadaca536..cdb371078 100644
--- a/src/kudu/tserver/tablet_copy_client-test.cc
+++ b/src/kudu/tserver/tablet_copy_client-test.cc
@@ -256,8 +256,8 @@ Status TabletCopyClientTest::ResetRemoteTabletCopyClient(
scoped_refptr<ConsensusMetadataManager> cmeta_manager(
new ConsensusMetadataManager(fs_manager_.get()));
- tablet_replica_->WaitUntilConsensusRunning(MonoDelta::FromSeconds(10.0));
- rpc::MessengerBuilder(CURRENT_TEST_NAME()).Build(&messenger_);
+ RETURN_NOT_OK(tablet_replica_->WaitUntilConsensusRunning(MonoDelta::FromSeconds(10.0)));
+ RETURN_NOT_OK(rpc::MessengerBuilder(CURRENT_TEST_NAME()).Build(&messenger_));
client_.reset(new RemoteTabletCopyClient(GetTabletId(),
fs_manager_.get(),
cmeta_manager,
@@ -281,7 +281,7 @@ Status TabletCopyClientTest::ResetLocalTabletCopyClient() {
tablet_id_ = tablet_replica_->tablet_id();
// Prepare parameters to create source FsManager.
- rpc::MessengerBuilder(CURRENT_TEST_NAME()).Build(&messenger_);
+ RETURN_NOT_OK(rpc::MessengerBuilder(CURRENT_TEST_NAME()).Build(&messenger_));
FsManagerOpts opts;
string wal = mini_server_->server()->fs_manager()->GetWalsRootDir();
@@ -617,7 +617,7 @@ TEST_P(TabletCopyClientBasicTest, TestFailedDiskStopsClient) {
// metadata directory).
while (true) {
if (rand() % 10 == 0) {
- dd_manager->MarkDirFailed(1, "injected failure in non-client thread");
+ ASSERT_OK(dd_manager->MarkDirFailed(1, "injected failure in non-client thread"));
LOG(INFO) << "INJECTING FAILURE";
break;
}
@@ -686,7 +686,7 @@ void TabletCopyClientAbortTest::CreateTestBlocks(int num_blocks) {
for (int i = 0; i < num_blocks; i++) {
unique_ptr<fs::WritableBlock> block;
ASSERT_OK(fs_manager_->CreateNewBlock({}, &block));
- block->Append("Test");
+ ASSERT_OK(block->Append("Test"));
ASSERT_OK(block->Close());
}
}
@@ -765,7 +765,7 @@ TEST_P(TabletCopyClientAbortTest, TestAbort) {
ASSERT_EQ(tablet::TABLET_DATA_TOMBSTONED, meta->tablet_data_state());
ASSERT_FALSE(fs_manager_->Exists(wal_path));
vector<BlockId> latest_blocks;
- fs_manager_->block_manager()->GetAllBlockIds(&latest_blocks);
+ ASSERT_OK(fs_manager_->block_manager()->GetAllBlockIds(&latest_blocks));
ASSERT_EQ(local_block_ids.size(), latest_blocks.size());
}
for (const auto& block_id : local_block_ids) {
diff --git a/src/kudu/tserver/tablet_copy_source_session-test.cc b/src/kudu/tserver/tablet_copy_source_session-test.cc
index 03811dce0..614741c07 100644
--- a/src/kudu/tserver/tablet_copy_source_session-test.cc
+++ b/src/kudu/tserver/tablet_copy_source_session-test.cc
@@ -23,6 +23,7 @@
#include <memory>
#include <ostream>
#include <string>
+#include <type_traits>
#include <utility>
#include <vector>
@@ -54,7 +55,6 @@
#include "kudu/tablet/ops/op.h"
#include "kudu/tablet/ops/write_op.h"
#include "kudu/tablet/tablet-test-util.h"
-#include "kudu/tablet/tablet.h"
#include "kudu/tablet/tablet_metadata.h"
#include "kudu/tablet/tablet_replica.h"
#include "kudu/tserver/tablet_copy.pb.h"
@@ -176,7 +176,7 @@ class TabletCopyTest : public KuduTabletTest,
shared_ptr<Messenger> messenger;
MessengerBuilder mbuilder(CURRENT_TEST_NAME());
- mbuilder.Build(&messenger);
+ ASSERT_OK(mbuilder.Build(&messenger));
log_anchor_registry_.reset(new LogAnchorRegistry());
tablet_replica_->SetBootstrapping();
diff --git a/src/kudu/tserver/tablet_server-test-base.cc b/src/kudu/tserver/tablet_server-test-base.cc
index 6f9aef844..7ca496c48 100644
--- a/src/kudu/tserver/tablet_server-test-base.cc
+++ b/src/kudu/tserver/tablet_server-test-base.cc
@@ -23,6 +23,7 @@
#include <iostream>
#include <memory>
#include <string>
+#include <type_traits>
#include <vector>
#include <gflags/gflags.h>
@@ -384,6 +385,24 @@ void TabletServerTestBase::ShutdownTablet() {
}
}
+Status TabletServerTestBase::ShutdownAndRebuildDataDirs(int num_data_dirs) {
+ ShutdownTablet();
+
+ // Start server.
+ mini_server_.reset(new MiniTabletServer(GetTestPath("TabletServerTest-fsroot"),
+ HostPort("127.0.0.1", 0), num_data_dirs));
+ mini_server_->options()->master_addresses.clear();
+ mini_server_->options()->master_addresses.emplace_back("255.255.255.255", 1);
+ // this should open the tablet created on StartTabletServer()
+ RETURN_NOT_OK(mini_server_->Start());
+
+ RETURN_NOT_OK(mini_server_->WaitStarted());
+
+ // Connect to the server.
+ ResetClientProxies();
+ return Status::OK();
+}
+
Status TabletServerTestBase::ShutdownAndRebuildTablet(int num_data_dirs) {
ShutdownTablet();
@@ -405,7 +424,7 @@ Status TabletServerTestBase::ShutdownAndRebuildTablet(int num_data_dirs) {
return Status::NotFound("Tablet was not found");
}
- // Connect to it.
+ // Connect to the server.
ResetClientProxies();
// Opening a tablet is async, we wait here instead of having to handle errors later.
diff --git a/src/kudu/tserver/tablet_server-test-base.h b/src/kudu/tserver/tablet_server-test-base.h
index 43478e873..8afd0b37a 100644
--- a/src/kudu/tserver/tablet_server-test-base.h
+++ b/src/kudu/tserver/tablet_server-test-base.h
@@ -108,6 +108,7 @@ class TabletServerTestBase : public KuduTest {
void ShutdownTablet();
+ Status ShutdownAndRebuildDataDirs(int num_data_dirs);
Status ShutdownAndRebuildTablet(int num_data_dirs = 1);
// Verifies that a set of expected rows (key, value) is present in the tablet.
diff --git a/src/kudu/tserver/tablet_server-test.cc b/src/kudu/tserver/tablet_server-test.cc
index c0d5f2a97..b58be42c1 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -785,7 +785,7 @@ TEST_P(TabletServerDiskSpaceTest, TestFullGroupAddsDir) {
// Otherwise the 'mini_server_' will fail starting later because of the failure to acquire
// the RocksDB LOCK file. Reset it in any case, there is no side effect.
dd_manager.reset();
- NO_FATALS(ShutdownAndRebuildTablet(kNumDirs));
+ ASSERT_OK(ShutdownAndRebuildTablet(kNumDirs));
dd_manager = mini_server_->server()->fs_manager()->dd_manager();
ASSERT_OK(dd_manager->FindDataDirsByTabletId(kTabletId, &dir_group));
ASSERT_EQ(kNumDirs, dir_group.size());
@@ -872,7 +872,7 @@ TEST_F(TabletServerStartupWebPageTest, TestStartupWebPage) {
const string url = Substitute("http://$0/startup", mini_server_->bound_http_addr().ToString());
// Verify if the startup status is complete.
- mini_server_->WaitStarted();
+ ASSERT_OK(mini_server_->WaitStarted());
EasyCurl c;
faststring buf;
@@ -905,8 +905,11 @@ TEST_F(TabletServerStartupWebPageTest, TestStartupWebPage) {
status_reader.join();
});
- mini_server_->Start();
- mini_server_->WaitStarted();
+ ASSERT_OK(mini_server_->Start());
+ const auto s = mini_server_->WaitStarted();
+ ASSERT_TRUE(s.IsIOError()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(), "some tablet data is in a failed directory");
+
run_status_reader = false;
// After the server has startup up, ensure every startup step has 100 percent status.
@@ -920,7 +923,7 @@ TEST_F(TabletServerStartupWebPageTest, TestAggregateStartupMetrics) {
string addr = mini_server_->bound_http_addr().ToString();
// Verify the startup steps remaining are zero once the server is started up.
- mini_server_->WaitStarted();
+ ASSERT_OK(mini_server_->WaitStarted());
ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics?metrics=startup_progress_steps_remaining",
addr),
&buf));
@@ -936,7 +939,7 @@ TEST_F(TabletServerStartupWebPageTest, TestTabletsMetrics) {
EasyCurl c;
faststring buf;
// Verify if the startup status is complete.
- mini_server_->WaitStarted();
+ ASSERT_OK(mini_server_->WaitStarted());
const string url = Substitute("http://$0/metrics", mini_server_->bound_http_addr().ToString());
// Validate populated metrics in case of zero tablets during startup.
ASSERT_OK(c.FetchURL(url, &buf));
@@ -966,7 +969,7 @@ TEST_F(TabletServerStartupWebPageTest, TestLogBlockContainerMetrics) {
EasyCurl c;
faststring buf;
- mini_server_->WaitStarted();
+ ASSERT_OK(mini_server_->WaitStarted());
string addr = mini_server_->bound_http_addr().ToString();
// Validate populated metrics in case of zero containers during startup.
ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics", addr), &buf));
@@ -1021,7 +1024,7 @@ class TabletServerMaintenanceManagerWebPageTest: public TabletServerStartupWebPa
};
TEST_F(TabletServerMaintenanceManagerWebPageTest, OpDataRetainedStats) {
- mini_server_->WaitStarted();
+ ASSERT_OK(mini_server_->WaitStarted());
faststring buf;
ASSERT_OK(EasyCurl().FetchURL(Substitute("http://$0/maintenance-manager",
@@ -1196,7 +1199,7 @@ TEST_F(TabletServerTest, TestAddRemoveDirectory) {
}
// Start with multiple data dirs so the dirs are suffixed with numbers, and
// so when we remove a data dirs, we'll be using the same set of dirs.
- NO_FATALS(ShutdownAndRebuildTablet(/*num_data_dirs*/2));
+ ASSERT_OK(ShutdownAndRebuildDataDirs(/*num_data_dirs*/2));
const char* kFooTablet1 = "fffffffffffffffffffffffffffffff1";
ASSERT_OK(mini_server_->AddTestTablet("footable", kFooTablet1, schema_));
ASSERT_OK(WaitForTabletRunning(kFooTablet1));
@@ -1204,7 +1207,7 @@ TEST_F(TabletServerTest, TestAddRemoveDirectory) {
// Shut down and restart with a new directory. This is allowed, and the
// tablet server will be able to use the new directory if we create a new
// tablet.
- NO_FATALS(ShutdownAndRebuildTablet(/*num_data_dirs*/3));
+ ASSERT_OK(ShutdownAndRebuildDataDirs(/*num_data_dirs*/3));
const char* kFooTablet2 = "fffffffffffffffffffffffffffffff2";
ASSERT_OK(mini_server_->AddTestTablet("footable", kFooTablet2, schema_));
ASSERT_OK(WaitForTabletRunning(kFooTablet2));
@@ -1212,7 +1215,12 @@ TEST_F(TabletServerTest, TestAddRemoveDirectory) {
// Now open up again with a our original two directories. The second tablet
// should fail because it should have been striped across the third
// directory. The first tablet should be unaffected.
- NO_FATALS(ShutdownAndRebuildTablet(/*num_data_dirs*/2));
+ const auto s = ShutdownAndRebuildDataDirs(/*num_data_dirs*/2);
+ ASSERT_TRUE(s.IsNotFound()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(),
+ "error retrieving tablet data dir group (one or more data dirs "
+ "may have been removed): could not find data dir group for tablet "
+ "fffffffffffffffffffffffffffffff2");
ASSERT_EVENTUALLY([&] {
scoped_refptr<TabletReplica> replica1;
ASSERT_TRUE(mini_server_->server()->tablet_manager()->LookupTablet(kFooTablet1, &replica1));
@@ -1429,7 +1437,7 @@ TEST_F(TabletServerTest, TestInsert) {
Timestamp now_before = mini_server_->server()->clock()->Now();
rows_inserted = nullptr;
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(schema_, { KeyValue(1, 1), KeyValue(2, 1), KeyValue(1234, 5678) });
// get the clock's timestamp after replay
@@ -1718,7 +1726,7 @@ TEST_F(TabletServerTest, TestInsertAndMutate) {
rows_inserted = nullptr;
rows_updated = nullptr;
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(schema_, { KeyValue(2, 3), KeyValue(3, 4), KeyValue(4, 4), KeyValue(6, 6) });
// get the clock's timestamp after replay
@@ -1919,7 +1927,7 @@ TEST_F(TabletServerTest, TestRecoveryWithMutationsWhileFlushing) {
// Shutdown the tserver and try and rebuild the tablet from the log
// produced on recovery (recovery flushed no state, but produced a new
// log).
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(schema_, { KeyValue(1, 10),
KeyValue(2, 20),
KeyValue(3, 30),
@@ -1932,7 +1940,7 @@ TEST_F(TabletServerTest, TestRecoveryWithMutationsWhileFlushing) {
// Shutdown and rebuild again to test that the log generated during
// the previous recovery allows to perform recovery again.
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(schema_, { KeyValue(1, 10),
KeyValue(2, 20),
KeyValue(3, 30),
@@ -1957,7 +1965,7 @@ TEST_F(TabletServerTest, TestRecoveryWithMutationsWhileFlushingAndCompacting) {
// flush the first time
ASSERT_OK(tablet_replica_->tablet()->Flush());
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(schema_, { KeyValue(1, 10),
KeyValue(2, 20),
KeyValue(3, 30),
@@ -1997,7 +2005,7 @@ TEST_F(TabletServerTest, TestRecoveryWithMutationsWhileFlushingAndCompacting) {
// Shutdown the tserver and try and rebuild the tablet from the log
// produced on recovery (recovery flushed no state, but produced a new
// log).
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(schema_, { KeyValue(1, 11),
KeyValue(2, 22),
KeyValue(3, 32),
@@ -2129,7 +2137,7 @@ TEST_F(TabletServerTest, TestExactlyOnceForErrorsAcrossRestart) {
req.clear_row_operations();
for (int i = 1; i <= 5; i++) {
SCOPED_TRACE(Substitute("restart attempt #$0", i));
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
rpc.Reset();
req_id.set_attempt_no(req_id.attempt_no() + 1);
rpc.SetRequestIdPB(unique_ptr<rpc::RequestIdPB>(new rpc::RequestIdPB(req_id)));
@@ -2374,7 +2382,7 @@ TEST_F(ScannerScansTest, TestScanMetricsAffectedOnOrderedScan) {
SchemaBuilder sb;
for (int i = schema_.num_key_columns(); i < schema_.num_columns(); i++) {
- sb.AddColumn(schema_.column(i), false);
+ ASSERT_OK(sb.AddColumn(schema_.column(i), false));
}
const Schema& projection = sb.BuildWithoutIds();
DoOrderedScanTest(projection, R"((int32 int_val=$1, string string_val="hello $0"))");
@@ -2535,7 +2543,7 @@ TEST_P(ScanCorruptedDeltasParamTest, Test) {
// Fudge with some delta blocks.
TabletSuperBlockPB superblock_pb;
- tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock_pb);
+ ASSERT_OK(tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock_pb));
FsManager* fs_manager = mini_server_->server()->fs_manager();
for (int rowset_no = 0; rowset_no < superblock_pb.rowsets_size(); rowset_no++) {
RowSetDataPB* rowset_pb = superblock_pb.mutable_rowsets(rowset_no);
@@ -2813,7 +2821,7 @@ TEST_F(ScannerScansTest, TestSnapshotScan_LastRow) {
// to the client.
SchemaBuilder sb(schema_);
for (int i = 0; i < schema_.num_key_columns(); i++) {
- sb.RemoveColumn(schema_.column(i).name());
+ ASSERT_OK(sb.RemoveColumn(schema_.column(i).name()));
}
const Schema& projection = sb.BuildWithoutIds();
@@ -3425,7 +3433,7 @@ TEST_F(ScannerScansTest, TestScanWithSimplifiablePredicates) {
// Set up a projection without the key columns or the column after the last key column
SchemaBuilder sb(schema_);
for (int i = 0; i <= schema_.num_key_columns(); i++) {
- sb.RemoveColumn(schema_.column(i).name());
+ ASSERT_OK(sb.RemoveColumn(schema_.column(i).name()));
}
const Schema& projection = sb.BuildWithoutIds();
ASSERT_OK(SchemaToColumnPBs(projection, scan->mutable_projected_columns()));
@@ -3975,7 +3983,7 @@ TEST_F(ScannerScansTest, TestOrderedScan_ProjectionWithKeyColumnsInOrder) {
// Build a projection with all the columns, but don't mark the key columns as such.
SchemaBuilder sb;
for (int i = 0; i < schema_.num_columns(); i++) {
- sb.AddColumn(schema_.column(i), false);
+ ASSERT_OK(sb.AddColumn(schema_.column(i), false));
}
const Schema& projection = sb.BuildWithoutIds();
DoOrderedScanTest(projection,
@@ -3987,7 +3995,7 @@ TEST_F(ScannerScansTest, TestOrderedScan_ProjectionWithoutKeyColumns) {
// Build a projection without the key columns.
SchemaBuilder sb;
for (int i = schema_.num_key_columns(); i < schema_.num_columns(); i++) {
- sb.AddColumn(schema_.column(i), false);
+ ASSERT_OK(sb.AddColumn(schema_.column(i), false));
}
const Schema& projection = sb.BuildWithoutIds();
DoOrderedScanTest(projection, R"((int32 int_val=$1, string string_val="hello $0"))");
@@ -3998,7 +4006,7 @@ TEST_F(ScannerScansTest, TestOrderedScan_ProjectionWithKeyColumnsOutOfOrder) {
// Build a projection with the order of the columns reversed.
SchemaBuilder sb;
for (int i = schema_.num_columns() - 1; i >= 0; i--) {
- sb.AddColumn(schema_.column(i), false);
+ ASSERT_OK(sb.AddColumn(schema_.column(i), false));
}
const Schema& projection = sb.BuildWithoutIds();
DoOrderedScanTest(projection,
@@ -4068,14 +4076,14 @@ TEST_F(TabletServerTest, TestAlterSchema) {
const Schema projection({ ColumnSchema("key", INT32), (ColumnSchema("c2", INT32)) }, 1);
// Try recovering from the original log
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(projection, { KeyValue(0, 7),
KeyValue(1, 7),
KeyValue(2, 5),
KeyValue(3, 5) });
// Try recovering from the log generated on recovery
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(projection, { KeyValue(0, 7),
KeyValue(1, 7),
KeyValue(2, 5),
@@ -4119,11 +4127,11 @@ TEST_F(TabletServerTest, TestAlterSchema_AddColWithoutWriteDefault) {
VerifyRows(projection, { KeyValue(0, 7), KeyValue(1, 7) });
// Try recovering from the original log
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(projection, { KeyValue(0, 7), KeyValue(1, 7) });
// Try recovering from the log generated on recovery
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
VerifyRows(projection, { KeyValue(0, 7), KeyValue(1, 7) });
}
@@ -4684,7 +4692,7 @@ TEST_F(TabletServerTest, TestFailedDnsResolution) {
TEST_F(TabletServerTest, TestDataDirGroupsCreated) {
// Get the original superblock.
TabletSuperBlockPB superblock;
- tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock);
+ ASSERT_OK(tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock));
DataDirGroupPB orig_group = superblock.data_dir_group();
// Remove the DataDirGroupPB on-disk.
@@ -4703,7 +4711,7 @@ TEST_F(TabletServerTest, TestDataDirGroupsCreated) {
// group will be created with all data directories and should be identical to
// the original one.
ASSERT_OK(ShutdownAndRebuildTablet());
- tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock);
+ ASSERT_OK(tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock));
DataDirGroupPB new_group = superblock.data_dir_group();
MessageDifferencer md;
ASSERT_TRUE(md.Compare(orig_group, new_group));
@@ -4827,7 +4835,7 @@ TEST_F(TabletServerTest, SetEncodedKeysWhenStartingUp) {
// maintenance operations when we restart.
FLAGS_enable_maintenance_manager = false;
TabletSuperBlockPB superblock_pb;
- tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock_pb);
+ ASSERT_OK(tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock_pb));
// Make sure the min/max key do not exist in the rowset metadata.
for (int rowset_no = 0; rowset_no < superblock_pb.rowsets_size(); rowset_no++) {
RowSetDataPB* rowset_pb = superblock_pb.mutable_rowsets(rowset_no);
@@ -4881,7 +4889,7 @@ TEST_F(TabletServerTest, SetEncodedKeysWhenStartingUp) {
}
TabletSuperBlockPB superblock_pb;
- tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock_pb);
+ ASSERT_OK(tablet_replica_->tablet()->metadata()->ToSuperBlock(&superblock_pb));
for (int rowset_no = 0; rowset_no < superblock_pb.rowsets_size(); rowset_no++) {
RowSetDataPB* rowset_pb = superblock_pb.mutable_rowsets(rowset_no);
if (set_keys_during_restart) {
@@ -4990,7 +4998,7 @@ TEST_F(TabletServerTest, TestScannerCheckMatchingUser) {
TEST_F(TabletServerTest, TestTimeBasedFlushDMS) {
SKIP_IF_SLOW_NOT_ALLOWED();
FLAGS_enable_maintenance_manager = false;
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
// We're going to generate a bunch of DMSs, and for that, we need multiple
// rowsets, so disable merge compactions.
FLAGS_enable_rowset_compaction = false;
@@ -5011,7 +5019,7 @@ TEST_F(TabletServerTest, TestTimeBasedFlushDMS) {
FLAGS_enable_maintenance_manager = true;
FLAGS_maintenance_manager_polling_interval_ms = 1;
FLAGS_flush_threshold_secs = 1;
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
// We should be able to wait for the flush threshold and very quickly see all
// DMSs get flushed. We'll wait 5x the flush threshold, which should be ample
// time given how little we're writing.
@@ -5025,7 +5033,7 @@ TEST_F(TabletServerTest, TestTimeBasedFlushDMS) {
TEST_F(TabletServerTest, StartupWithConcurrentOpsBenchmark) {
SKIP_IF_SLOW_NOT_ALLOWED();
FLAGS_enable_maintenance_manager = false;
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
// Create a bunch of tablets with a bunch of rowsets.
ObjectIdGenerator oid;
@@ -5060,7 +5068,7 @@ TEST_F(TabletServerTest, StartupWithConcurrentOpsBenchmark) {
TEST_F(TabletServerTest, TestStarvePerfImprovementOpsInColdTablet) {
SKIP_IF_SLOW_NOT_ALLOWED();
FLAGS_enable_maintenance_manager = true;
- NO_FATALS(ShutdownAndRebuildTablet());
+ ASSERT_OK(ShutdownAndRebuildTablet());
// Disable flushing and compactions to create overlapping rowsets.
FLAGS_enable_flush_memrowset = false;
diff --git a/src/kudu/tserver/tablet_server_authorization-test.cc b/src/kudu/tserver/tablet_server_authorization-test.cc
index 6ec8c43bd..1255505b7 100644
--- a/src/kudu/tserver/tablet_server_authorization-test.cc
+++ b/src/kudu/tserver/tablet_server_authorization-test.cc
@@ -504,13 +504,13 @@ class ScanPrivilegeAuthzTest : public AuthzTabletServerTestBase,
SchemaBuilder schema_builder;
for (int i = 0; i < kNumKeys; i++) {
const string key = Substitute("key$0", i);
- schema_builder.AddKeyColumn(key, DataType::INT32);
+ ASSERT_OK(schema_builder.AddKeyColumn(key, DataType::INT32));
col_names_.emplace_back(key);
}
for (int i = 0; i < kNumVals; i++) {
const string val = Substitute("val$0", kNumKeys + i);
- schema_builder.AddColumn(ColumnSchema(val, DataType::INT32),
- /*is_key=*/false);
+ ASSERT_OK(schema_builder.AddColumn(ColumnSchema(val, DataType::INT32),
+ /*is_key=*/false));
col_names_.emplace_back(val);
}
schema_ = schema_builder.Build();
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index b56524dde..f7f3269c1 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -1969,7 +1969,8 @@ void TSTabletManager::FailTabletsInDataDir(const string& uuid,
return;
}
// Fail the directory to prevent other tablets from being placed in it.
- dd_manager->MarkDirFailed(uuid_idx);
+ WARN_NOT_OK(dd_manager->MarkDirFailed(uuid_idx),
+ "error marking data directory as failed");
set<string> tablets = dd_manager->FindTabletsByDirUuidIdx(uuid_idx);
LOG(INFO) << Substitute("Data dir $0 has $1 tablets", uuid, tablets.size());
for (const string& tablet_id : dd_manager->FindTabletsByDirUuidIdx(uuid_idx)) {