Reserve 1% of disk space by default Many people are not aware that Kudu is able to reserve a certain amount of space per disk for non-Kudu usage and they don't set the corresponding flags. Let's bump the default from 0 to a special value indicating one percent, which seems like a reasonable default in most cases.
Change-Id: I578afeefd9a520fd56bfca597c5fcec8f0f3f98d Reviewed-on: http://gerrit.cloudera.org:8080/6192 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo <[email protected]> Reviewed-on: http://gerrit.cloudera.org:8080/6278 Reviewed-by: Jean-Daniel Cryans <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/86055e29 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/86055e29 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/86055e29 Branch: refs/heads/branch-1.3.x Commit: 86055e2950a3cb606ea567c2235da434dae5a72b Parents: 7395221 Author: Mike Percy <[email protected]> Authored: Tue Feb 28 17:23:09 2017 -0800 Committer: Jean-Daniel Cryans <[email protected]> Committed: Wed Mar 8 18:24:23 2017 +0000 ---------------------------------------------------------------------- src/kudu/consensus/log.cc | 10 ++++++++-- src/kudu/fs/data_dirs.cc | 10 ++++++++-- src/kudu/util/env_util-test.cc | 32 +++++++++++++++++++++++++------- src/kudu/util/env_util.cc | 6 ++++++ src/kudu/util/env_util.h | 3 ++- 5 files changed, 49 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/86055e29/src/kudu/consensus/log.cc ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc index 716baa0..d51e326 100644 --- a/src/kudu/consensus/log.cc +++ b/src/kudu/consensus/log.cc @@ -116,8 +116,14 @@ DEFINE_double(log_inject_io_error_on_preallocate_fraction, 0.0, TAG_FLAG(log_inject_io_error_on_preallocate_fraction, unsafe); TAG_FLAG(log_inject_io_error_on_preallocate_fraction, runtime); -DEFINE_int64(fs_wal_dir_reserved_bytes, 0, - "Number of bytes to reserve on the log directory filesystem for non-Kudu usage"); +DEFINE_int64(fs_wal_dir_reserved_bytes, -1, + "Number of bytes to reserve on the log directory filesystem for " + "non-Kudu usage. The default, which is represented by -1, is that " + "1% of the disk space on each disk will be reserved. Any other " + "value specified represents the number of bytes reserved and must " + "be greater than or equal to 0. Explicit percentages to reserve " + "are not currently supported"); +DEFINE_validator(fs_wal_dir_reserved_bytes, [](const char* /*n*/, int64_t v) { return v >= -1; }); TAG_FLAG(fs_wal_dir_reserved_bytes, runtime); TAG_FLAG(fs_wal_dir_reserved_bytes, evolving); http://git-wip-us.apache.org/repos/asf/kudu/blob/86055e29/src/kudu/fs/data_dirs.cc ---------------------------------------------------------------------- diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc index 8453378..44e9f1b 100644 --- a/src/kudu/fs/data_dirs.cc +++ b/src/kudu/fs/data_dirs.cc @@ -51,8 +51,14 @@ #include "kudu/util/stopwatch.h" #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."); +DEFINE_int64(fs_data_dirs_reserved_bytes, -1, + "Number of bytes to reserve on each data directory filesystem for " + "non-Kudu usage. The default, which is represented by -1, is that " + "1% of the disk space on each disk will be reserved. Any other " + "value specified represents the number of bytes reserved and must " + "be greater than or equal to 0. Explicit percentages to reserve " + "are not currently supported"); +DEFINE_validator(fs_data_dirs_reserved_bytes, [](const char* /*n*/, int64_t v) { return v >= -1; }); 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/86055e29/src/kudu/util/env_util-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/env_util-test.cc b/src/kudu/util/env_util-test.cc index 7c79068..715f03b 100644 --- a/src/kudu/util/env_util-test.cc +++ b/src/kudu/util/env_util-test.cc @@ -33,7 +33,6 @@ #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" -DECLARE_int64(disk_reserved_bytes); DECLARE_int64(disk_reserved_bytes_free_for_testing); using std::string; @@ -47,20 +46,39 @@ namespace env_util { class EnvUtilTest: public KuduTest { }; +// Assert that Status 's' indicates there is not enough space left on the +// device for the request. +static void AssertNoSpace(const Status& s) { + ASSERT_TRUE(s.IsIOError()); + ASSERT_EQ(ENOSPC, s.posix_code()); + ASSERT_STR_CONTAINS(s.ToString(), "Insufficient disk space"); +} + TEST_F(EnvUtilTest, TestDiskSpaceCheck) { - const int64_t kRequestedBytes = 0; + const int64_t kZeroRequestedBytes = 0; + const int64_t kRequestOnePercentReservation = -1; int64_t reserved_bytes = 0; - ASSERT_OK(VerifySufficientDiskSpace(env_, test_dir_, kRequestedBytes, reserved_bytes)); + ASSERT_OK(VerifySufficientDiskSpace(env_, test_dir_, kZeroRequestedBytes, reserved_bytes)); + + // Check 1% reservation logic. We loop this in case there are other FS + // operations happening concurrent with this test. + NO_FATALS(AssertEventually([&] { + SpaceInfo space_info; + ASSERT_OK(env_->GetSpaceInfo(test_dir_, &space_info)); + // Try for 1 less byte than 1% free. This request should be rejected. + int64_t target_free_bytes = (space_info.capacity_bytes / 100) - 1; + int64_t bytes_to_request = std::max(0L, space_info.free_bytes - target_free_bytes); + NO_FATALS(AssertNoSpace(VerifySufficientDiskSpace(env_, test_dir_, bytes_to_request, + kRequestOnePercentReservation))); + })); // Make it seem as if the disk is full and specify that we should have // reserved 200 bytes. Even asking for 0 bytes should return an error // indicating we are out of space. FLAGS_disk_reserved_bytes_free_for_testing = 0; reserved_bytes = 200; - Status s = VerifySufficientDiskSpace(env_, test_dir_, kRequestedBytes, reserved_bytes); - ASSERT_TRUE(s.IsIOError()); - ASSERT_EQ(ENOSPC, s.posix_code()); - ASSERT_STR_CONTAINS(s.ToString(), "Insufficient disk space"); + NO_FATALS(AssertNoSpace(VerifySufficientDiskSpace(env_, test_dir_, kZeroRequestedBytes, + reserved_bytes))); } // Ensure that we can recursively create directories using both absolute and http://git-wip-us.apache.org/repos/asf/kudu/blob/86055e29/src/kudu/util/env_util.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/env_util.cc b/src/kudu/util/env_util.cc index 4c7e93c..b471951 100644 --- a/src/kudu/util/env_util.cc +++ b/src/kudu/util/env_util.cc @@ -123,6 +123,7 @@ static void OverrideBytesFreeWithTestingFlags(const string& path, int64_t* bytes Status VerifySufficientDiskSpace(Env *env, const std::string& path, int64_t requested_bytes, int64_t reserved_bytes) { + const int64_t kOnePercentReservation = -1; DCHECK_GE(requested_bytes, 0); SpaceInfo space_info; @@ -138,6 +139,11 @@ Status VerifySufficientDiskSpace(Env *env, const std::string& path, OverrideBytesFreeWithTestingFlags(path, &available_bytes); } + // If they requested a one percent reservation, calculate what that is in bytes. + if (reserved_bytes == kOnePercentReservation) { + reserved_bytes = space_info.capacity_bytes / 100; + } + if (available_bytes - requested_bytes < reserved_bytes) { return Status::IOError(Substitute("Insufficient disk space to allocate $0 bytes under path $1 " "($2 bytes available vs $3 bytes reserved)", http://git-wip-us.apache.org/repos/asf/kudu/blob/86055e29/src/kudu/util/env_util.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/env_util.h b/src/kudu/util/env_util.h index 884b17c..a2f96a4 100644 --- a/src/kudu/util/env_util.h +++ b/src/kudu/util/env_util.h @@ -42,7 +42,8 @@ Status OpenFileForSequential(Env *env, const std::string &path, // Returns Status::IOError with POSIX code ENOSPC if there is not sufficient // disk space to write 'bytes' bytes to the file system represented by 'path'. // Otherwise returns OK. - +// If 'reserved_bytes' equals -1, it is interpreted as a 1% reservation. No +// other values less than 0 are supported at this time. Status VerifySufficientDiskSpace(Env *env, const std::string& path, int64_t requested_bytes, int64_t reserved_bytes);
