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 <[email protected]> Reviewed-by: David Ribeiro Alves <[email protected]> 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 <[email protected]> Authored: Thu Mar 2 12:39:52 2017 -0800 Committer: Mike Percy <[email protected]> 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();
