This is an automated email from the ASF dual-hosted git repository.

zhangyifan 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 2af34c154 [fs] Simplify the data directory inheritance hierarchy
2af34c154 is described below

commit 2af34c15422921172e182fb8532496d34301a350
Author: Yingchun Lai <laiyingc...@apache.org>
AuthorDate: Tue Dec 26 01:07:49 2023 +0800

    [fs] Simplify the data directory inheritance hierarchy
    
    This patch doesn't introduce functional changes, but
    only simplify the inheritance hierarchy of class Dir
    to reduce the complex when extend the Dir class, for
    example, when adding a sub-class of Dir to contain a
    RocksDB instance, see http://gerrit.cloudera.org:8080/18569.
    
    Because there are not sufficient reasons, so the
    DataDir sub-class is removed and the implementation
    functions are moved to the super class Dir.
    
    Change-Id: I2d7059e32c7fd8444256ea031bebd153b318a5a3
    Reviewed-on: http://gerrit.cloudera.org:8080/20833
    Tested-by: Yingchun Lai <laiyingc...@apache.org>
    Reviewed-by: Ádám Bakai <aba...@cloudera.com>
    Reviewed-by: Yifan Zhang <chinazhangyi...@163.com>
---
 src/kudu/fs/data_dirs.cc   | 25 ++++---------------------
 src/kudu/fs/data_dirs.h    | 14 --------------
 src/kudu/fs/dir_manager.cc | 12 ++++++++++++
 src/kudu/fs/dir_manager.h  |  4 ++--
 src/kudu/fs/fs_manager.h   |  2 +-
 5 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc
index c8b4c8960..20a73ddf0 100644
--- a/src/kudu/fs/data_dirs.cc
+++ b/src/kudu/fs/data_dirs.cc
@@ -51,7 +51,7 @@
 #include "kudu/util/random.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_util_prod.h"
-#include "kudu/util/threadpool.h"
+#include "kudu/util/threadpool.h" // IWYU pragma: keep
 
 DEFINE_int32(fs_target_data_dirs_per_tablet, 3,
              "Indicates the target number of data dirs to spread each "
@@ -193,34 +193,17 @@ Status DataDirGroup::CopyToPB(const UuidByUuidIndexMap& 
uuid_by_uuid_idx,
 }
 
 ////////////////////////////////////////////////////////////
-// DataDir
+// DataDirManager
 ////////////////////////////////////////////////////////////
 
-DataDir::DataDir(Env* env, DirMetrics* metrics, FsType fs_type, std::string 
dir,
-                 std::unique_ptr<DirInstanceMetadataFile> metadata_file,
-                 std::unique_ptr<ThreadPool> pool)
-    : Dir(env, metrics, fs_type, std::move(dir), std::move(metadata_file), 
std::move(pool)) {}
-
 std::unique_ptr<Dir> DataDirManager::CreateNewDir(
     Env* env, DirMetrics* metrics, FsType fs_type,
     std::string dir, std::unique_ptr<DirInstanceMetadataFile> metadata_file,
     std::unique_ptr<ThreadPool> pool) {
-  return unique_ptr<Dir>(new DataDir(env, metrics, fs_type, std::move(dir),
-                                     std::move(metadata_file), 
std::move(pool)));
-}
-
-int DataDir::available_space_cache_secs() const {
-  return FLAGS_fs_data_dirs_available_space_cache_seconds;
-}
-
-int DataDir::reserved_bytes() const {
-  return FLAGS_fs_data_dirs_reserved_bytes;
+  return std::make_unique<Dir>(env, metrics, fs_type, std::move(dir),
+                               std::move(metadata_file), std::move(pool));
 }
 
-////////////////////////////////////////////////////////////
-// DataDirManager
-////////////////////////////////////////////////////////////
-
 DataDirManagerOptions::DataDirManagerOptions()
     : DirManagerOptions(FLAGS_block_manager, fs::kDefaultTenantID) {}
 
diff --git a/src/kudu/fs/data_dirs.h b/src/kudu/fs/data_dirs.h
index d6f22d66b..b1ff3730e 100644
--- a/src/kudu/fs/data_dirs.h
+++ b/src/kudu/fs/data_dirs.h
@@ -86,20 +86,6 @@ class DataDirGroup {
 
 }  // namespace internal
 
-// Instantiation of a directory that uses the appropriate gflags.
-class DataDir : public Dir {
- public:
-  DataDir(Env* env,
-          DirMetrics* metrics,
-          FsType fs_type,
-          std::string dir,
-          std::unique_ptr<DirInstanceMetadataFile> metadata_file,
-          std::unique_ptr<ThreadPool> pool);
-
-  int available_space_cache_secs() const override;
-  int reserved_bytes() const override;
-};
-
 struct DataDirManagerOptions : public DirManagerOptions {
   DataDirManagerOptions();
 };
diff --git a/src/kudu/fs/dir_manager.cc b/src/kudu/fs/dir_manager.cc
index 94d7c945b..fe8d42a4d 100644
--- a/src/kudu/fs/dir_manager.cc
+++ b/src/kudu/fs/dir_manager.cc
@@ -30,6 +30,7 @@
 #include <utility>
 #include <vector>
 
+#include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 
 #include "kudu/fs/dir_util.h"
@@ -56,6 +57,9 @@ using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
+DECLARE_int32(fs_data_dirs_available_space_cache_seconds);
+DECLARE_int64(fs_data_dirs_reserved_bytes);
+
 namespace kudu {
 
 namespace {
@@ -159,6 +163,14 @@ Status Dir::RefreshAvailableSpace(RefreshMode mode) {
   return Status::OK();
 }
 
+int Dir::available_space_cache_secs() {
+  return FLAGS_fs_data_dirs_available_space_cache_seconds;
+}
+
+int Dir::reserved_bytes() {
+  return FLAGS_fs_data_dirs_reserved_bytes;
+}
+
 DirManagerOptions::DirManagerOptions(string dir_type,
                                      string tid)
     : dir_type(std::move(dir_type)),
diff --git a/src/kudu/fs/dir_manager.h b/src/kudu/fs/dir_manager.h
index 4502740b5..a6f14ff9d 100644
--- a/src/kudu/fs/dir_manager.h
+++ b/src/kudu/fs/dir_manager.h
@@ -153,11 +153,11 @@ class Dir {
 
   // The amount of time to cache the amount of available space in this
   // directory.
-  virtual int available_space_cache_secs() const = 0;
+  static int available_space_cache_secs();
 
   // The number of bytes to reserve in each directory for non-Kudu usage. A
   // value of -1 means 1% of the disk space in a directory will be reserved.
-  virtual int reserved_bytes() const = 0;
+  static int reserved_bytes();
 
  private:
   Env* env_;
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index 2b0135d03..7abdb9fac 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -237,7 +237,7 @@ class FsManager {
   // Registers an error-handling callback with the FsErrorManager.
   //
   // If a disk failure is detected, this callback will be invoked with the
-  // relevant DataDir's UUID as its input parameter.
+  // relevant Dir's UUID as its input parameter.
   void SetErrorNotificationCb(fs::ErrorHandlerType e, fs::ErrorNotificationCb 
cb);
 
   // Unregisters the error-handling callback with the FsErrorManager.

Reply via email to