kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mpe...@apache.org
Subject [1/2] kudu git commit: env: Add support for getting FS capacity
Date Tue, 07 Mar 2017 00:56:37 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 45548c90c -> 578bf84bb


env: Add support for getting FS capacity

We need a way to get FS capacity in a follow-up patch. This change adds
that capability and changes the Env API to allow for fetching both
capacity and free bytes with a single call. This also allows us to fetch
both values with a single syscall. This API is inspired by
boost::filesystem::space().

This patch also removes the difference in the "free" space returned when
this API is invoked as a superuser vs. a non-privileged user. Now, only
the space available to non-privileged processes is returned. This is
also consistent with the boost API and makes the API more predictable.

Change-Id: Id43275876d3352f5cf943e24ed4513b9f2c131aa
Reviewed-on: http://gerrit.cloudera.org:8080/6255
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>
Reviewed-by: David Ribeiro Alves <dralves@apache.org>


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

Branch: refs/heads/master
Commit: 573e232de129bc9b6afff524bfae7f4e5bfc9e7b
Parents: 45548c9
Author: Mike Percy <mpercy@apache.org>
Authored: Thu Mar 2 12:39:52 2017 -0800
Committer: Mike Percy <mpercy@apache.org>
Committed: Tue Mar 7 00:56:13 2017 +0000

----------------------------------------------------------------------
 src/kudu/util/env-test.cc  | 41 +++++++++++++++++++++++++++--------------
 src/kudu/util/env.h        | 14 ++++++++++----
 src/kudu/util/env_posix.cc | 11 ++++-------
 src/kudu/util/env_util.cc  | 15 ++++++++-------
 4 files changed, 49 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/573e232d/src/kudu/util/env-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 01e0db6..a8e5f70 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -25,6 +25,7 @@
 #include <gtest/gtest.h>
 
 #include "kudu/gutil/bind.h"
+#include "kudu/gutil/strings/human_readable.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/util/env.h"
@@ -802,28 +803,40 @@ TEST_F(TestEnv, TestTempRWFile) {
   ASSERT_OK(env_->DeleteFile(path));
 }
 
-TEST_F(TestEnv, TestGetBytesFree) {
+// Test that when we write data to disk we see SpaceInfo.free_bytes go down.
+TEST_F(TestEnv, TestGetSpaceInfoFreeBytes) {
   const string kDataDir = GetTestPath("parent");
   const string kTestFilePath = JoinPathSegments(kDataDir, "testfile");
   const int kFileSizeBytes = 256;
-  int64_t orig_bytes_free;
-  int64_t cur_bytes_free;
   ASSERT_OK(env_->CreateDir(kDataDir));
 
-  // Loop several times in case there are concurrent tests running that are
-  // modifying the filesystem.
-  const int kIters = 100;
-  for (int i = 0; i < kIters; i++) {
+  // Loop in case there are concurrent tests running that are modifying the
+  // filesystem.
+  NO_FATALS(AssertEventually([&] {
     if (env_->FileExists(kTestFilePath)) {
-      ASSERT_OK(env_->DeleteFile(kTestFilePath));
+      ASSERT_OK(env_->DeleteFile(kTestFilePath)); // Clean up the previous iteration.
     }
-    ASSERT_OK(env_->GetBytesFree(kDataDir, &orig_bytes_free));
+    SpaceInfo before_space_info;
+    ASSERT_OK(env_->GetSpaceInfo(kDataDir, &before_space_info));
+
     NO_FATALS(WriteTestFile(env_, kTestFilePath, kFileSizeBytes));
-    ASSERT_OK(env_->GetBytesFree(kDataDir, &cur_bytes_free));
-    if (orig_bytes_free - cur_bytes_free >= kFileSizeBytes) break;
-  }
-  ASSERT_GE(orig_bytes_free - cur_bytes_free, kFileSizeBytes)
-      << "Failed after " << kIters << " attempts";
+
+    SpaceInfo after_space_info;
+    ASSERT_OK(env_->GetSpaceInfo(kDataDir, &after_space_info));
+    ASSERT_GE(before_space_info.free_bytes - after_space_info.free_bytes, kFileSizeBytes);
+  }));
+}
+
+// Basic sanity check for GetSpaceInfo().
+TEST_F(TestEnv, TestGetSpaceInfoBasicInvariants) {
+  string path = GetTestDataDirectory();
+  SpaceInfo space_info;
+  ASSERT_OK(env_->GetSpaceInfo(path, &space_info));
+  ASSERT_GT(space_info.capacity_bytes, 0);
+  ASSERT_LE(space_info.free_bytes, space_info.capacity_bytes);
+  VLOG(1) << "Path " << path << " has capacity "
+          << HumanReadableNumBytes::ToString(space_info.capacity_bytes)
+          << " (" << HumanReadableNumBytes::ToString(space_info.free_bytes) <<
" free)";
 }
 
 TEST_F(TestEnv, TestChangeDir) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/573e232d/src/kudu/util/env.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 442a451..bbf8f9c 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -36,6 +36,12 @@ struct RandomAccessFileOptions;
 struct RWFileOptions;
 struct WritableFileOptions;
 
+// Returned by Env::GetSpaceInfo().
+struct SpaceInfo {
+  int64_t capacity_bytes; // Capacity of a filesystem, in bytes.
+  int64_t free_bytes;     // Bytes available to non-privileged processes.
+};
+
 class Env {
  public:
   // Governs if/how the file is created.
@@ -191,10 +197,10 @@ class Env {
   // *block_size. fname must exist but it may be a file or a directory.
   virtual Status GetBlockSize(const std::string& fname, uint64_t* block_size) = 0;
 
-  // Determine the number of bytes free on the filesystem specified by 'path'.
-  // "Free space" accounting on the underlying filesystem may be more coarse
-  // than single bytes.
-  virtual Status GetBytesFree(const std::string& path, int64_t* bytes_free) = 0;
+  // Determine the capacity and number of bytes free on the filesystem
+  // specified by 'path'. "Free space" accounting on the underlying filesystem
+  // may be more coarse than single bytes.
+  virtual Status GetSpaceInfo(const std::string& path, SpaceInfo* space_info) = 0;
 
   // Rename file src to target.
   virtual Status RenameFile(const std::string& src,

http://git-wip-us.apache.org/repos/asf/kudu/blob/573e232d/src/kudu/util/env_posix.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index 6158bdc..4da3d47 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -1020,15 +1020,12 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status GetBytesFree(const string& path, int64_t* bytes_free) OVERRIDE {
-    TRACE_EVENT1("io", "PosixEnv::GetBytesFree", "path", path);
+  virtual Status GetSpaceInfo(const string& path, SpaceInfo* space_info) OVERRIDE {
+    TRACE_EVENT1("io", "PosixEnv::GetSpaceInfo", "path", path);
     struct statvfs buf;
     RETURN_NOT_OK(StatVfs(path, &buf));
-    if (geteuid() == 0) {
-      *bytes_free = buf.f_frsize * buf.f_bfree;
-    } else {
-      *bytes_free = buf.f_frsize * buf.f_bavail;
-    }
+    space_info->capacity_bytes = buf.f_frsize * buf.f_blocks;
+    space_info->free_bytes = buf.f_frsize * buf.f_bavail;
     return Status::OK();
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/573e232d/src/kudu/util/env_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_util.cc b/src/kudu/util/env_util.cc
index 178d391..4c7e93c 100644
--- a/src/kudu/util/env_util.cc
+++ b/src/kudu/util/env_util.cc
@@ -125,22 +125,23 @@ Status VerifySufficientDiskSpace(Env *env, const std::string& path,
                                  int64_t requested_bytes, int64_t reserved_bytes) {
   DCHECK_GE(requested_bytes, 0);
 
-  int64_t bytes_free;
-  RETURN_NOT_OK(env->GetBytesFree(path, &bytes_free));
+  SpaceInfo space_info;
+  RETURN_NOT_OK(env->GetSpaceInfo(path, &space_info));
+  int64_t available_bytes = space_info.free_bytes;
 
   // Allow overriding these values by tests.
   if (PREDICT_FALSE(FLAGS_disk_reserved_bytes_free_for_testing > -1)) {
-    bytes_free = FLAGS_disk_reserved_bytes_free_for_testing;
+    available_bytes = FLAGS_disk_reserved_bytes_free_for_testing;
   }
   if (PREDICT_FALSE(FLAGS_disk_reserved_override_prefix_1_bytes_free_for_testing != -1 ||
                     FLAGS_disk_reserved_override_prefix_2_bytes_free_for_testing != -1))
{
-    OverrideBytesFreeWithTestingFlags(path, &bytes_free);
+    OverrideBytesFreeWithTestingFlags(path, &available_bytes);
   }
 
-  if (bytes_free - requested_bytes < reserved_bytes) {
+  if (available_bytes - requested_bytes < reserved_bytes) {
     return Status::IOError(Substitute("Insufficient disk space to allocate $0 bytes under
path $1 "
-                                      "($2 bytes free vs $3 bytes reserved)",
-                                      requested_bytes, path, bytes_free, reserved_bytes),
+                                      "($2 bytes available vs $3 bytes reserved)",
+                                      requested_bytes, path, available_bytes, reserved_bytes),
                            "", ENOSPC);
   }
   return Status::OK();


Mime
View raw message