Force a primary flush before secondary verification when WAL is disabled or manual_wal_flush is set (#13338)

Summary:
https://github.com/facebook/rocksdb/issues/13281 added support for verifying secondaries in the crash tests. We are trying to check that the values returned by the secondary in `Get` requests fall within an expected range of values. We do reads from the shared expected state before and after we read from the secondary.

There are some rare verification failures where `VerifyValueRange` fails with `Unexpected value found outside of the value base range`.

I have some ideas on what the root cause could be. The secondary can read the WAL, MANIFEST, and SST files, but in some scenarios some of these pieces may not be present.

I noticed that the failures had `manual_wal_flush_one_in=1000`, which means that `options.manual_wal_flush` is set to `true`. With this setting, RocksDB has its own internal buffers that need to be manually flushed for the WAL to be persisted.

Although the test failures I looked at did not disable the WAL, I realized that, when the WAL is disabled, we should flush the primary's memtables, since the secondary needs to be able to find SST files to fully catch up.

Injected faults further complicate matters, so I have a check to skip secondary verification whenever the WAL or memtable flushes fail due to fault injection.

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

Test Plan:
Locally:
```
python3 tools/db_crashtest.py --simple blackbox --test_secondary=1
python3 tools/db_crashtest.py --simple whitebox --test_secondary=1
python3 tools/db_crashtest.py --simple blackbox --test_secondary=1 --disable_wal=1
python3 tools/db_crashtest.py --simple blackbox --test_secondary=1 --disable_wal=0 --manual_wal_flush_one_in=1000
```

I will monitor the recurring crash tests after this gets merged.

Reviewed By: anand1976

Differential Revision: D68741287

Pulled By: archang19

fbshipit-source-id: 86f474c41a68b7b06f2ed80a851c6cb52a47ebe7
This commit is contained in:
Andrew Chang
2025-01-28 10:56:13 -08:00
committed by Facebook GitHub Bot
parent 880f85a162
commit b8d915c7fa
+37
View File
@@ -170,6 +170,43 @@ class NonBatchedOpsStressTest : public StressTest {
shared->Get(static_cast<int>(cf), i));
}
if (FLAGS_disable_wal) {
// The secondary relies on the WAL to be able to catch up with the
// primary's memtable changes. If there is no WAL, before
// verification we should make sure the changes are reflected in the
// SST files
Status memtable_flush_status =
db_->Flush(FlushOptions(), column_families_[cf]);
if (!memtable_flush_status.ok()) {
if (IsErrorInjectedAndRetryable(memtable_flush_status)) {
fprintf(stdout,
"Skipping secondary verification because error was "
"injected into memtable flush\n");
continue;
}
VerificationAbort(shared,
"Failed to flush primary's memtables before "
"secondary verification");
}
} else if (FLAGS_manual_wal_flush_one_in > 0) {
// RocksDB maintains internal buffers of WAL data when
// manual_wal_flush is used. The secondary can read the WAL to catch
// up with the primary's memtable changes, but these changes need to
// be flushed first.
Status flush_wal_status = db_->FlushWAL(/*sync=*/true);
if (!flush_wal_status.ok()) {
if (IsErrorInjectedAndRetryable(flush_wal_status)) {
fprintf(stdout,
"Skipping secondary verification because error was "
"injected into WAL flush\n");
continue;
}
VerificationAbort(shared,
"Failed to flush primary's WAL before "
"secondary verification");
}
}
Status s = secondary_db_->TryCatchUpWithPrimary();
if (!s.ok()) {
VerificationAbort(shared,