Skip to content

Commit

Permalink
Revert "Bugfix/fix manual flush blocking bug (facebook#9893)" (facebo…
Browse files Browse the repository at this point in the history
…ok#9992)

Summary:
This reverts commit 6d2577e.

A proposal for resolving our current internal test failures. A fix is
being planned.

More context can be found: facebook#9893 (comment)
TSAN error: facebook#9893 (comment)

Pull Request resolved: facebook#9992

Reviewed By: siying

Differential Revision: D36379154

Pulled By: riversand963

fbshipit-source-id: b240261e766eff099513799cf5631832093f4cd2
  • Loading branch information
riversand963 authored and facebook-github-bot committed May 13, 2022
1 parent f6d9730 commit b58a1a0
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 49 deletions.
6 changes: 2 additions & 4 deletions db/db_impl/db_impl_compaction_flush.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1705,11 +1705,9 @@ Status DBImpl::Flush(const FlushOptions& flush_options,
Status s;
if (immutable_db_options_.atomic_flush) {
s = AtomicFlushMemTables({cfh->cfd()}, flush_options,
FlushReason::kManualFlush,
write_controller().IsStopped());
FlushReason::kManualFlush);
} else {
s = FlushMemTable(cfh->cfd(), flush_options, FlushReason::kManualFlush,
write_controller().IsStopped());
s = FlushMemTable(cfh->cfd(), flush_options, FlushReason::kManualFlush);
}

ROCKS_LOG_INFO(immutable_db_options_.info_log,
Expand Down
45 changes: 0 additions & 45 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7036,51 +7036,6 @@ TEST_F(DBTest, MemoryUsageWithMaxWriteBufferSizeToMaintain) {
}
}

TEST_F(DBTest, ManualFlushWithStoppedWritesTest) {
Options options = CurrentOptions();

// Setting a small write buffer size. This will block writes
// rather quickly until a flush is made.
constexpr unsigned int memtableSize = 1000;
options.db_write_buffer_size = memtableSize;
options.max_background_flushes = 1;
Reopen(options);

std::atomic<bool> done(false);

// Will suppress future flushes.
// This simulates a situation where the writes will be stopped for any reason.
ASSERT_OK(dbfull()->PauseBackgroundWork());

port::Thread backgroundWriter([&]() {
Random rnd(301);
// These writes won't finish.
ASSERT_OK(Put("k1", rnd.RandomString(memtableSize / 2)));
ASSERT_OK(Put("k2", rnd.RandomString(memtableSize / 2)));
ASSERT_OK(Put("k3", rnd.RandomString(memtableSize / 2)));
ASSERT_OK(Put("k4", rnd.RandomString(memtableSize / 2)));
done.store(true);
});

env_->SleepForMicroseconds(1000000 / 2);
// make sure thread is stuck waiting for flush.
ASSERT_FALSE(done.load());

FlushOptions flushOptions;
flushOptions.wait = false;
flushOptions.allow_write_stall = true;

// This is the main goal of the test. This manual flush should not deadlock
// as we use wait=false, and even allow_write_stall=true for extra safety.
ASSERT_OK(dbfull()->Flush(flushOptions));

// This will re-allow background flushes.
ASSERT_OK(dbfull()->ContinueBackgroundWork());

backgroundWriter.join();
ASSERT_TRUE(done.load());
}

#endif

} // namespace ROCKSDB_NAMESPACE
Expand Down

0 comments on commit b58a1a0

Please sign in to comment.