kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [1/2] kudu git commit: env: generalize resource limits and add RLIMIT_NPROC support
Date Thu, 08 Mar 2018 02:44:02 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 954d9f71b -> debcb8ea2


env: generalize resource limits and add RLIMIT_NPROC support

A follow-on patch will use this to cap the max number of threads in some
process-wide thread pools (see KUDU-1913).

Change-Id: I567f581a6f8a85ac1f08878b61ac316cc2da36a0
Reviewed-on: http://gerrit.cloudera.org:8080/9521
Reviewed-by: David Ribeiro Alves <davidralves@gmail.com>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: ede0cf0ec3924a067f82b625eb61410e8b734183
Parents: 954d9f7
Author: Adar Dembo <adar@cloudera.com>
Authored: Tue Mar 6 15:45:40 2018 -0800
Committer: Adar Dembo <adar@cloudera.com>
Committed: Thu Mar 8 02:43:31 2018 +0000

----------------------------------------------------------------------
 src/kudu/fs/block_manager.cc | 13 ++++----
 src/kudu/rpc/rpc-test.cc     |  2 +-
 src/kudu/util/env-test.cc    | 27 +++++++++++++----
 src/kudu/util/env.h          | 35 +++++++++++++++++-----
 src/kudu/util/env_posix.cc   | 62 +++++++++++++++++++++++++++++++--------
 5 files changed, 106 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/ede0cf0e/src/kudu/fs/block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager.cc b/src/kudu/fs/block_manager.cc
index e5257c1..d3ce1c0 100644
--- a/src/kudu/fs/block_manager.cc
+++ b/src/kudu/fs/block_manager.cc
@@ -83,19 +83,20 @@ int64_t GetFileCacheCapacityForBlockManager(Env* env) {
   // Maximize this process' open file limit first, if possible.
   static std::once_flag once;
   std::call_once(once, [&]() {
-    env->IncreaseOpenFileLimit();
+    env->IncreaseResourceLimit(Env::ResourceLimitType::OPEN_FILES_PER_PROCESS);
   });
 
+  int64_t rlimit =
+      env->GetResourceLimit(Env::ResourceLimitType::OPEN_FILES_PER_PROCESS);
   // See block_manager_max_open_files.
   if (FLAGS_block_manager_max_open_files == -1) {
-    return (2 * env->GetOpenFileLimit()) / 5;
+    return (2 * rlimit) / 5;
   }
-  int64_t file_limit = env->GetOpenFileLimit();
-  LOG_IF(FATAL, FLAGS_block_manager_max_open_files > file_limit) <<
+  LOG_IF(FATAL, FLAGS_block_manager_max_open_files > rlimit) <<
       Substitute(
           "Configured open file limit (block_manager_max_open_files) $0 "
-          "exceeds process fd limit (ulimit) $1",
-          FLAGS_block_manager_max_open_files, file_limit);
+          "exceeds process open file limit (ulimit) $1",
+          FLAGS_block_manager_max_open_files, rlimit);
   return FLAGS_block_manager_max_open_files;
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/ede0cf0e/src/kudu/rpc/rpc-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc-test.cc b/src/kudu/rpc/rpc-test.cc
index f279bff..f6496cf 100644
--- a/src/kudu/rpc/rpc-test.cc
+++ b/src/kudu/rpc/rpc-test.cc
@@ -348,7 +348,7 @@ TEST_P(TestRpc, TestHighFDs) {
   // This test can only run if ulimit is set high.
   const int kNumFakeFiles = 3500;
   const int kMinUlimit = kNumFakeFiles + 100;
-  if (env_->GetOpenFileLimit() < kMinUlimit) {
+  if (env_->GetResourceLimit(Env::ResourceLimitType::OPEN_FILES_PER_PROCESS) < kMinUlimit)
{
     LOG(INFO) << "Test skipped: must increase ulimit -n to at least " << kMinUlimit;
     return;
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/ede0cf0e/src/kudu/util/env-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index c152b87..83bf67d 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -664,12 +664,27 @@ TEST_F(TestEnv, TestIsDirectory) {
   ASSERT_FALSE(is_dir);
 }
 
-// Regression test for KUDU-1776.
-TEST_F(TestEnv, TestIncreaseOpenFileLimit) {
-  int64_t limit_before = env_->GetOpenFileLimit();
-  env_->IncreaseOpenFileLimit();
-  int64_t limit_after = env_->GetOpenFileLimit();
-  ASSERT_GE(limit_after, limit_before) << "Failed to retain/increase open file limit";
+class ResourceLimitTypeTest : public TestEnv,
+                              public ::testing::WithParamInterface<Env::ResourceLimitType>
{};
+
+INSTANTIATE_TEST_CASE_P(ResourceLimitTypes,
+                        ResourceLimitTypeTest,
+                        ::testing::Values(Env::ResourceLimitType::OPEN_FILES_PER_PROCESS,
+                                          Env::ResourceLimitType::RUNNING_THREADS_PER_EUID));
+
+// Regression test for KUDU-1798.
+TEST_P(ResourceLimitTypeTest, TestIncreaseLimit) {
+  // Increase the resource limit. It should either increase or remain the same.
+  Env::ResourceLimitType t = GetParam();
+  int64_t limit_before = env_->GetResourceLimit(t);
+  env_->IncreaseResourceLimit(t);
+  int64_t limit_after = env_->GetResourceLimit(t);
+  ASSERT_GE(limit_after, limit_before);
+
+  // Try again. It should definitely be the same now.
+  env_->IncreaseResourceLimit(t);
+  int64_t limit_after_again = env_->GetResourceLimit(t);
+  ASSERT_EQ(limit_after, limit_after_again);
 }
 
 static Status TestWalkCb(unordered_set<string>* actual,

http://git-wip-us.apache.org/repos/asf/kudu/blob/ede0cf0e/src/kudu/util/env.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index e3eb82a..4ce3376 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -302,17 +302,33 @@ class Env {
   // All directory entries in 'path' must exist on the filesystem.
   virtual Status Canonicalize(const std::string& path, std::string* result) = 0;
 
-  // Get the total amount of RAM installed on this machine.
+  // Gets the total amount of RAM installed on this machine.
   virtual Status GetTotalRAMBytes(int64_t* ram) = 0;
 
-  // Get the max number of file descriptors that this process can open.
-  virtual int64_t GetOpenFileLimit() = 0;
+  enum class ResourceLimitType {
+    // The maximum number of file descriptors that this process can have open
+    // at any given time.
+    //
+    // Corresponds to RLIMIT_NOFILE on UNIX platforms.
+    OPEN_FILES_PER_PROCESS,
+
+    // The maximum number of threads (or processes) that this process's
+    // effective user ID may have spawned and running at any given time.
+    //
+    // Corresponds to RLIMIT_NPROC on UNIX platforms.
+    RUNNING_THREADS_PER_EUID,
+  };
 
-  // Increase the max number of file descriptors that this process can open as
-  // much as possible. On UNIX platforms, this means increasing the
-  // RLIMIT_NOFILE resource soft limit (the limit actually enforced by the
-  // kernel) to be equal to the hard limit.
-  virtual void IncreaseOpenFileLimit() = 0;
+  // Gets the process' current limit for the given resource type.
+  //
+  // On UNIX platforms, this is equivalent to the resource's soft limit.
+  virtual int64_t GetResourceLimit(ResourceLimitType t) = 0;
+
+  // Increases the resource limit by as much as possible.
+  //
+  // On UNIX platforms, this means increasing the resource's soft limit (the
+  // limit actually enforced by the kernel) to be equal to the hard limit.
+  virtual void IncreaseResourceLimit(ResourceLimitType t) = 0;
 
   // Checks whether the given path resides on an ext2, ext3, or ext4
   // filesystem.
@@ -656,6 +672,9 @@ extern Status WriteStringToFile(Env* env, const Slice& data,
 extern Status ReadFileToString(Env* env, const std::string& fname,
                                faststring* data);
 
+// Overloaded operator for printing Env::ResourceLimitType.
+std::ostream& operator<<(std::ostream& o, Env::ResourceLimitType t);
+
 }  // namespace kudu
 
 #endif  // STORAGE_LEVELDB_INCLUDE_ENV_H_

http://git-wip-us.apache.org/repos/asf/kudu/blob/ede0cf0e/src/kudu/util/env_posix.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index ddd94aa..fa4682b 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -501,6 +501,36 @@ Status DoIsOnXfsFilesystem(const string& path, bool* result) {
   return Status::OK();
 }
 
+const char* ResourceLimitTypeToString(Env::ResourceLimitType t) {
+  switch (t) {
+    case Env::ResourceLimitType::OPEN_FILES_PER_PROCESS:
+      return "open files per process";
+    case Env::ResourceLimitType::RUNNING_THREADS_PER_EUID:
+      return "running threads per effective uid";
+    default: LOG(FATAL) << "Unknown resource limit type";
+  }
+}
+
+int ResourceLimitTypeToUnixRlimit(Env::ResourceLimitType t) {
+  switch (t) {
+    case Env::ResourceLimitType::OPEN_FILES_PER_PROCESS: return RLIMIT_NOFILE;
+    case Env::ResourceLimitType::RUNNING_THREADS_PER_EUID: return RLIMIT_NPROC;
+    default: LOG(FATAL) << "Unknown resource limit type: " << t;
+  }
+}
+
+#ifdef __APPLE__
+const char* ResourceLimitTypeToMacosRlimit(Env::ResourceLimitType t) {
+  switch (t) {
+    case Env::ResourceLimitType::OPEN_FILES_PER_PROCESS:
+      return "kern.maxfilesperproc";
+    case Env::ResourceLimitType::RUNNING_THREADS_PER_EUID:
+      return "kern.maxprocperuid";
+    default: LOG(FATAL) << "Unknown resource limit type: " << t;
+  }
+}
+#endif
+
 class PosixSequentialFile: public SequentialFile {
  private:
   std::string filename_;
@@ -1545,46 +1575,50 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual int64_t GetOpenFileLimit() OVERRIDE {
+  virtual int64_t GetResourceLimit(ResourceLimitType t) OVERRIDE {
     // There's no reason for this to ever fail.
     struct rlimit l;
-    PCHECK(getrlimit(RLIMIT_NOFILE, &l) == 0);
+    PCHECK(getrlimit(ResourceLimitTypeToUnixRlimit(t), &l) == 0);
     return l.rlim_cur;
   }
 
-  virtual void IncreaseOpenFileLimit() OVERRIDE {
+  virtual void IncreaseResourceLimit(ResourceLimitType t) OVERRIDE {
     // There's no reason for this to ever fail; any process should have
     // sufficient privilege to increase its soft limit up to the hard limit.
     //
     // This change is logged because it is process-wide.
+
+    int rlimit_type = ResourceLimitTypeToUnixRlimit(t);
     struct rlimit l;
-    PCHECK(getrlimit(RLIMIT_NOFILE, &l) == 0);
+    PCHECK(getrlimit(rlimit_type, &l) == 0);
 #if defined(__APPLE__)
     // OS X 10.11 can return RLIM_INFINITY from getrlimit, but allows rlim_cur and
     // rlim_max to be raised only as high as the value of the maxfilesperproc
-    // kernel variable. Emperically, this value is 10240 across all tested macOS
+    // kernel variable. Empirically, this value is 10240 across all tested macOS
     // versions. Testing on OS X 10.10 and macOS 10.12 revealed that getrlimit
     // returns the true limits (not RLIM_INFINITY), rlim_max can *not* be raised
     // (when running as non-root), and rlim_cur can only be raised as high as
     // rlim_max (this is consistent with Linux).
-    // TLDR; OS X 10.11 is wack.
+    // TLDR; OS X 10.11 is whack.
     if (l.rlim_max == RLIM_INFINITY) {
       uint32_t limit;
       size_t len = sizeof(limit);
-      PCHECK(sysctlbyname("kern.maxfilesperproc", &limit, &len, nullptr, 0) == 0);
+      PCHECK(sysctlbyname(ResourceLimitTypeToMacosRlimit(t), &limit, &len,
+                          nullptr, 0) == 0);
       // Make sure no uninitialized bits are present in the result.
       DCHECK_EQ(sizeof(limit), len);
       l.rlim_max = limit;
     }
 #endif
+    const char* rlimit_str = ResourceLimitTypeToString(t);
     if (l.rlim_cur < l.rlim_max) {
-      LOG(INFO) << Substitute("Raising process file limit from $0 to $1",
-                              l.rlim_cur, l.rlim_max);
+      LOG(INFO) << Substitute("Raising this process' $0 limit from $1 to $2",
+                              rlimit_str, l.rlim_cur, l.rlim_max);
       l.rlim_cur = l.rlim_max;
-      PCHECK(setrlimit(RLIMIT_NOFILE, &l) == 0);
+      PCHECK(setrlimit(rlimit_type, &l) == 0);
     } else {
-      LOG(INFO) << Substitute("Not raising process file limit of $0; it is "
-          "already as high as it can go", l.rlim_cur);
+      LOG(INFO) << Substitute("Not raising this process' $0 limit of $1; it "
+          "is already as high as it can go", rlimit_str, l.rlim_cur);
     }
   }
 
@@ -1751,4 +1785,8 @@ Env* Env::Default() {
   return default_env;
 }
 
+std::ostream& operator<<(std::ostream& o, Env::ResourceLimitType t) {
+  return o << ResourceLimitTypeToString(t);
+}
+
 }  // namespace kudu


Mime
View raw message