Deflake ChargeFileMetadataTestWithParam/ChargeFileMetadataTestWithParam.Basic/0 (#10481)

Summary:
**Context/summary:**
`ChargeFileMetadataTestWithParam/ChargeFileMetadataTestWithParam.Basic/0 ` relies on `DBImpl::BackgroundCallCompaction:PurgedObsoleteFiles` happens before verifying `EXPECT_EQ(file_metadata_charge_only_cache->GetCacheCharge(),
              1 * CacheReservationManagerImpl<
                      CacheEntryRole::kFileMetadata>::GetDummyEntrySize());` or `EXPECT_EQ(file_metadata_charge_only_cache->GetCacheCharge(), 0);` to ensure appropriate cache reservation release is done before checking.

However, this might not be the case under some timing delay and spurious wake-up as coerced below.

```
 diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc
index 4378f3212..3e4f60853 100644
 --- a/db/db_impl/db_impl_compaction_flush.cc
+++ b/db/db_impl/db_impl_compaction_flush.cc
@@ -2989,6 +2989,8 @@ void DBImpl::BackgroundCallCompaction(PrepickedCompaction* prepicked_compaction,
     if (job_context.HaveSomethingToClean() ||
         job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) {
       mutex_.Unlock();
+      bg_cv_.SignalAll();
+      usleep(1000);
         // Have to flush the info logs before bg_compaction_scheduled_--
        // because if bg_flush_scheduled_ becomes 0 and the lock is
        // released, the deconstructor of DB can kick in and destroy all the
        // states of DB so info_log might not be available after that point.
        // It also applies to access other states that DB owns.
        log_buffer.FlushBufferToLog();
        if (job_context.HaveSomethingToDelete()) {
          PurgeObsoleteFiles(job_context);
          TEST_SYNC_POINT("DBImpl::BackgroundCallCompaction:PurgedObsoleteFiles");
        }
```

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10481

Test Plan:
The test of interest failed often at the above coercion:

After fix, the test of interest passed at the above coercion:

Reviewed By: jay-zhuang

Differential Revision: D38438256

Pulled By: hx235

fbshipit-source-id: de80ecdb250174f00e7c2f5e4d952695ed56f51e
main
Hui Xiao 2 years ago committed by Facebook GitHub Bot
parent 9d77bf8f7b
commit 56dbcb4f72
  1. 9
      db/version_set_test.cc

@ -3526,9 +3526,15 @@ TEST_P(ChargeFileMetadataTestWithParam, Basic) {
// file metadata above. This results in 1 *
// CacheReservationManagerImpl<CacheEntryRole::kFileMetadata>::GetDummyEntrySize()
// cache reservation for file metadata.
SyncPoint::GetInstance()->LoadDependency(
{{"DBImpl::BackgroundCallCompaction:PurgedObsoleteFiles",
"ChargeFileMetadataTestWithParam::"
"PreVerifyingCacheReservationRelease"}});
SyncPoint::GetInstance()->EnableProcessing();
ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
ASSERT_EQ("0,1", FilesPerLevel(0));
TEST_SYNC_POINT(
"ChargeFileMetadataTestWithParam::PreVerifyingCacheReservationRelease");
if (charge_file_metadata == CacheEntryRoleOptions::Decision::kEnabled) {
EXPECT_EQ(file_metadata_charge_only_cache->GetCacheCharge(),
1 * CacheReservationManagerImpl<
@ -3536,6 +3542,7 @@ TEST_P(ChargeFileMetadataTestWithParam, Basic) {
} else {
EXPECT_EQ(file_metadata_charge_only_cache->GetCacheCharge(), 0);
}
SyncPoint::GetInstance()->DisableProcessing();
// Destroying the db will delete the remaining 1 new file metadata
// This results in no cache reservation for file metadata.

Loading…
Cancel
Save