From 2f1700c8c59830154e694775e582bd6b43534345 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Tue, 5 May 2020 13:09:12 -0700 Subject: [PATCH] Fix failure to write output in SpecialEnv::GetCurrentTime (#6803) Summary: This very old test code bug was causing a new valgrind failure in MultiGetDeadlineExceeded Also fix hang in MultiGetDeadlineExceeded by unifying with some logic from another test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6803 Test Plan: run that unit test under valgrind, make check Reviewed By: siying Differential Revision: D21388470 Pulled By: pdillinger fbshipit-source-id: 0ce99d6d5eb8cd3195b17406892c8c5cff5fa5dd --- db/db_basic_test.cc | 3 +-- db/db_sst_test.cc | 26 +++++++------------------- db/db_test_util.cc | 9 +++++++++ db/db_test_util.h | 20 +++++++++++++++++++- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/db/db_basic_test.cc b/db/db_basic_test.cc index 979c2309d..3bfcff0f2 100644 --- a/db/db_basic_test.cc +++ b/db/db_basic_test.cc @@ -2533,9 +2533,8 @@ TEST_F(DBBasicTestMultiGetDeadline, MultiGetDeadlineExceeded) { std::shared_ptr fs( new DBBasicTestMultiGetDeadline::DeadlineFS(env_)); std::unique_ptr env(new CompositeEnvWrapper(env_, fs)); - env_->no_slowdown_ = true; - env_->time_elapse_only_sleep_.store(true); Options options = CurrentOptions(); + env_->SetTimeElapseOnlySleep(&options); std::shared_ptr cache = NewLRUCache(1048576); BlockBasedTableOptions table_options; diff --git a/db/db_sst_test.cc b/db/db_sst_test.cc index e0ecfb641..01a6880c4 100644 --- a/db/db_sst_test.cc +++ b/db/db_sst_test.cc @@ -356,37 +356,25 @@ TEST_F(DBSSTTest, RateLimitedDelete) { "InstrumentedCondVar::TimedWaitInternal", [&](void* arg) { // Turn timed wait into a simulated sleep uint64_t* abs_time_us = static_cast(arg); - int64_t cur_time = 0; - env_->GetCurrentTime(&cur_time); - if (*abs_time_us > static_cast(cur_time)) { - env_->addon_time_.fetch_add(*abs_time_us - - static_cast(cur_time)); + uint64_t cur_time = env_->NowMicros(); + if (*abs_time_us > cur_time) { + env_->addon_time_.fetch_add(*abs_time_us - cur_time); } // Randomly sleep shortly env_->addon_time_.fetch_add( static_cast(Random::GetTLSInstance()->Uniform(10))); - // Set wait until time to before current to force not to sleep. - int64_t real_cur_time = 0; - Env::Default()->GetCurrentTime(&real_cur_time); - *abs_time_us = static_cast(real_cur_time); + // Set wait until time to before (actual) current time to force not + // to sleep + *abs_time_us = Env::Default()->NowMicros(); }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); - env_->no_slowdown_ = true; - env_->time_elapse_only_sleep_ = true; Options options = CurrentOptions(); + env_->SetTimeElapseOnlySleep(&options); options.disable_auto_compactions = true; - // Need to disable stats dumping and persisting which also use - // RepeatableThread, one of whose member variables is of type - // InstrumentedCondVar. The callback for - // InstrumentedCondVar::TimedWaitInternal can be triggered by stats dumping - // and persisting threads and cause time_spent_deleting measurement to become - // incorrect. - options.stats_dump_period_sec = 0; - options.stats_persist_period_sec = 0; options.env = env_; int64_t rate_bytes_per_sec = 1024 * 10; // 10 Kbs / Sec diff --git a/db/db_test_util.cc b/db/db_test_util.cc index a8651b8da..c2c1d44ea 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -14,10 +14,19 @@ namespace ROCKSDB_NAMESPACE { +namespace { +int64_t MaybeCurrentTime(Env* env) { + int64_t time = 1337346000; // arbitrary fallback default + (void)env->GetCurrentTime(&time); + return time; +} +} // namespace + // Special Env used to delay background operations SpecialEnv::SpecialEnv(Env* base) : EnvWrapper(base), + maybe_starting_time_(MaybeCurrentTime(base)), rnd_(301), sleep_counter_(this), addon_time_(0), diff --git a/db/db_test_util.h b/db/db_test_util.h index 55d888f31..8b124b522 100644 --- a/db/db_test_util.h +++ b/db/db_test_util.h @@ -502,10 +502,14 @@ class SpecialEnv : public EnvWrapper { virtual Status GetCurrentTime(int64_t* unix_time) override { Status s; - if (!time_elapse_only_sleep_) { + if (time_elapse_only_sleep_) { + *unix_time = maybe_starting_time_; + } else { s = target()->GetCurrentTime(unix_time); } if (s.ok()) { + // FIXME: addon_time_ sometimes used to mean seconds (here) and + // sometimes microseconds *unix_time += addon_time_.load(); } return s; @@ -531,6 +535,20 @@ class SpecialEnv : public EnvWrapper { return target()->DeleteFile(fname); } + void SetTimeElapseOnlySleep(Options* options) { + time_elapse_only_sleep_ = true; + no_slowdown_ = true; + // Need to disable stats dumping and persisting which also use + // RepeatableThread, which uses InstrumentedCondVar::TimedWaitInternal. + // With time_elapse_only_sleep_, this can hang on some platforms. + // TODO: why? investigate/fix + options->stats_dump_period_sec = 0; + options->stats_persist_period_sec = 0; + } + + // Something to return when mocking current time + const int64_t maybe_starting_time_; + Random rnd_; port::Mutex rnd_mutex_; // Lock to pretect rnd_