Fix test race conditions with OnFlushCompleted()

[ajkr]

We often see flaky tests due to DB::Flush() or DBImpl::TEST_WaitForFlushMemTable() not waiting until event listeners complete. For example, #9084, #9400, #9528, plus two new ones this week: "EventListenerTest.OnSingleDBFlushTest" and "DBFlushTest.FireOnFlushCompletedAfterCommittedResult". I ran a make check with the below race condition-coercing patch and fixed issues it found besides old BlobDB.

diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc
index 0e1864788..aaba68c4a 100644
--- a/db/db_impl/db_impl_compaction_flush.cc
+++ b/db/db_impl/db_impl_compaction_flush.cc
@@ -861,6 +861,8 @@ void DBImpl::NotifyOnFlushCompleted(
        mutable_cf_options.level0_stop_writes_trigger);
   // release lock while notifying events
   mutex_.Unlock();
+  bg_cv_.SignalAll();
+  sleep(1);
   {
     for (auto& info : *flush_jobs_info) {
       info->triggered_writes_slowdown = triggered_writes_slowdown;

The reason I did not fix old BlobDB issues is because it appears to have a fundamental (non-test) issue. In particular, it uses an EventListener to keep track of the files. OnFlushCompleted() could be delayed until even after a compaction involving that flushed file completes, causing the compaction to unexpectedly delete an untracked file.

Test Plan: make check including the race condition coercing patch

[facebook-github-bot]

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[hx235]

LGTM and good PR summary!

[ajkr]

Thanks for the review!