From 440c7f630633e85d0a32f982779ab40fa56fd8d5 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Tue, 3 May 2022 13:38:38 -0700 Subject: [PATCH] db_basic_bench fix for DB object cleanup (#9939) Summary: Use `unique_ptr` to make sure the DB object is deleted. Previously it was not, which led to accumulating file descriptors for deleted directories because a `DBImpl::db_dir_` from each test remained alive. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9939 Test Plan: run `lsof -p $(pidof db_basic_bench)` while benchmark runs; verify no FDs for deleted directories. Reviewed By: jay-zhuang Differential Revision: D36108761 Pulled By: ajkr fbshipit-source-id: cfe02646b038a445af7d5db8989eb1f40d658359 --- microbench/db_basic_bench.cc | 89 +++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/microbench/db_basic_bench.cc b/microbench/db_basic_bench.cc index 8f4a30c84..64bff08fc 100644 --- a/microbench/db_basic_bench.cc +++ b/microbench/db_basic_bench.cc @@ -118,7 +118,8 @@ class KeyGenerator { } }; -static void SetupDB(benchmark::State& state, Options& options, DB** dpptr, +static void SetupDB(benchmark::State& state, Options& options, + std::unique_ptr* db, const std::string& test_name = "") { options.create_if_missing = true; auto env = Env::Default(); @@ -132,15 +133,17 @@ static void SetupDB(benchmark::State& state, Options& options, DB** dpptr, db_path + kFilePathSeparator + test_name + std::to_string(getpid()); DestroyDB(db_name, options); - s = DB::Open(options, db_name, dpptr); + DB* db_ptr = nullptr; + s = DB::Open(options, db_name, &db_ptr); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); return; } + db->reset(db_ptr); } -static void TeardownDB(benchmark::State& state, DB* db, const Options& options, - KeyGenerator& kg) { +static void TeardownDB(benchmark::State& state, const std::unique_ptr& db, + const Options& options, KeyGenerator& kg) { char min_buff[256], max_buff[256]; const Range r(kg.MinKey(min_buff), kg.MaxKey(max_buff)); uint64_t size; @@ -160,7 +163,7 @@ static void TeardownDB(benchmark::State& state, DB* db, const Options& options, static void DBOpen(benchmark::State& state) { // create DB - DB* db = nullptr; + std::unique_ptr db; Options options; SetupDB(state, options, &db, "DBOpen"); @@ -172,12 +175,17 @@ static void DBOpen(benchmark::State& state) { auto rnd = Random(123); for (auto _ : state) { - Status s = DB::Open(options, db_name, &db); - if (!s.ok()) { - state.SkipWithError(s.ToString().c_str()); + { + DB* db_ptr = nullptr; + Status s = DB::Open(options, db_name, &db_ptr); + if (!s.ok()) { + state.SkipWithError(s.ToString().c_str()); + } + db.reset(db_ptr); } state.PauseTiming(); auto wo = WriteOptions(); + Status s; for (int i = 0; i < 2; i++) { for (int j = 0; j < 100; j++) { s = db->Put(wo, rnd.RandomString(10), rnd.RandomString(100)); @@ -204,7 +212,7 @@ BENCHMARK(DBOpen)->Iterations(200); // specify iteration number as the db size static void DBClose(benchmark::State& state) { // create DB - DB* db; + std::unique_ptr db; Options options; SetupDB(state, options, &db, "DBClose"); @@ -217,11 +225,16 @@ static void DBClose(benchmark::State& state) { for (auto _ : state) { state.PauseTiming(); - Status s = DB::Open(options, db_name, &db); - if (!s.ok()) { - state.SkipWithError(s.ToString().c_str()); + { + DB* db_ptr = nullptr; + Status s = DB::Open(options, db_name, &db_ptr); + if (!s.ok()) { + state.SkipWithError(s.ToString().c_str()); + } + db.reset(db_ptr); } auto wo = WriteOptions(); + Status s; for (int i = 0; i < 2; i++) { for (int j = 0; j < 100; j++) { s = db->Put(wo, rnd.RandomString(10), rnd.RandomString(100)); @@ -255,7 +268,7 @@ static void DBPut(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db = nullptr; + static std::unique_ptr db = nullptr; Options options; if (enable_statistics) { options.statistics = CreateDBStatistics(); @@ -284,7 +297,7 @@ static void DBPut(benchmark::State& state) { } if (state.thread_index() == 0) { - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); Status s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -332,7 +345,7 @@ static void ManualCompaction(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db; + static std::unique_ptr db; Options options; if (enable_statistics) { options.statistics = CreateDBStatistics(); @@ -391,7 +404,7 @@ static void ManualCompaction(benchmark::State& state) { } if (state.thread_index() == 0) { - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -453,7 +466,7 @@ static void ManualFlush(benchmark::State& state) { bool enable_statistics = true; // setup DB - static DB* db; + static std::unique_ptr db; Options options; if (enable_statistics) { options.statistics = CreateDBStatistics(); @@ -489,7 +502,7 @@ static void ManualFlush(benchmark::State& state) { } if (state.thread_index() == 0) { - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); Status s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -529,7 +542,7 @@ static void DBGet(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db; + static std::unique_ptr db; Options options; if (enable_statistics) { options.statistics = CreateDBStatistics(); @@ -573,7 +586,7 @@ static void DBGet(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -648,7 +661,7 @@ BENCHMARK(DBGet)->Threads(8)->Iterations(kDBGetNum / 8)->Apply(DBGetArguments); static void SimpleGetWithPerfContext(benchmark::State& state) { // setup DB - static DB* db; + static std::unique_ptr db; std::string db_name; Options options; options.create_if_missing = true; @@ -668,10 +681,14 @@ static void SimpleGetWithPerfContext(benchmark::State& state) { db_name = db_path + "/simple_get_" + std::to_string(getpid()); DestroyDB(db_name, options); - s = DB::Open(options, db_name, &db); - if (!s.ok()) { - state.SkipWithError(s.ToString().c_str()); - return; + { + DB* db_ptr = nullptr; + s = DB::Open(options, db_name, &db_ptr); + if (!s.ok()) { + state.SkipWithError(s.ToString().c_str()); + return; + } + db.reset(db_ptr); } // load db auto wo = WriteOptions(); @@ -682,7 +699,7 @@ static void SimpleGetWithPerfContext(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -858,7 +875,7 @@ static void IteratorSeek(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db; + static std::unique_ptr db; Options options; if (enable_statistics) { options.statistics = CreateDBStatistics(); @@ -894,7 +911,7 @@ static void IteratorSeek(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -961,7 +978,7 @@ static void IteratorNext(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db; + static std::unique_ptr db; Options options; options.compaction_style = compaction_style; @@ -987,7 +1004,7 @@ static void IteratorNext(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -1036,7 +1053,7 @@ BENCHMARK(IteratorNext) static void IteratorNextWithPerfContext(benchmark::State& state) { // setup DB - static DB* db; + static std::unique_ptr db; Options options; auto rnd = Random(301 + state.thread_index()); @@ -1053,7 +1070,7 @@ static void IteratorNextWithPerfContext(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); Status s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -1128,7 +1145,7 @@ static void IteratorPrev(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db; + static std::unique_ptr db; std::string db_name; Options options; options.compaction_style = compaction_style; @@ -1155,7 +1172,7 @@ static void IteratorPrev(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); @@ -1212,7 +1229,7 @@ static void PrefixSeek(benchmark::State& state) { uint64_t key_num = max_data / per_key_size; // setup DB - static DB* db; + static std::unique_ptr db; Options options; if (enable_statistics) { options.statistics = CreateDBStatistics(); @@ -1249,7 +1266,7 @@ static void PrefixSeek(benchmark::State& state) { state.SkipWithError(s.ToString().c_str()); } - auto db_full = static_cast_with_check(db); + auto db_full = static_cast_with_check(db.get()); s = db_full->WaitForCompact(true); if (!s.ok()) { state.SkipWithError(s.ToString().c_str()); -- GitLab