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 2021/09/14 07:11:08 UTC

[kudu] branch master updated: KUDU-3318 [fs] Add size limit for log block container metadata

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 49abc3a  KUDU-3318 [fs] Add size limit for log block container metadata
49abc3a is described below

commit 49abc3a6788bc29c8c023cafff0f0955142a2409
Author: Yingchun Lai <ac...@gmail.com>
AuthorDate: Fri Sep 10 01:27:37 2021 +0800

    KUDU-3318 [fs] Add size limit for log block container metadata
    
    In some cases, log block container's data file is very small
    after punching hole, while metadata file is relatively large
    because it contains many CREATE-DELETE pairs. LBM reclaims
    both .data and corresponding .metadata container files when
    the .data container file becomes full (i.e. reaches its size
    or block count threshold).
    So, the .metadata container file might grow without any limit
    before the .data container becomes full.
    This patch adds a size limit for log block container's metadata,
    taking it into account while determining the container's 'full'
    condition. That gives the LBM an opportunity to reclaim the disk
    space once the container becomes full.
    
    Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
    Reviewed-on: http://gerrit.cloudera.org:8080/17837
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/fs/log_block_manager-test.cc | 39 ++++++++++++++++++++++++++++++++++-
 src/kudu/fs/log_block_manager.cc      | 11 +++++++++-
 src/kudu/util/pb_util.cc              |  5 +++++
 src/kudu/util/pb_util.h               |  5 ++++-
 4 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/kudu/fs/log_block_manager-test.cc b/src/kudu/fs/log_block_manager-test.cc
index 28fee93..6af5992 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -87,6 +87,7 @@ DECLARE_string(block_manager_preflush_control);
 DECLARE_string(env_inject_eio_globs);
 DECLARE_uint64(log_container_preallocate_bytes);
 DECLARE_uint64(log_container_max_size);
+DECLARE_uint64(log_container_metadata_max_size);
 DEFINE_int32(startup_benchmark_block_count_for_testing, 1000000,
              "Block count to do startup benchmark.");
 DEFINE_int32(startup_benchmark_data_dir_count_for_testing, 8,
@@ -1128,7 +1129,7 @@ TEST_F(LogBlockManagerTest, TestLookupBlockLimit) {
   }
 }
 
-TEST_F(LogBlockManagerTest, TestContainerBlockLimiting) {
+TEST_F(LogBlockManagerTest, TestContainerBlockLimitingByBlockNum) {
   const int kNumBlocks = 1000;
 
   // Creates 'kNumBlocks' blocks with minimal data.
@@ -1162,6 +1163,42 @@ TEST_F(LogBlockManagerTest, TestContainerBlockLimiting) {
   NO_FATALS(AssertNumContainers(4));
 }
 
+TEST_F(LogBlockManagerTest, TestContainerBlockLimitingByMetadataSize) {
+  const int kNumBlocks = 1000;
+
+  // Creates 'kNumBlocks' blocks with minimal data.
+  auto create_some_blocks = [&]() {
+    for (int i = 0; i < kNumBlocks; i++) {
+      unique_ptr<WritableBlock> block;
+      RETURN_NOT_OK(bm_->CreateBlock(test_block_opts_, &block));
+      RETURN_NOT_OK(block->Append("aaaa"));
+      RETURN_NOT_OK(block->Close());
+    }
+    return Status::OK();
+  };
+
+  // All of these blocks should fit into one container.
+  ASSERT_OK(create_some_blocks());
+  NO_FATALS(AssertNumContainers(1));
+
+  // With a limit imposed, the existing container is immediately full, and we
+  // need a few more to satisfy another metadata file size.
+  // Each CREATE type entry in metadata protobuf file is 39 bytes, so 400 of
+  // such entries added by 'create_some_blocks' will make the container full.
+  FLAGS_log_container_metadata_max_size = 400 * 39;
+  ASSERT_OK(ReopenBlockManager());
+  ASSERT_OK(create_some_blocks());
+  NO_FATALS(AssertNumContainers(4));
+
+  // Now remove the limit and create more blocks. They should go into existing
+  // containers, which are now no longer full.
+  FLAGS_log_container_metadata_max_size = 0;
+  ASSERT_OK(ReopenBlockManager());
+
+  ASSERT_OK(create_some_blocks());
+  NO_FATALS(AssertNumContainers(4));
+}
+
 TEST_F(LogBlockManagerTest, TestMisalignedBlocksFuzz) {
   FLAGS_log_container_preallocate_bytes = 0;
   const int kNumBlocks = 100;
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index eb47b08..993ef80 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -80,6 +80,13 @@ DEFINE_uint64(log_container_max_size, 10LU * 1024 * 1024 * 1024,
               "Maximum size (soft) of a log container");
 TAG_FLAG(log_container_max_size, advanced);
 
+DEFINE_uint64(log_container_metadata_max_size, 0,
+              "Maximum size (soft) of a log container's metadata. Use 0 for "
+              "no limit.");
+TAG_FLAG(log_container_metadata_max_size, advanced);
+TAG_FLAG(log_container_metadata_max_size, experimental);
+TAG_FLAG(log_container_metadata_max_size, runtime);
+
 DEFINE_int64(log_container_max_blocks, -1,
              "Maximum number of blocks (soft) of a log container. Use 0 for "
              "no limit. Use -1 for no limit except in the case of a kernel "
@@ -534,7 +541,9 @@ class LogBlockContainer: public RefCountedThreadSafe<LogBlockContainer> {
   int32_t blocks_being_written() const { return blocks_being_written_.Load(); }
   bool full() const {
     return next_block_offset() >= FLAGS_log_container_max_size ||
-        (max_num_blocks_ && (total_blocks() >= max_num_blocks_));
+        (max_num_blocks_ && (total_blocks() >= max_num_blocks_)) ||
+        (FLAGS_log_container_metadata_max_size > 0 &&
+         (metadata_file_->Offset() >= FLAGS_log_container_metadata_max_size));
   }
   bool dead() const { return dead_.Load(); }
   const LogBlockManagerMetrics* metrics() const { return metrics_; }
diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc
index 6e1f1a3..cfdff3d 100644
--- a/src/kudu/util/pb_util.cc
+++ b/src/kudu/util/pb_util.cc
@@ -772,6 +772,11 @@ Status WritablePBContainerFile::Sync() {
   return Status::OK();
 }
 
+uint64_t WritablePBContainerFile::Offset() const {
+  std::lock_guard<Mutex> l(offset_lock_);
+  return offset_;
+}
+
 Status WritablePBContainerFile::Close() {
   if (state_ != FileState::CLOSED) {
     state_ = FileState::CLOSED;
diff --git a/src/kudu/util/pb_util.h b/src/kudu/util/pb_util.h
index 87bf4e2..0ad4cba 100644
--- a/src/kudu/util/pb_util.h
+++ b/src/kudu/util/pb_util.h
@@ -315,6 +315,9 @@ class WritablePBContainerFile {
   // only needed in the former case.
   Status Sync();
 
+  // Get current offset of underlying file.
+  uint64_t Offset() const;
+
   // Closes the container.
   //
   // Not thread-safe.
@@ -350,7 +353,7 @@ class WritablePBContainerFile {
   FileState state_;
 
   // Protects offset_.
-  Mutex offset_lock_;
+  mutable Mutex offset_lock_;
 
   // Current write offset into the file.
   uint64_t offset_;