From 0a77617820f6a6a3ebde2a59a5e43c0f0e318b21 Mon Sep 17 00:00:00 2001 From: Cheng Chang Date: Fri, 24 Apr 2020 23:58:13 -0700 Subject: [PATCH] Disable O_DIRECT in stress test when db directory does not support direct IO (#6727) Summary: In crash test, the db directory might be set to /dev/shm or /tmp, in certain environments such as internal testing infrastructure, neither of these directories support direct IO, so direct IO is never enabled in crash test. This PR sets up SyncPoints in direct IO related code paths to disable O_DIRECT flag in calls to `open`, so the direct IO code paths will be executed, all direct IO related assertions will be checked, but no real direct IO request will be issued to the file system. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6727 Test Plan: export CRASH_TEST_EXT_ARGS="--use_direct_reads=1 --mmap_read=0" make -j24 crash_test Reviewed By: zhichao-cao Differential Revision: D21139250 Pulled By: cheng-chang fbshipit-source-id: db9adfe78d91aa4759835b1af91c5db7b27b62ee --- db/db_test_util.cc | 15 +-------------- db_stress_tool/db_stress_common.h | 1 + db_stress_tool/db_stress_gflags.cc | 3 +++ db_stress_tool/db_stress_tool.cc | 4 ++++ test_util/testutil.cc | 19 +++++++++++++++++++ test_util/testutil.h | 4 ++++ tools/db_crashtest.py | 23 +++++++++++++++++++---- 7 files changed, 51 insertions(+), 18 deletions(-) diff --git a/db/db_test_util.cc b/db/db_test_util.cc index c73abde41..a8651b8da 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -399,20 +399,7 @@ Options DBTestBase::GetOptions( options.use_direct_reads = true; options.use_direct_io_for_flush_and_compaction = true; options.compaction_readahead_size = 2 * 1024 * 1024; - #if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && \ - !defined(OS_AIX) && !defined(OS_OPENBSD) - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "NewWritableFile:O_DIRECT", [&](void* arg) { - int* val = static_cast(arg); - *val &= ~O_DIRECT; - }); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "NewRandomAccessFile:O_DIRECT", [&](void* arg) { - int* val = static_cast(arg); - *val &= ~O_DIRECT; - }); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); -#endif + test::SetupSyncPointsToMockDirectIO(); break; } #endif // ROCKSDB_LITE diff --git a/db_stress_tool/db_stress_common.h b/db_stress_tool/db_stress_common.h index b4ac518b7..ff71b889f 100644 --- a/db_stress_tool/db_stress_common.h +++ b/db_stress_tool/db_stress_common.h @@ -155,6 +155,7 @@ DECLARE_bool(mmap_read); DECLARE_bool(mmap_write); DECLARE_bool(use_direct_reads); DECLARE_bool(use_direct_io_for_flush_and_compaction); +DECLARE_bool(mock_direct_io); DECLARE_bool(statistics); DECLARE_bool(sync); DECLARE_bool(use_fsync); diff --git a/db_stress_tool/db_stress_gflags.cc b/db_stress_tool/db_stress_gflags.cc index a4d502261..f4f538c66 100644 --- a/db_stress_tool/db_stress_gflags.cc +++ b/db_stress_tool/db_stress_gflags.cc @@ -395,6 +395,9 @@ DEFINE_bool(use_direct_io_for_flush_and_compaction, ROCKSDB_NAMESPACE::Options().use_direct_io_for_flush_and_compaction, "Use O_DIRECT for writing data"); +DEFINE_bool(mock_direct_io, false, + "Mock direct IO by not using O_DIRECT for direct IO read"); + DEFINE_bool(statistics, false, "Create database statistics"); DEFINE_bool(sync, false, "Sync all writes to disk"); diff --git a/db_stress_tool/db_stress_tool.cc b/db_stress_tool/db_stress_tool.cc index 925ccfcef..d7c4fcbac 100644 --- a/db_stress_tool/db_stress_tool.cc +++ b/db_stress_tool/db_stress_tool.cc @@ -45,6 +45,10 @@ int db_stress_tool(int argc, char** argv) { SanitizeDoubleParam(&FLAGS_memtable_prefix_bloom_size_ratio); SanitizeDoubleParam(&FLAGS_max_bytes_for_level_multiplier); + if (FLAGS_mock_direct_io) { + test::SetupSyncPointsToMockDirectIO(); + } + if (FLAGS_statistics) { dbstats = ROCKSDB_NAMESPACE::CreateDBStatistics(); if (FLAGS_test_secondary) { diff --git a/test_util/testutil.cc b/test_util/testutil.cc index 65c8a7c93..4c6ef0999 100644 --- a/test_util/testutil.cc +++ b/test_util/testutil.cc @@ -9,6 +9,7 @@ #include "test_util/testutil.h" +#include #include #include #include @@ -20,6 +21,7 @@ #include "file/sequence_file_reader.h" #include "file/writable_file_writer.h" #include "port/port.h" +#include "test_util/sync_point.h" namespace ROCKSDB_NAMESPACE { namespace test { @@ -521,5 +523,22 @@ void ResetTmpDirForDirectIO() { #endif } +void SetupSyncPointsToMockDirectIO() { +#if !defined(NDEBUG) && !defined(OS_MACOSX) && !defined(OS_WIN) && \ + !defined(OS_SOLARIS) && !defined(OS_AIX) && !defined(OS_OPENBSD) + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( + "NewWritableFile:O_DIRECT", [&](void* arg) { + int* val = static_cast(arg); + *val &= ~O_DIRECT; + }); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( + "NewRandomAccessFile:O_DIRECT", [&](void* arg) { + int* val = static_cast(arg); + *val &= ~O_DIRECT; + }); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); +#endif +} + } // namespace test } // namespace ROCKSDB_NAMESPACE diff --git a/test_util/testutil.h b/test_util/testutil.h index ef1f2913e..83779b538 100644 --- a/test_util/testutil.h +++ b/test_util/testutil.h @@ -805,5 +805,9 @@ size_t GetLinesCount(const std::string& fname, const std::string& pattern); // Tries to set TEST_TMPDIR to a directory supporting direct IO. void ResetTmpDirForDirectIO(); +// Sets up sync points to mock direct IO instead of actually issuing direct IO +// to the file system. +void SetupSyncPointsToMockDirectIO(); + } // namespace test } // namespace ROCKSDB_NAMESPACE diff --git a/tools/db_crashtest.py b/tools/db_crashtest.py index a395a3637..e8cdb9d42 100644 --- a/tools/db_crashtest.py +++ b/tools/db_crashtest.py @@ -81,6 +81,7 @@ default_params = { "target_file_size_multiplier": 2, "use_direct_reads": lambda: random.randint(0, 1), "use_direct_io_for_flush_and_compaction": lambda: random.randint(0, 1), + "mock_direct_io": False, "use_full_merge_v1": lambda: random.randint(0, 1), "use_merge": lambda: random.randint(0, 1), "verify_checksum": 1, @@ -121,14 +122,20 @@ default_params = { } _TEST_DIR_ENV_VAR = 'TEST_TMPDIR' +_DEBUG_LEVEL_ENV_VAR = 'DEBUG_LEVEL' + + +def is_release_mode(): + return os.environ.get(_DEBUG_LEVEL_ENV_VAR) == "0" def get_dbname(test_name): + test_dir_name = "rocksdb_crashtest_" + test_name test_tmpdir = os.environ.get(_TEST_DIR_ENV_VAR) if test_tmpdir is None or test_tmpdir == "": - dbname = tempfile.mkdtemp(prefix='rocksdb_crashtest_' + test_name) + dbname = tempfile.mkdtemp(prefix=test_dir_name) else: - dbname = test_tmpdir + "/rocksdb_crashtest_" + test_name + dbname = test_tmpdir + "/" + test_dir_name shutil.rmtree(dbname, True) os.mkdir(dbname) return dbname @@ -215,10 +222,18 @@ def finalize_and_sanitize(src_params): dest_params["compression_zstd_max_train_bytes"] = 0 if dest_params.get("allow_concurrent_memtable_write", 1) == 1: dest_params["memtablerep"] = "skip_list" - if dest_params["mmap_read"] == 1 or not is_direct_io_supported( - dest_params["db"]): + if dest_params["mmap_read"] == 1: dest_params["use_direct_io_for_flush_and_compaction"] = 0 dest_params["use_direct_reads"] = 0 + if (dest_params["use_direct_io_for_flush_and_compaction"] == 1 + or dest_params["use_direct_reads"] == 1) and \ + not is_direct_io_supported(dest_params["db"]): + if is_release_mode(): + print("{} does not support direct IO".format(dest_params["db"])) + sys.exit(1) + else: + dest_params["mock_direct_io"] = True + # DeleteRange is not currnetly compatible with Txns if dest_params.get("test_batches_snapshots") == 1 or \ dest_params.get("use_txn") == 1: