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/02/16 07:43:18 UTC
[kudu] branch master updated: KUDU-3371 [fs] make LogBlockManager a base class
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 4883497ce KUDU-3371 [fs] make LogBlockManager a base class
4883497ce is described below
commit 4883497ce072bcb5a904275662c8f855e5cbcfcb
Author: Yingchun Lai <ac...@gmail.com>
AuthorDate: Thu Jul 7 00:32:07 2022 +0800
KUDU-3371 [fs] make LogBlockManager a base class
This patch makes LogBlockManager a base class for the newly
added LogBlockManagerNativeMeta.
FileMetaLogBlockManager is the log-backed block manager which
manages the sequential written file to store containers' metadata,
it is how we do as before.
Most of the member functions are left in the base class LogBlockManager
to avoid too large updating on existing code, I'm planning to do
that in next patches.
Change-Id: I59c1287a9539668e0c08036452bb96a3006ed356
Reviewed-on: http://gerrit.cloudera.org:8080/18709
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
---
src/kudu/fs/block_manager-stress-test.cc | 6 +--
src/kudu/fs/block_manager-test.cc | 17 +++++---
src/kudu/fs/fs_manager.cc | 8 ++--
src/kudu/fs/log_block_manager-test.cc | 6 ++-
src/kudu/fs/log_block_manager.cc | 9 ++---
src/kudu/fs/log_block_manager.h | 69 +++++++++++++++++++-------------
6 files changed, 69 insertions(+), 46 deletions(-)
diff --git a/src/kudu/fs/block_manager-stress-test.cc b/src/kudu/fs/block_manager-stress-test.cc
index 4871e442d..f8f042836 100644
--- a/src/kudu/fs/block_manager-stress-test.cc
+++ b/src/kudu/fs/block_manager-stress-test.cc
@@ -481,7 +481,7 @@ int BlockManagerStressTest<FileBlockManager>::GetMaxFdCount() const {
}
template <>
-int BlockManagerStressTest<LogBlockManager>::GetMaxFdCount() const {
+int BlockManagerStressTest<LogBlockManagerNativeMeta>::GetMaxFdCount() const {
return FLAGS_max_open_files +
// If all containers are full, each open block could theoretically
// result in a new container, which is two files briefly outside the
@@ -495,7 +495,7 @@ void BlockManagerStressTest<FileBlockManager>::InjectNonFatalInconsistencies() {
}
template <>
-void BlockManagerStressTest<LogBlockManager>::InjectNonFatalInconsistencies() {
+void BlockManagerStressTest<LogBlockManagerNativeMeta>::InjectNonFatalInconsistencies() {
LBMCorruptor corruptor(env_, dd_manager_->GetDirs(), rand_seed_);
ASSERT_OK(corruptor.Init());
@@ -506,7 +506,7 @@ void BlockManagerStressTest<LogBlockManager>::InjectNonFatalInconsistencies() {
// What kinds of BlockManagers are supported?
#if defined(__linux__)
-typedef ::testing::Types<FileBlockManager, LogBlockManager> BlockManagers;
+typedef ::testing::Types<FileBlockManager, LogBlockManagerNativeMeta> BlockManagers;
#else
typedef ::testing::Types<FileBlockManager> BlockManagers;
#endif
diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index 5cf7664cb..433c5f6e0 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -113,7 +113,7 @@ template<>
string block_manager_type<FileBlockManager>() { return "file"; }
template<>
-string block_manager_type<LogBlockManager>() { return "log"; }
+string block_manager_type<LogBlockManagerNativeMeta>() { return "log"; }
template <typename T>
class BlockManagerTest : public KuduTest {
@@ -230,6 +230,10 @@ class BlockManagerTest : public KuduTest {
});
}
+ ~BlockManagerTest() override {
+ dd_manager_->WaitOnClosures();
+ }
+
// Keep an internal copy of the data dir group to act as metadata.
DataDirGroupPB test_group_pb_;
string test_tablet_name_;
@@ -241,7 +245,7 @@ class BlockManagerTest : public KuduTest {
};
template <>
-void BlockManagerTest<LogBlockManager>::SetUp() {
+void BlockManagerTest<LogBlockManagerNativeMeta>::SetUp() {
RETURN_NOT_LOG_BLOCK_MANAGER();
// Pass in a report to prevent the block manager from logging unnecessarily.
FsReport report;
@@ -285,7 +289,8 @@ void BlockManagerTest<FileBlockManager>::RunBlockDistributionTest(const vector<s
}
template <>
-void BlockManagerTest<LogBlockManager>::RunBlockDistributionTest(const vector<string>& paths) {
+void BlockManagerTest<LogBlockManagerNativeMeta>::RunBlockDistributionTest(
+ const vector<string>& paths) {
vector<int> files_in_each_path(paths.size());
int num_blocks_per_dir = 30;
// Spread across 1, then 3, then 5 data directories.
@@ -364,7 +369,7 @@ void BlockManagerTest<FileBlockManager>::RunMultipathTest(const vector<string>&
}
template <>
-void BlockManagerTest<LogBlockManager>::RunMultipathTest(const vector<string>& paths) {
+void BlockManagerTest<LogBlockManagerNativeMeta>::RunMultipathTest(const vector<string>& paths) {
// Write (3 * numPaths * 2) blocks, in groups of (numPaths * 2). That should
// yield two containers per path.
CreateBlockOptions opts({ "multipath_test" });
@@ -414,7 +419,7 @@ void BlockManagerTest<FileBlockManager>::RunMemTrackerTest() {
}
template <>
-void BlockManagerTest<LogBlockManager>::RunMemTrackerTest() {
+void BlockManagerTest<LogBlockManagerNativeMeta>::RunMemTrackerTest() {
shared_ptr<MemTracker> tracker = MemTracker::CreateTracker(-1, "test tracker");
ASSERT_OK(ReopenBlockManager(scoped_refptr<MetricEntity>(),
tracker,
@@ -434,7 +439,7 @@ void BlockManagerTest<LogBlockManager>::RunMemTrackerTest() {
// What kinds of BlockManagers are supported?
#if defined(__linux__)
-typedef ::testing::Types<FileBlockManager, LogBlockManager> BlockManagers;
+typedef ::testing::Types<FileBlockManager, LogBlockManagerNativeMeta> BlockManagers;
#else
typedef ::testing::Types<FileBlockManager> BlockManagers;
#endif
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 6d9b9f4b9..9f52001f5 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -172,7 +172,7 @@ using kudu::fs::ErrorNotificationCb;
using kudu::fs::FsErrorManager;
using kudu::fs::FileBlockManager;
using kudu::fs::FsReport;
-using kudu::fs::LogBlockManager;
+using kudu::fs::LogBlockManagerNativeMeta;
using kudu::fs::ReadableBlock;
using kudu::fs::UpdateInstanceBehavior;
using kudu::fs::WritableBlock;
@@ -380,9 +380,11 @@ void FsManager::InitBlockManager() {
if (opts_.block_manager_type == "file") {
block_manager_.reset(new FileBlockManager(
env_, dd_manager_.get(), error_manager_.get(), opts_.file_cache, std::move(bm_opts)));
- } else {
- block_manager_.reset(new LogBlockManager(
+ } else if (opts_.block_manager_type == "log") {
+ block_manager_.reset(new LogBlockManagerNativeMeta(
env_, dd_manager_.get(), error_manager_.get(), opts_.file_cache, std::move(bm_opts)));
+ } else {
+ LOG(FATAL) << "Unknown block_manager_type: " << opts_.block_manager_type;
}
}
diff --git a/src/kudu/fs/log_block_manager-test.cc b/src/kudu/fs/log_block_manager-test.cc
index d3f5cbafc..2143df510 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -85,6 +85,7 @@ DECLARE_double(log_container_excess_space_before_cleanup_fraction);
DECLARE_double(log_container_live_metadata_before_compact_ratio);
DECLARE_int32(fs_target_data_dirs_per_tablet);
DECLARE_int64(log_container_max_blocks);
+DECLARE_string(block_manager);
DECLARE_string(block_manager_preflush_control);
DECLARE_string(env_inject_eio_globs);
DECLARE_uint64(log_container_preallocate_bytes);
@@ -156,8 +157,9 @@ class LogBlockManagerTest : public KuduTest, public ::testing::WithParamInterfac
BlockManagerOptions opts;
opts.metric_entity = metric_entity;
- return new LogBlockManager(env_, dd_manager_.get(), &error_manager_,
- &file_cache_, std::move(opts));
+ CHECK_EQ(FLAGS_block_manager, "log");
+ return new LogBlockManagerNativeMeta(
+ env_, dd_manager_.get(), &error_manager_, &file_cache_, std::move(opts));
}
Status ReopenBlockManager(const scoped_refptr<MetricEntity>& metric_entity = nullptr,
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index 02a13618a..2f9639344 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -434,9 +434,9 @@ class LogBlockContainer: public RefCountedThreadSafe<LogBlockContainer> {
// Opens an existing block container in 'dir'.
//
- // Every container is comprised of two files: "<dir>/<id>.data" and
- // "<dir>/<id>.metadata". Together, 'dir' and 'id' fully describe both
- // files.
+ // Every container is comprised of two parts: "<dir>/<id>.data" and
+ // metadata (e.g. "<dir>/<id>.metadata"). Together, 'dir' and 'id'
+ // fully describe both files.
//
// Returns Status::Aborted() in the case that the metadata and data files
// both appear to have no data (e.g. due to a crash just after creating
@@ -3116,8 +3116,7 @@ Status LogBlockManager::Repair(
// leftover container files.
if (report->incomplete_container_check) {
for (auto& ic : report->incomplete_container_check->entries) {
- Status s = env_->DeleteFile(
- StrCat(ic.container, kContainerMetadataFileSuffix));
+ Status s = env_->DeleteFile(StrCat(ic.container, kContainerMetadataFileSuffix));
if (!s.ok() && !s.IsNotFound()) {
WARN_NOT_OK_LBM_DISK_FAILURE(s, "could not delete incomplete container metadata file");
}
diff --git a/src/kudu/fs/log_block_manager.h b/src/kudu/fs/log_block_manager.h
index 5d6c0d873..828b0d9de 100644
--- a/src/kudu/fs/log_block_manager.h
+++ b/src/kudu/fs/log_block_manager.h
@@ -78,20 +78,18 @@ typedef scoped_refptr<internal::LogBlockContainer> LogBlockContainerRefPtr;
// Implementation details
// ----------------------
//
-// A container is comprised of two files, one for metadata and one for
-// data. Both are written to sequentially. During a write, the block's data
-// is written as-is to the data file. After the block has been
-// synchronized, a small record is written to the metadata file containing
-// the block's ID and its location within the data file.
+// A container is comprised of two parts, one for metadata and one for
+// data. During a write, the block's data is written as-is to the data file.
+// After the block has been synchronized, a small record is written to the
+// metadata part containing the block's ID and its location within the data
+// file.
//
// Block deletions are handled similarly. When a block is deleted, a record
// is written describing the deletion, orphaning the old block data. The
// orphaned data can be reclaimed instantaneously via hole punching, or
// later via garbage collection. The latter is used when hole punching is
// not supported on the filesystem, or on next boot if there's a crash
-// after deletion but before hole punching. The metadata file itself is not
-// compacted, as it is expected to remain quite small even after a great
-// many create/delete cycles.
+// after deletion but before hole punching.
//
// Data and metadata operations are carefully ordered to ensure the
// correctness of the persistent representation at all times. During the
@@ -150,15 +148,7 @@ typedef scoped_refptr<internal::LogBlockContainer> LogBlockContainerRefPtr;
// collected every now and then, though newer systems can take advantage of
// filesystem hole punching (as described above) to reclaim space.
//
-// The on-disk container metadata design favors simplicity and contiguous
-// access over space consumption and scalability to a very large number of
-// blocks. To be more specific, the separation of metadata from data allows
-// for high performance sustained reads at block manager open time at a
-// manageability cost: a container is not a single file, and needs multiple
-// open fds to be of use. Moreover, the log-structured nature of the
-// metadata is simple and performant at open time.
-//
-// Likewise, the default container placement policy favors simplicity over
+// The default container placement policy favors simplicity over
// performance. In the future, locality hints will ensure that blocks
// pertaining to similar data are colocated, improving scan performance.
//
@@ -185,14 +175,6 @@ class LogBlockManager : public BlockManager {
static const char* kContainerMetadataFileSuffix;
static const char* kContainerDataFileSuffix;
- // Note: all objects passed as pointers should remain alive for the lifetime
- // of the block manager.
- LogBlockManager(Env* env,
- DataDirManager* dd_manager,
- FsErrorManager* error_manager,
- FileCache* file_cache,
- BlockManagerOptions opts);
-
~LogBlockManager() override;
Status Open(FsReport* report, std::atomic<int>* containers_processed,
@@ -214,6 +196,15 @@ class LogBlockManager : public BlockManager {
FsErrorManager* error_manager() override { return error_manager_; }
+ protected:
+ // Note: all objects passed as pointers should remain alive for the lifetime
+ // of the block manager.
+ LogBlockManager(Env* env,
+ DataDirManager* dd_manager,
+ FsErrorManager* error_manager,
+ FileCache* file_cache,
+ BlockManagerOptions opts);
+
private:
FRIEND_TEST(LogBlockManagerTest, TestAbortBlock);
FRIEND_TEST(LogBlockManagerTest, TestCloseFinalizedBlock);
@@ -222,6 +213,7 @@ class LogBlockManager : public BlockManager {
FRIEND_TEST(LogBlockManagerTest, TestLIFOContainerSelection);
FRIEND_TEST(LogBlockManagerTest, TestLookupBlockLimit);
FRIEND_TEST(LogBlockManagerTest, TestMetadataTruncation);
+ FRIEND_TEST(LogBlockManagerTest, TestMisalignedBlocksFuzz);
FRIEND_TEST(LogBlockManagerTest, TestParseKernelRelease);
FRIEND_TEST(LogBlockManagerTest, TestBumpBlockIds);
FRIEND_TEST(LogBlockManagerTest, TestReuseBlockIds);
@@ -230,6 +222,7 @@ class LogBlockManager : public BlockManager {
friend class internal::LogBlockContainer;
friend class internal::LogBlockDeletionTransaction;
friend class internal::LogWritableBlock;
+ friend class LogBlockManagerTest;
// Type for the actual block map used to store all live blocks.
// We use sparse_hash_map<> here to reduce memory overhead.
@@ -314,7 +307,7 @@ class LogBlockManager : public BlockManager {
bool AddLogBlock(LogBlockRefPtr lb);
// Removes the given set of LogBlocks from in-memory data structures, and
- // appends the block deletion metadata to record the on-disk deletion.
+ // adds the block deletion metadata to record the on-disk deletion.
// The 'log_blocks' out parameter will be set with the LogBlocks that were
// successfully removed. The 'deleted' out parameter will be set with the
// blocks were already deleted, e.g encountered 'NotFound' error during removal.
@@ -351,7 +344,7 @@ class LogBlockManager : public BlockManager {
std::string,
std::vector<BlockRecordPB>> low_live_block_containers);
- // Rewrites a container metadata file, appending all entries in 'records'.
+ // Rewrites a container metadata file, adding all entries in 'records'.
// The new metadata file is created as a temporary file and renamed over the
// existing file after it is fully written.
//
@@ -485,6 +478,28 @@ class LogBlockManager : public BlockManager {
DISALLOW_COPY_AND_ASSIGN(LogBlockManager);
};
+// The metadata part of a container is written to a native metadata file.
+// The metadata file itself is not compacted, as it is expected to remain quite
+// small even after many create/delete cycles.
+//
+// The on-disk container metadata design favors simplicity and contiguous
+// access over space consumption and scalability to a very large number of
+// blocks. To be more specific, the separation of metadata from data allows
+// for high performance sustained reads at block manager open time at a
+// manageability cost: a container is not a single file, and needs multiple
+// open fds to be of use. Moreover, the log-structured nature of the
+// metadata is simple and performant at open time.
+class LogBlockManagerNativeMeta : public LogBlockManager {
+ public:
+ LogBlockManagerNativeMeta(Env* env,
+ DataDirManager* dd_manager,
+ FsErrorManager* error_manager,
+ FileCache* file_cache,
+ BlockManagerOptions opts)
+ : LogBlockManager(env, dd_manager, error_manager, file_cache, std::move(opts)) {
+ }
+};
+
} // namespace fs
} // namespace kudu