kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: block_manager: disk space checking everywhere
Date Fri, 04 Nov 2016 19:33:42 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 1f35e1eb5 -> a833a1573


block_manager: disk space checking everywhere

This patch extends the disk space checking feature to the file block manager
and to the log block manager without preallocation. Neither of these are
hotly requested, but it's easy to do and it removes all conditions from the
help text, so I think it's a net benefit.

Change-Id: Ibbce30d7fa981255949ade23373c13939b1e3d43
Reviewed-on: http://gerrit.cloudera.org:8080/4832
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mpercy@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a833a157
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a833a157
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a833a157

Branch: refs/heads/master
Commit: a833a157314dceed9791727576db107c6449dd44
Parents: 1f35e1e
Author: Adar Dembo <adar@cloudera.com>
Authored: Mon Oct 24 19:09:40 2016 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Fri Nov 4 19:26:26 2016 +0000

----------------------------------------------------------------------
 src/kudu/fs/block_manager-test.cc               |  8 +--
 src/kudu/fs/data_dirs.cc                        |  3 +-
 src/kudu/fs/file_block_manager.cc               | 60 ++++++++++----------
 src/kudu/fs/log_block_manager.cc                | 25 +++++---
 .../integration-tests/disk_reservation-itest.cc | 19 -------
 5 files changed, 52 insertions(+), 63 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/a833a157/src/kudu/fs/block_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index e780caf..cdf3ebf 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -980,9 +980,7 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
                                      false));
 }
 
-TEST_F(LogBlockManagerTest, TestDiskSpaceCheck) {
-  RETURN_NOT_LOG_BLOCK_MANAGER();
-
+TYPED_TEST(BlockManagerTest, TestDiskSpaceCheck) {
   // Reopen the block manager with metrics enabled.
   MetricRegistry registry;
   scoped_refptr<MetricEntity> entity = METRIC_ENTITY_server.Instantiate(&registry,
"test");
@@ -1009,7 +1007,7 @@ TEST_F(LogBlockManagerTest, TestDiskSpaceCheck) {
     for (int attempt = 0; attempt < 3; attempt++) {
       gscoped_ptr<WritableBlock> writer;
       LOG(INFO) << "Attempt #" << ++i;
-      Status s = bm_->CreateBlock(&writer);
+      Status s = this->bm_->CreateBlock(&writer);
       if (FLAGS_disk_reserved_bytes_free_for_testing < FLAGS_fs_data_dirs_reserved_bytes)
{
         if (data_dir_observed_full) {
           // The dir was previously observed as full, so CreateBlock() checked
@@ -1018,6 +1016,7 @@ TEST_F(LogBlockManagerTest, TestDiskSpaceCheck) {
           ASSERT_STR_CONTAINS(s.ToString(), "All data directories are full");
         } else {
           ASSERT_OK(s);
+          ASSERT_OK(writer->Append("test data"));
           ASSERT_OK(writer->Close());
 
           // The dir was not previously full so CreateBlock() did not check for
@@ -1031,6 +1030,7 @@ TEST_F(LogBlockManagerTest, TestDiskSpaceCheck) {
         // CreateBlock() succeeded regardless of the previously fullness state,
         // and the new state is definitely not full.
         ASSERT_OK(s);
+        ASSERT_OK(writer->Append("test data"));
         ASSERT_OK(writer->Close());
         data_dir_observed_full = false;
         ASSERT_EQ(0, down_cast<AtomicGauge<uint64_t>*>(

http://git-wip-us.apache.org/repos/asf/kudu/blob/a833a157/src/kudu/fs/data_dirs.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc
index 6a2b78c..3cfcf65 100644
--- a/src/kudu/fs/data_dirs.cc
+++ b/src/kudu/fs/data_dirs.cc
@@ -53,8 +53,7 @@
 #include "kudu/util/threadpool.h"
 
 DEFINE_int64(fs_data_dirs_reserved_bytes, 0,
-             "Number of bytes to reserve on each data directory filesystem for non-Kudu usage.
"
-             "Only works when --log_container_preallocate_bytes is non-zero.");
+             "Number of bytes to reserve on each data directory filesystem for non-Kudu usage.");
 TAG_FLAG(fs_data_dirs_reserved_bytes, runtime);
 TAG_FLAG(fs_data_dirs_reserved_bytes, evolving);
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/a833a157/src/kudu/fs/file_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/file_block_manager.cc b/src/kudu/fs/file_block_manager.cc
index 5e85670..5c60329 100644
--- a/src/kudu/fs/file_block_manager.cc
+++ b/src/kudu/fs/file_block_manager.cc
@@ -54,10 +54,10 @@ namespace internal {
 //
 // A block ID uniquely locates a block. Every ID is a uint64_t, broken down
 // into multiple logical components:
-// 1. Bytes 0 (MSB) and 1 identify the block's root path by path set index. See
+// 1. Bytes 0 (MSB) and 1 identify the block's data dir by path set index. See
 //    fs.proto for more details on path sets.
-// 2. Bytes 2-7 (LSB) uniquely identify the block within the root path. As more
-//    and more blocks are created in a root path, the likelihood of a collision
+// 2. Bytes 2-7 (LSB) uniquely identify the block within the data dir. As more
+//    and more blocks are created in a data dir, the likelihood of a collision
 //    becomes greater. In the event of a collision, the block manager will
 //    retry(see CreateBlock()).
 //
@@ -71,16 +71,16 @@ class FileBlockLocation {
   }
 
   // Construct a location from its constituent parts.
-  static FileBlockLocation FromParts(const string& root_path,
-                                     uint16_t root_path_idx,
+  static FileBlockLocation FromParts(DataDir* data_dir,
+                                     uint16_t data_dir_idx,
                                      const BlockId& block_id);
 
   // Construct a location from a full block ID.
-  static FileBlockLocation FromBlockId(const string& root_path,
+  static FileBlockLocation FromBlockId(DataDir* data_dir,
                                        const BlockId& block_id);
 
-  // Get the root path index of a given block ID.
-  static uint16_t GetRootPathIdx(const BlockId& block_id) {
+  // Get the data dir index of a given block ID.
+  static uint16_t GetDataDirIdx(const BlockId& block_id) {
     return block_id.id() >> 48;
   }
 
@@ -101,12 +101,12 @@ class FileBlockLocation {
   void GetAllParentDirs(vector<string>* parent_dirs) const;
 
   // Simple accessors.
-  const string& root_path() const { return root_path_; }
+  DataDir* data_dir() const { return data_dir_; }
   const BlockId& block_id() const { return block_id_; }
 
  private:
-  FileBlockLocation(string root_path, BlockId block_id)
-      : root_path_(std::move(root_path)), block_id_(std::move(block_id)) {}
+  FileBlockLocation(DataDir* data_dir, BlockId block_id)
+      : data_dir_(data_dir), block_id_(block_id) {}
 
   // These per-byte accessors yield subdirectories in which blocks are grouped.
   string byte2() const {
@@ -122,27 +122,27 @@ class FileBlockLocation {
                         (block_id_.id() & 0x00000000FF000000ULL) >> 24);
   }
 
-  string root_path_;
+  DataDir* data_dir_;
   BlockId block_id_;
 };
 
-FileBlockLocation FileBlockLocation::FromParts(const string& root_path,
-                                               uint16_t root_path_idx,
+FileBlockLocation FileBlockLocation::FromParts(DataDir* data_dir,
+                                               uint16_t data_dir_idx,
                                                const BlockId& block_id) {
-  // The combined ID consists of 'root_path_idx' (top 2 bytes) and 'block_id'
+  // The combined ID consists of 'data_dir_idx' (top 2 bytes) and 'block_id'
   // (bottom 6 bytes). The top 2 bytes of 'block_id' are dropped.
-  uint64_t combined_id = static_cast<uint64_t>(root_path_idx) << 48;
+  uint64_t combined_id = static_cast<uint64_t>(data_dir_idx) << 48;
   combined_id |= block_id.id() & ((1ULL << 48) - 1);
-  return FileBlockLocation(root_path, BlockId(combined_id));
+  return FileBlockLocation(data_dir, BlockId(combined_id));
 }
 
-FileBlockLocation FileBlockLocation::FromBlockId(const string& root_path,
+FileBlockLocation FileBlockLocation::FromBlockId(DataDir* data_dir,
                                                  const BlockId& block_id) {
-  return FileBlockLocation(root_path, block_id);
+  return FileBlockLocation(data_dir, block_id);
 }
 
 string FileBlockLocation::GetFullPath() const {
-  string p = root_path_;
+  string p = data_dir_->dir();
   p = JoinPathSegments(p, byte2());
   p = JoinPathSegments(p, byte3());
   p = JoinPathSegments(p, byte4());
@@ -152,10 +152,10 @@ string FileBlockLocation::GetFullPath() const {
 
 Status FileBlockLocation::CreateBlockDir(Env* env,
                                          vector<string>* created_dirs) {
-  DCHECK(env->FileExists(root_path_));
+  DCHECK(env->FileExists(data_dir_->dir()));
 
   bool path0_created;
-  string path0 = JoinPathSegments(root_path_, byte2());
+  string path0 = JoinPathSegments(data_dir_->dir(), byte2());
   RETURN_NOT_OK(env_util::CreateDirIfMissing(env, path0, &path0_created));
 
   bool path1_created;
@@ -173,13 +173,13 @@ Status FileBlockLocation::CreateBlockDir(Env* env,
     created_dirs->push_back(path0);
   }
   if (path0_created) {
-    created_dirs->push_back(root_path_);
+    created_dirs->push_back(data_dir_->dir());
   }
   return Status::OK();
 }
 
 void FileBlockLocation::GetAllParentDirs(vector<string>* parent_dirs) const {
-  string path0 = JoinPathSegments(root_path_, byte2());
+  string path0 = JoinPathSegments(data_dir_->dir(), byte2());
   string path1 = JoinPathSegments(path0, byte3());
   string path2 = JoinPathSegments(path1, byte4());
 
@@ -188,7 +188,7 @@ void FileBlockLocation::GetAllParentDirs(vector<string>* parent_dirs)
const {
   parent_dirs->push_back(path2);
   parent_dirs->push_back(path1);
   parent_dirs->push_back(path0);
-  parent_dirs->push_back(root_path_);
+  parent_dirs->push_back(data_dir_->dir());
 }
 
 ////////////////////////////////////////////////////////////
@@ -296,6 +296,8 @@ Status FileWritableBlock::Append(const Slice& data) {
       << "Invalid state: " << state_;
 
   RETURN_NOT_OK(writer_->Append(data));
+  RETURN_NOT_OK(location_.data_dir()->RefreshIsFull(
+      DataDir::RefreshMode::ALWAYS));
   state_ = DIRTY;
   bytes_appended_ += data.size();
   return Status::OK();
@@ -401,7 +403,7 @@ FileReadableBlock::FileReadableBlock(const FileBlockManager* block_manager,
                                      BlockId block_id,
                                      shared_ptr<RandomAccessFile> reader)
     : block_manager_(block_manager),
-      block_id_(std::move(block_id)),
+      block_id_(block_id),
       reader_(std::move(reader)),
       closed_(false) {
   if (block_manager_->metrics_) {
@@ -489,10 +491,10 @@ Status FileBlockManager::SyncMetadata(const internal::FileBlockLocation&
locatio
 bool FileBlockManager::FindBlockPath(const BlockId& block_id,
                                      string* path) const {
   DataDir* dir = dd_manager_.FindDataDirByUuidIndex(
-      internal::FileBlockLocation::GetRootPathIdx(block_id));
+      internal::FileBlockLocation::GetDataDirIdx(block_id));
   if (dir) {
     *path = internal::FileBlockLocation::FromBlockId(
-        dir->dir(), block_id).GetFullPath();
+        dir, block_id).GetFullPath();
   }
   return dir != nullptr;
 }
@@ -565,7 +567,7 @@ Status FileBlockManager::CreateBlock(const CreateBlockOptions& opts,
       id.SetId(next_block_id_.Increment());
     } while (id.IsNull());
 
-    location = internal::FileBlockLocation::FromParts(dir->dir(), uuid_idx, id);
+    location = internal::FileBlockLocation::FromParts(dir, uuid_idx, id);
     path = location.GetFullPath();
     RETURN_NOT_OK_PREPEND(location.CreateBlockDir(env_, &created_dirs), path);
     WritableFileOptions wr_opts;

http://git-wip-us.apache.org/repos/asf/kudu/blob/a833a157/src/kudu/fs/log_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index a480a94..0ab26d5 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -580,9 +580,16 @@ Status LogBlockContainer::DeleteBlock(int64_t offset, int64_t length)
{
 
 Status LogBlockContainer::WriteData(int64_t offset, const Slice& data) {
   DCHECK_GE(offset, 0);
+  {
+    std::lock_guard<Mutex> l(data_writer_lock_);
+    return data_file_->Write(offset, data);
+  }
 
-  std::lock_guard<Mutex> l(data_writer_lock_);
-  return data_file_->Write(offset, data);
+  if (PREDICT_FALSE(!FLAGS_log_container_preallocate_bytes)) {
+    // Without preallocation, every call grows the container.
+    RETURN_NOT_OK(data_dir_->RefreshIsFull(DataDir::RefreshMode::ALWAYS));
+  }
+  return Status::OK();
 }
 
 Status LogBlockContainer::ReadData(int64_t offset, size_t length,
@@ -631,6 +638,11 @@ Status LogBlockContainer::SyncMetadata() {
 Status LogBlockContainer::Preallocate(size_t length) {
   RETURN_NOT_OK(data_file_->PreAllocate(total_bytes_written(), length));
   preallocated_offset_ = total_bytes_written() + length;
+
+  // Preallocation succeeded and the container has grown; recheck its data
+  // directory fullness.
+  RETURN_NOT_OK(data_dir_->RefreshIsFull(DataDir::RefreshMode::ALWAYS));
+
   return Status::OK();
 }
 
@@ -707,7 +719,7 @@ class LogBlock : public RefCountedThreadSafe<LogBlock> {
 LogBlock::LogBlock(LogBlockContainer* container, BlockId block_id,
                    int64_t offset, int64_t length)
     : container_(container),
-      block_id_(std::move(block_id)),
+      block_id_(block_id),
       offset_(offset),
       length_(length),
       deleted_(false) {
@@ -808,7 +820,7 @@ class LogWritableBlock : public WritableBlock {
 LogWritableBlock::LogWritableBlock(LogBlockContainer* container,
                                    BlockId block_id, int64_t block_offset)
     : container_(container),
-      block_id_(std::move(block_id)),
+      block_id_(block_id),
       block_offset_(block_offset),
       block_length_(0),
       state_(CLEAN) {
@@ -1180,11 +1192,6 @@ Status LogBlockManager::CreateBlock(const CreateBlockOptions& opts,
   RETURN_NOT_OK(GetOrCreateContainer(&container));
   if (FLAGS_log_container_preallocate_bytes) {
     RETURN_NOT_OK(container->Preallocate(FLAGS_log_container_preallocate_bytes));
-
-    // Preallocation succeeded and the container has grown; recheck its data
-    // directory fullness.
-    RETURN_NOT_OK(container->mutable_data_dir()->RefreshIsFull(
-        DataDir::RefreshMode::ALWAYS));
   }
 
   // Generate a free block ID.

http://git-wip-us.apache.org/repos/asf/kudu/blob/a833a157/src/kudu/integration-tests/disk_reservation-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/disk_reservation-itest.cc b/src/kudu/integration-tests/disk_reservation-itest.cc
index f5a6dd0..a4357b0 100644
--- a/src/kudu/integration-tests/disk_reservation-itest.cc
+++ b/src/kudu/integration-tests/disk_reservation-itest.cc
@@ -26,11 +26,8 @@
 using std::string;
 using strings::Substitute;
 
-DECLARE_string(block_manager);
-
 METRIC_DECLARE_entity(server);
 METRIC_DECLARE_gauge_uint64(data_dirs_full);
-METRIC_DECLARE_counter(log_block_manager_containers);
 
 namespace kudu {
 
@@ -52,11 +49,6 @@ class DiskReservationITest : public ExternalMiniClusterITestBase {
 // use other disks for data blocks until all disks are full, at which time we
 // crash. This functionality is only implemented in the log block manager.
 TEST_F(DiskReservationITest, TestFillMultipleDisks) {
-  if (FLAGS_block_manager != "log") {
-    LOG(INFO) << "This platform does not use the log block manager by default. Skipping
test.";
-    return;
-  }
-
   vector<string> ts_flags;
   // Don't preallocate very many bytes so we run the "full disk" check often.
   ts_flags.push_back("--log_container_preallocate_bytes=100000");
@@ -86,17 +78,6 @@ TEST_F(DiskReservationITest, TestFillMultipleDisks) {
   workload.Setup();
   workload.Start();
 
-  // Wait until we have 2 active containers.
-  while (true) {
-    int64_t num_containers;
-    ASSERT_OK(GetTsCounterValue(cluster_->tablet_server(0), &METRIC_log_block_manager_containers,
-                                &num_containers));
-    if (num_containers >= 2) break;
-    SleepFor(MonoDelta::FromMilliseconds(10));
-  }
-
-  LOG(INFO) << "Two log block containers are active";
-
   // Simulate that /a has 0 bytes free.
   ASSERT_OK(cluster_->SetFlag(cluster_->tablet_server(0),
                               "disk_reserved_override_prefix_1_bytes_free_for_testing", "0"));


Mime
View raw message