You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by la...@apache.org on 2023/01/10 12:01:37 UTC
[kudu] branch master updated: [refactor] fix some tidy warnings on fs module
This is an automated email from the ASF dual-hosted git repository.
laiyingchun 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 57d3abc1f [refactor] fix some tidy warnings on fs module
57d3abc1f is described below
commit 57d3abc1f335e1d6cb6b6d430e84bd18b937500b
Author: Yingchun Lai <la...@apache.org>
AuthorDate: Mon Jan 9 14:19:36 2023 +0800
[refactor] fix some tidy warnings on fs module
There are no functional changes in this patch, but only
some refactors including:
1. remove the default arguments from BlockManager::Open
to mute warnings like:
warning: default arguments on virtual or override methods are prohibited [google-default-arguments]
2. add 'override' and remove 'virtual' for derived
classes destructors of BlockManager to mute warnings
like:
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
Change-Id: I5fc6d78d8eb1468988fe53e64b2fcaed59652ffe
Reviewed-on: http://gerrit.cloudera.org:8080/19406
Reviewed-by: Yuqi Du <sh...@gmail.com>
Tested-by: Yingchun Lai <ac...@gmail.com>
Reviewed-by: Wang Xixu <14...@qq.com>
Reviewed-by: Yifan Zhang <ch...@163.com>
---
src/kudu/fs/block_manager-stress-test.cc | 5 +++--
src/kudu/fs/block_manager-test.cc | 11 ++++++-----
src/kudu/fs/block_manager.h | 4 ++--
src/kudu/fs/file_block_manager.h | 6 +++---
src/kudu/fs/fs_manager.cc | 2 +-
src/kudu/fs/log_block_manager-test.cc | 6 +++---
src/kudu/fs/log_block_manager.h | 6 +++---
7 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/src/kudu/fs/block_manager-stress-test.cc b/src/kudu/fs/block_manager-stress-test.cc
index b9aeae918..4871e442d 100644
--- a/src/kudu/fs/block_manager-stress-test.cc
+++ b/src/kudu/fs/block_manager-stress-test.cc
@@ -24,6 +24,7 @@
#include <ostream>
#include <string>
#include <thread>
+#include <type_traits>
#include <unordered_map>
#include <utility>
#include <vector>
@@ -148,7 +149,7 @@ class BlockManagerStressTest : public KuduTest {
// Defer block manager creation until after the above flags are set.
bm_.reset(CreateBlockManager());
CHECK_OK(file_cache_.Init());
- CHECK_OK(bm_->Open(nullptr));
+ CHECK_OK(bm_->Open(nullptr, nullptr, nullptr));
CHECK_OK(dd_manager_->CreateDataDirGroup(test_tablet_name_));
CHECK_OK(dd_manager_->GetDataDirGroupPB(test_tablet_name_, &test_group_pb_));
}
@@ -541,7 +542,7 @@ TYPED_TEST(BlockManagerStressTest, StressTest) {
LOG(INFO) << "Running on populated block manager (restart #" << i << ")";
this->bm_.reset(this->CreateBlockManager());
FsReport report;
- ASSERT_OK(this->bm_->Open(&report));
+ ASSERT_OK(this->bm_->Open(&report, nullptr, nullptr));
ASSERT_OK(this->dd_manager_->LoadDataDirGroupFromPB(this->test_tablet_name_,
this->test_group_pb_));
ASSERT_OK(report.LogAndCheckForFatalErrors());
diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index 7407e3e5e..5cf7664cb 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -26,6 +26,7 @@
#include <ostream>
#include <string>
#include <thread>
+#include <type_traits>
#include <unordered_set>
#include <vector>
@@ -126,10 +127,10 @@ class BlockManagerTest : public KuduTest {
CHECK_OK(file_cache_.Init());
}
- virtual void SetUp() override {
+ void SetUp() override {
// Pass in a report to prevent the block manager from logging unnecessarily.
FsReport report;
- ASSERT_OK(bm_->Open(&report));
+ ASSERT_OK(bm_->Open(&report, nullptr, nullptr));
ASSERT_OK(dd_manager_->CreateDataDirGroup(test_tablet_name_));
ASSERT_OK(dd_manager_->GetDataDirGroupPB(test_tablet_name_, &test_group_pb_));
}
@@ -190,7 +191,7 @@ class BlockManagerTest : public KuduTest {
env_, paths, opts, &dd_manager_));
}
bm_.reset(CreateBlockManager(metric_entity, parent_mem_tracker));
- RETURN_NOT_OK(bm_->Open(nullptr));
+ RETURN_NOT_OK(bm_->Open(nullptr, nullptr, nullptr));
// Certain tests may maintain their own directory groups, in which case
// the default test group should not be used.
@@ -244,7 +245,7 @@ void BlockManagerTest<LogBlockManager>::SetUp() {
RETURN_NOT_LOG_BLOCK_MANAGER();
// Pass in a report to prevent the block manager from logging unnecessarily.
FsReport report;
- ASSERT_OK(bm_->Open(&report));
+ ASSERT_OK(bm_->Open(&report, nullptr, nullptr));
ASSERT_OK(dd_manager_->CreateDataDirGroup(test_tablet_name_));
// Store the DataDirGroupPB for tests that reopen the block manager.
@@ -745,7 +746,7 @@ TYPED_TEST(BlockManagerTest, PersistenceTest) {
unique_ptr<BlockManager> new_bm(this->CreateBlockManager(
scoped_refptr<MetricEntity>(),
MemTracker::CreateTracker(-1, "other tracker")));
- ASSERT_OK(new_bm->Open(nullptr));
+ ASSERT_OK(new_bm->Open(nullptr, nullptr, nullptr));
// Test that the state of all three blocks is properly reflected.
unique_ptr<ReadableBlock> read_block;
diff --git a/src/kudu/fs/block_manager.h b/src/kudu/fs/block_manager.h
index 1c74e40e1..e3286b061 100644
--- a/src/kudu/fs/block_manager.h
+++ b/src/kudu/fs/block_manager.h
@@ -223,8 +223,8 @@ class BlockManager {
// If 'containers_processed' and 'containers_total' are not nullptr, they will
// be populated with total containers attempted to be opened/processed and
// total containers present respectively.
- virtual Status Open(FsReport* report, std::atomic<int>* containers_processed = nullptr,
- std::atomic<int>* containers_total = nullptr) = 0;
+ virtual Status Open(FsReport* report, std::atomic<int>* containers_processed,
+ std::atomic<int>* containers_total) = 0;
// Creates a new block using the provided options and opens it for
// writing. The block's ID will be generated.
diff --git a/src/kudu/fs/file_block_manager.h b/src/kudu/fs/file_block_manager.h
index f3cadb853..01ba46e77 100644
--- a/src/kudu/fs/file_block_manager.h
+++ b/src/kudu/fs/file_block_manager.h
@@ -78,10 +78,10 @@ class FileBlockManager : public BlockManager {
FileCache* file_cache,
BlockManagerOptions opts);
- virtual ~FileBlockManager();
+ ~FileBlockManager() override;
- Status Open(FsReport* report, std::atomic<int>* containers_processed = nullptr,
- std::atomic<int>* containers_total = nullptr) override;
+ Status Open(FsReport* report, std::atomic<int>* containers_processed,
+ std::atomic<int>* containers_total) override;
Status CreateBlock(const CreateBlockOptions& opts,
std::unique_ptr<WritableBlock>* block) override;
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 68df1e3ca..d2c7231cb 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -547,7 +547,7 @@ Status FsManager::Open(FsReport* report, Timer* read_instance_metadata_files,
InitBlockManager();
LOG_TIMING(INFO, "opening block manager") {
if (opts_.block_manager_type == "file") {
- RETURN_NOT_OK(block_manager_->Open(report));
+ RETURN_NOT_OK(block_manager_->Open(report, nullptr, nullptr));
} else {
RETURN_NOT_OK(block_manager_->Open(report, containers_processed, containers_total));
}
diff --git a/src/kudu/fs/log_block_manager-test.cc b/src/kudu/fs/log_block_manager-test.cc
index d62a42d1e..d3f5cbafc 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -138,7 +138,7 @@ class LogBlockManagerTest : public KuduTest, public ::testing::WithParamInterfac
void SetUp() override {
// Pass in a report to prevent the block manager from logging unnecessarily.
FsReport report;
- ASSERT_OK(bm_->Open(&report));
+ ASSERT_OK(bm_->Open(&report, nullptr, nullptr));
ASSERT_OK(dd_manager_->CreateDataDirGroup(test_tablet_name_));
ASSERT_OK(dd_manager_->GetDataDirGroupPB(test_tablet_name_, &test_group_pb_));
}
@@ -184,7 +184,7 @@ class LogBlockManagerTest : public KuduTest, public ::testing::WithParamInterfac
}
bm_.reset(CreateBlockManager(metric_entity, test_data_dirs));
- RETURN_NOT_OK(bm_->Open(report));
+ RETURN_NOT_OK(bm_->Open(report, nullptr, nullptr));
return Status::OK();
}
@@ -1914,7 +1914,7 @@ TEST_P(LogBlockManagerTest, TestOpenWithFailedDirectories) {
// Check the report, ensuring the correct directory has failed.
FsReport report;
- ASSERT_OK(bm_->Open(&report));
+ ASSERT_OK(bm_->Open(&report, nullptr, nullptr));
ASSERT_EQ(kNumDirs - 1, report.data_dirs.size());
for (const string& data_dir : report.data_dirs) {
ASSERT_NE(data_dir, test_dirs[failed_idx]);
diff --git a/src/kudu/fs/log_block_manager.h b/src/kudu/fs/log_block_manager.h
index a6eba35b1..5d6c0d873 100644
--- a/src/kudu/fs/log_block_manager.h
+++ b/src/kudu/fs/log_block_manager.h
@@ -193,10 +193,10 @@ class LogBlockManager : public BlockManager {
FileCache* file_cache,
BlockManagerOptions opts);
- ~LogBlockManager();
+ ~LogBlockManager() override;
- Status Open(FsReport* report, std::atomic<int>* containers_processed = nullptr,
- std::atomic<int>* containers_total = nullptr) override;
+ Status Open(FsReport* report, std::atomic<int>* containers_processed,
+ std::atomic<int>* containers_total) override;
Status CreateBlock(const CreateBlockOptions& opts,
std::unique_ptr<WritableBlock>* block) override;