Skip to content

Commit

Permalink
Detecting if the data/log/wals allow file creation with O_DIRECT.
Browse files Browse the repository at this point in the history
Summary:
This code tests that data/logs/wals allow file creation with O_DIRECT flag and gives out an
appropiate message if it cannot. If it does not and durable_wal_write flag is set, we check if the
prefer_durable_write flag is set of not. If it is set, we crash with the appropiate error otherwise
we soft downgrade durable_wal_write flag to false.

Test Plan:
1) Unit test in path_util-test.cc
2) Simulate this by trying to create a cluster with WAL/Data dirs pointing to a file system that
does not allow O_DIRECT writes.

Reviewers: bogdan, mikhail

Reviewed By: mikhail

Subscribers: ybase, bharat

Differential Revision: https://phabricator.dev.yugabyte.com/D4123
  • Loading branch information
ayushsengupta1991 committed Apr 2, 2018
1 parent d79f33a commit 4c5813a
Show file tree
Hide file tree
Showing 11 changed files with 183 additions and 12 deletions.
4 changes: 2 additions & 2 deletions src/yb/consensus/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ set(CONSENSUS_SRCS
peer_manager.cc
quorum_util.cc
raft_consensus.cc
replica_state.cc
)
replica_state.cc)

add_library(consensus ${CONSENSUS_SRCS})
cotire(consensus)
Expand Down Expand Up @@ -156,6 +155,7 @@ ADD_YB_TEST(mt-log-test)
ADD_YB_TEST(quorum_util-test)
ADD_YB_TEST(raft_consensus_quorum-test)
ADD_YB_TEST(replica_state-test)
ADD_YB_TEST(log_util-test)

set_source_files_properties(raft_consensus-test.cc PROPERTIES COMPILE_FLAGS
"-Wno-inconsistent-missing-override")
Expand Down
50 changes: 50 additions & 0 deletions src/yb/consensus/log_util-test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) YugaByte, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
// in compliance with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software distributed under the License
// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
// or implied. See the License for the specific language governing permissions and limitations
// under the License.
//

#include <gtest/gtest.h>

#include "yb/consensus/log_util.h"
#include "yb/util/test_macros.h"

namespace yb {

#if defined(__linux__)
TEST(TestLogUtil, TestModifyDurableWriteFlagIfNotDirectNegative) {
gflags::FlagSaver flag_saver;
FLAGS_fs_data_dirs = "/var/run";

// Test that the method does not crash and durable_wal_write is flipped
// when directory is not O_DIRECT writable.
FLAGS_durable_wal_write = true;
FLAGS_require_durable_wal_write = false;
ASSERT_OK(log::ModifyDurableWriteFlagIfNotODirect());
ASSERT_EQ(FLAGS_durable_wal_write, false);

// Test that the method crashes when durable_wal_write is set to true.
FLAGS_durable_wal_write = true;
FLAGS_require_durable_wal_write = true;
ASSERT_NOK(log::ModifyDurableWriteFlagIfNotODirect());
}
#endif

TEST(TestLogUtil, TestModifyDurableWriteFlagIfNotODirectPositive) {
gflags::FlagSaver flag_saver;
// Test that the method does not crash when durable_wal_write is set true and paths are O_DIRECT
// Writable.
FLAGS_durable_wal_write = true;
ASSERT_OK(log::ModifyDurableWriteFlagIfNotODirect());
// Test that the method does not crash when durable_wal_write is set to false.
FLAGS_durable_wal_write = false;
ASSERT_OK(log::ModifyDurableWriteFlagIfNotODirect());
}
} // namespace yb
55 changes: 55 additions & 0 deletions src/yb/consensus/log_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ DEFINE_bool(log_async_preallocate_segments, true,
"Whether the WAL segments preallocation should happen asynchronously");
TAG_FLAG(log_async_preallocate_segments, advanced);

DECLARE_string(fs_data_dirs);

DEFINE_bool(require_durable_wal_write, false, "Whether durable WAL write is required."
"In case you cannot write using O_DIRECT in WAL and data directories and this flag is set true"
"the system will deliberately crash with the appropriate error. If this flag is set false, "
"the system will soft downgrade the durable_wal_write flag.");
TAG_FLAG(require_durable_wal_write, stable);

namespace yb {
namespace log {

Expand Down Expand Up @@ -845,5 +853,52 @@ bool IsLogFileName(const string& fname) {
return true;
}

std::vector<std::string> ParseDirFlags(string flag_dirs, string flag_name) {
std::vector<std::string> paths = strings::Split(flag_dirs, ",", strings::SkipEmpty());
if (paths.size() < 1) {
LOG(ERROR) << "Flag " << flag_name << " is empty.";
}
return paths;
}

Status CheckPathsAreODirectWritable(const std::vector<std::string> &paths) {
Env *def_env = Env::Default();
for (const auto &path : paths) {
RETURN_NOT_OK(CheckODirectTempFileCreationInDir(def_env, path));
}
return Status::OK();
}

Status CheckRelevantPathsAreODirectWritable() {
if (!FLAGS_log_dir.empty()) {
RETURN_NOT_OK_PREPEND(CheckPathsAreODirectWritable(ParseDirFlags(
FLAGS_log_dir, "--log_dir")), "Not all log_dirs are O_DIRECT Writable.");
}
RETURN_NOT_OK_PREPEND(CheckPathsAreODirectWritable(ParseDirFlags(
FLAGS_fs_data_dirs, "--data_dirs")), "Not all fs_data_dirs are O_DIRECT Writable.");

RETURN_NOT_OK_PREPEND(CheckPathsAreODirectWritable(ParseDirFlags(
FLAGS_fs_wal_dirs, "--wal_dirs")), "Not all fs_wal_dirs are O_DIRECT Writable.");
return Status::OK();
}

Status ModifyDurableWriteFlagIfNotODirect() {
if (FLAGS_durable_wal_write) {
Status s = CheckRelevantPathsAreODirectWritable();
if (!s.ok()) {
if (FLAGS_require_durable_wal_write) {
// Crash with appropriate error.
RETURN_NOT_OK_PREPEND(s, "require_durable_wal_write is set true, but O_DIRECT is "
"not allowed.")
} else {
// Report error but do not crash.
LOG(ERROR) << "O_DIRECT is not allowed in some of the directories. "
"Setting durable wal write flag to false.";
FLAGS_durable_wal_write = false;
}
}
}
return Status::OK();
}
} // namespace log
} // namespace yb
10 changes: 9 additions & 1 deletion src/yb/consensus/log_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@

// Used by other classes, now part of the API.
DECLARE_bool(durable_wal_write);
DECLARE_bool(require_durable_wal_write);
DECLARE_string(fs_wal_dirs);
DECLARE_string(fs_data_dirs);

namespace yb {

Expand Down Expand Up @@ -99,7 +102,6 @@ struct LogOptions {
LogOptions();
};


// A sequence of segments, ordered by increasing sequence number.
typedef std::vector<scoped_refptr<ReadableLogSegment> > SegmentSequence;
typedef std::vector<std::unique_ptr<LogEntryPB>> LogEntries;
Expand Down Expand Up @@ -418,6 +420,12 @@ void CreateBatchFromAllocatedOperations(const ReplicateMsgs& msgs,
// Checks if 'fname' is a correctly formatted name of log segment file.
bool IsLogFileName(const std::string& fname);

CHECKED_STATUS CheckPathsAreODirectWritable(const std::vector<std::string>& paths);
CHECKED_STATUS CheckRelevantPathsAreODirectWritable();

// Modify durable wal write flag depending on the value of FLAGS_require_durable_wal_write.
CHECKED_STATUS ModifyDurableWriteFlagIfNotODirect();

} // namespace log
} // namespace yb

Expand Down
4 changes: 2 additions & 2 deletions src/yb/fs/fs_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ DECLARE_string(fs_data_dirs);

DEFINE_string(fs_wal_dirs, "",
"Comma-separated list of directories for write-ahead logs. This is an optional "
"argument. If this is not specified, fs_data_dirs is used for write-ahead logs "
"also and that's a reasonable default for most use cases.");
"argument. If this is not specified, fs_data_dirs is used for write-ahead logs "
"also and that's a reasonable default for most use cases.");
TAG_FLAG(fs_wal_dirs, stable);

using google::protobuf::Message;
Expand Down
3 changes: 2 additions & 1 deletion src/yb/master/master_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "yb/gutil/strings/substitute.h"
#include "yb/master/call_home.h"
#include "yb/master/master.h"
#include "yb/consensus/log_util.h"
#include "yb/util/flags.h"
#include "yb/util/init.h"
#include "yb/util/logging.h"
Expand Down Expand Up @@ -74,7 +75,7 @@ static int MasterMain(int argc, char** argv) {
std::cerr << "usage: " << argv[0] << std::endl;
return 1;
}

LOG_AND_RETURN_FROM_MAIN_NOT_OK(log::ModifyDurableWriteFlagIfNotODirect());
LOG_AND_RETURN_FROM_MAIN_NOT_OK(InitYB(MasterOptions::kServerType, argv[0]));

auto opts_result = MasterOptions::CreateMasterOptions();
Expand Down
2 changes: 2 additions & 0 deletions src/yb/tserver/tablet_server_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "yb/rpc/io_thread_pool.h"
#include "yb/rpc/scheduler.h"
#include "yb/tserver/tablet_server.h"
#include "yb/consensus/log_util.h"
#include "yb/util/flags.h"
#include "yb/util/init.h"
#include "yb/util/logging.h"
Expand Down Expand Up @@ -112,6 +113,7 @@ static int TabletServerMain(int argc, char** argv) {
std::cerr << "usage: " << argv[0] << std::endl;
return 1;
}
LOG_AND_RETURN_FROM_MAIN_NOT_OK(log::ModifyDurableWriteFlagIfNotODirect());
LOG_AND_RETURN_FROM_MAIN_NOT_OK(InitYB(TabletServerOptions::kServerType, argv[0]));

#ifdef TCMALLOC_ENABLED
Expand Down
3 changes: 3 additions & 0 deletions src/yb/util/main_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

#include "yb/util/result.h"
#include "yb/util/status.h"
#include "yb/util/env.h"
#include "yb/gutil/strings/split.h"
#include "yb/util/path_util.h"

namespace yb {

Expand Down
10 changes: 10 additions & 0 deletions src/yb/util/path_util-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
#include <gtest/gtest.h>

#include "yb/util/path_util.h"
#include "yb/util/test_macros.h"
#include "yb/util/init.h"

namespace yb {

Expand Down Expand Up @@ -78,4 +80,12 @@ TEST(TestPathUtil, JoinPathSegments) {
ASSERT_EQ(JoinPathSegments("/usr/bin/", "vim"), "/usr/bin/vim");
}


#if defined(__linux__)
TEST(TestPathUtil, TestODirectFileCreationInDir) {
string dir = "/var/run";
Env* env_test = yb::Env::Default();
ASSERT_NOK(CheckODirectTempFileCreationInDir(env_test, dir));
}
#endif
} // namespace yb
47 changes: 42 additions & 5 deletions src/yb/util/path_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,28 @@

// Use the POSIX version of dirname(3).
#include <libgen.h>
#include <fcntl.h>

#include <string>

#include "yb/util/env.h"
#include "yb/util/env_util.h"
#include "yb/gutil/gscoped_ptr.h"

#if defined(__APPLE__)
#include <mutex>
#endif // defined(__APPLE__)

#if defined(__linux__)
#include <linux/falloc.h>
#include <sys/sysinfo.h>
#endif

#include "yb/util/env_util.h"
#include "yb/util/debug/trace_event.h"
#include "yb/gutil/gscoped_ptr.h"

using std::string;

namespace yb {

static const char* const kTmpTemplateSuffix = ".tmp.XXXXXX";

std::string JoinPathSegments(const std::string &a,
const std::string &b) {
CHECK(!a.empty()) << "empty first component: " << a;
Expand All @@ -62,6 +69,16 @@ std::string JoinPathSegments(const std::string &a,
}
}

Status FileCreationError(const std::string& path_dir, int err_number) {
switch (err_number) {
case EACCES: FALLTHROUGH_INTENDED;
case EPERM: FALLTHROUGH_INTENDED;
case EINVAL:
return STATUS(NotSupported, path_dir, ErrnoToString(err_number), err_number);
}
return Status::OK();
}

string DirName(const string& path) {
gscoped_ptr<char[], FreeDeleter> path_copy(strdup(path.c_str()));
#if defined(__APPLE__)
Expand Down Expand Up @@ -99,4 +116,24 @@ Status SetupRootDir(
return Status::OK();
}

Status CheckODirectTempFileCreationInDir(Env* env,
const std::string& dir_path) {
string name_template = dir_path + kTmpTemplateSuffix;
ThreadRestrictions::AssertIOAllowed();
std::unique_ptr<char[]> fname(new char[name_template.size() + 1]);
::snprintf(fname.get(), name_template.size() + 1, "%s", name_template.c_str());
#if defined(__linux__)
int fd = -1;
fd = ::mkostemp(fname.get(), O_DIRECT);

if (fd < 0) {
return FileCreationError(dir_path, errno);
}

if (unlink(fname.get()) != 0) {
return FileCreationError(dir_path, errno);
}
#endif
return Status::OK();
}
} // namespace yb
7 changes: 6 additions & 1 deletion src/yb/util/path_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <string>

#include "yb/util/status.h"
#include "yb/util/env.h"

namespace yb {

Expand All @@ -60,11 +61,15 @@ std::string BaseName(const std::string& path);
std::string GetYbDataPath(const std::string& root);
std::string GetServerTypeDataPath(const std::string& root, const std::string& server_type);

// For the user specified root dirs, setup the two level heirarchy below it and report the final
// For the user specified root dirs, setup the two level hierarchy below it and report the final
// path as well as whether we created the final path or not.
Status SetupRootDir(
Env* env, const std::string& root, const std::string& server_type, std::string* out_dir,
bool* created);

// Tests whether a given path allows creation of temporary files with O_DIRECT given a
// environment variable.
Status CheckODirectTempFileCreationInDir(Env* env, const std::string& dir_path);

} // namespace yb
#endif /* YB_UTIL_PATH_UTIL_H */

0 comments on commit 4c5813a

Please sign in to comment.