You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by jd...@apache.org on 2017/03/08 18:25:17 UTC
[3/3] kudu git commit: Reserve 1% of disk space by default
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 <ad...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/6278
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
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 <mp...@apache.org>
Authored: Tue Feb 28 17:23:09 2017 -0800
Committer: Jean-Daniel Cryans <jd...@apache.org>
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);