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
This commit is contained in:
Peter Dillinger 2020-05-05 13:09:12 -07:00 committed by Facebook GitHub Bot
parent 91bc0130fa
commit 2f1700c8c5
4 changed files with 36 additions and 22 deletions

View File

@ -2533,9 +2533,8 @@ TEST_F(DBBasicTestMultiGetDeadline, MultiGetDeadlineExceeded) {
std::shared_ptr<DBBasicTestMultiGetDeadline::DeadlineFS> fs( std::shared_ptr<DBBasicTestMultiGetDeadline::DeadlineFS> fs(
new DBBasicTestMultiGetDeadline::DeadlineFS(env_)); new DBBasicTestMultiGetDeadline::DeadlineFS(env_));
std::unique_ptr<Env> env(new CompositeEnvWrapper(env_, fs)); std::unique_ptr<Env> env(new CompositeEnvWrapper(env_, fs));
env_->no_slowdown_ = true;
env_->time_elapse_only_sleep_.store(true);
Options options = CurrentOptions(); Options options = CurrentOptions();
env_->SetTimeElapseOnlySleep(&options);
std::shared_ptr<Cache> cache = NewLRUCache(1048576); std::shared_ptr<Cache> cache = NewLRUCache(1048576);
BlockBasedTableOptions table_options; BlockBasedTableOptions table_options;

View File

@ -356,37 +356,25 @@ TEST_F(DBSSTTest, RateLimitedDelete) {
"InstrumentedCondVar::TimedWaitInternal", [&](void* arg) { "InstrumentedCondVar::TimedWaitInternal", [&](void* arg) {
// Turn timed wait into a simulated sleep // Turn timed wait into a simulated sleep
uint64_t* abs_time_us = static_cast<uint64_t*>(arg); uint64_t* abs_time_us = static_cast<uint64_t*>(arg);
int64_t cur_time = 0; uint64_t cur_time = env_->NowMicros();
env_->GetCurrentTime(&cur_time); if (*abs_time_us > cur_time) {
if (*abs_time_us > static_cast<uint64_t>(cur_time)) { env_->addon_time_.fetch_add(*abs_time_us - cur_time);
env_->addon_time_.fetch_add(*abs_time_us -
static_cast<uint64_t>(cur_time));
} }
// Randomly sleep shortly // Randomly sleep shortly
env_->addon_time_.fetch_add( env_->addon_time_.fetch_add(
static_cast<uint64_t>(Random::GetTLSInstance()->Uniform(10))); static_cast<uint64_t>(Random::GetTLSInstance()->Uniform(10)));
// Set wait until time to before current to force not to sleep. // Set wait until time to before (actual) current time to force not
int64_t real_cur_time = 0; // to sleep
Env::Default()->GetCurrentTime(&real_cur_time); *abs_time_us = Env::Default()->NowMicros();
*abs_time_us = static_cast<uint64_t>(real_cur_time);
}); });
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
env_->no_slowdown_ = true;
env_->time_elapse_only_sleep_ = true;
Options options = CurrentOptions(); Options options = CurrentOptions();
env_->SetTimeElapseOnlySleep(&options);
options.disable_auto_compactions = true; 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_; options.env = env_;
int64_t rate_bytes_per_sec = 1024 * 10; // 10 Kbs / Sec int64_t rate_bytes_per_sec = 1024 * 10; // 10 Kbs / Sec

View File

@ -14,10 +14,19 @@
namespace ROCKSDB_NAMESPACE { 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 // Special Env used to delay background operations
SpecialEnv::SpecialEnv(Env* base) SpecialEnv::SpecialEnv(Env* base)
: EnvWrapper(base), : EnvWrapper(base),
maybe_starting_time_(MaybeCurrentTime(base)),
rnd_(301), rnd_(301),
sleep_counter_(this), sleep_counter_(this),
addon_time_(0), addon_time_(0),

View File

@ -502,10 +502,14 @@ class SpecialEnv : public EnvWrapper {
virtual Status GetCurrentTime(int64_t* unix_time) override { virtual Status GetCurrentTime(int64_t* unix_time) override {
Status s; Status s;
if (!time_elapse_only_sleep_) { if (time_elapse_only_sleep_) {
*unix_time = maybe_starting_time_;
} else {
s = target()->GetCurrentTime(unix_time); s = target()->GetCurrentTime(unix_time);
} }
if (s.ok()) { if (s.ok()) {
// FIXME: addon_time_ sometimes used to mean seconds (here) and
// sometimes microseconds
*unix_time += addon_time_.load(); *unix_time += addon_time_.load();
} }
return s; return s;
@ -531,6 +535,20 @@ class SpecialEnv : public EnvWrapper {
return target()->DeleteFile(fname); 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_; Random rnd_;
port::Mutex rnd_mutex_; // Lock to pretect rnd_ port::Mutex rnd_mutex_; // Lock to pretect rnd_