Skip to content

Commit

Permalink
db_stress begin tracking expected state after verification (facebook#…
Browse files Browse the repository at this point in the history
…9470)

Summary:
Previously we enabled tracking expected state changes during
`FinishInitDb()`, as soon as the DB was opened. This meant tracing was
enabled during `VerifyDb()`. This cost extra CPU by requiring
`DBImpl::trace_mutex_` to be acquired on each read operation. It was
unnecessary since we know there are no expected state changes during the
`VerifyDb()` phase. So, this PR delays tracking expected state changes
until after the `VerifyDb()` phase has completed.

Pull Request resolved: facebook#9470

Test Plan:
Measured this PR reduced `VerifyDb()` 76% (387 -> 92 seconds) with
`-disable_wal=1` (i.e., expected state tracking enabled).

- benchmark command: `./db_stress -max_key=100000000 -ops_per_thread=1 -destroy_db_initially=1 -expected_values_dir=/dev/shm/dbstress_expected/ -db=/dev/shm/dbstress/ --clear_column_family_one_in=0 --disable_wal=1 --reopen=0`
- without this PR, `VerifyDb()` takes 387 seconds:

```
2022/01/30-21:43:04  Initializing worker threads
Crash-recovery verification passed :)
2022/01/30-21:49:31  Starting database operations
```

- with this PR, `VerifyDb()` takes 92 seconds

```
2022/01/30-21:59:06  Initializing worker threads
Crash-recovery verification passed :)
2022/01/30-22:00:38  Starting database operations
```

Reviewed By: riversand963

Differential Revision: D33884596

Pulled By: ajkr

fbshipit-source-id: 5f259de8087de5b0531f088e11297f37ed2f7685
  • Loading branch information
ajkr authored and facebook-github-bot committed Jan 31, 2022
1 parent 8dbd0bd commit c7ce03d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
4 changes: 4 additions & 0 deletions db_stress_tool/db_stress_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ bool RunStressTest(StressTest* stress) {
}
}

// This is after the verification step to avoid making all those `Get()`s
// and `MultiGet()`s contend on the DB-wide trace mutex.
stress->TrackExpectedState(&shared);

now = clock->NowMicros();
fprintf(stdout, "%s Starting database operations\n",
clock->TimeToString(now / 1000000).c_str());
Expand Down
20 changes: 11 additions & 9 deletions db_stress_tool/db_stress_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,6 @@ void StressTest::FinishInitDb(SharedState* shared) {
}
}

if ((FLAGS_sync_fault_injection || FLAGS_disable_wal) && IsStateTracked()) {
Status s = shared->SaveAtAndAfter(db_);
if (!s.ok()) {
fprintf(stderr, "Error enabling history tracing: %s\n",
s.ToString().c_str());
exit(1);
}
}

if (FLAGS_enable_compaction_filter) {
auto* compaction_filter_factory =
reinterpret_cast<DbStressCompactionFilterFactory*>(
Expand All @@ -333,6 +324,17 @@ void StressTest::FinishInitDb(SharedState* shared) {
}
}

void StressTest::TrackExpectedState(SharedState* shared) {
if ((FLAGS_sync_fault_injection || FLAGS_disable_wal) && IsStateTracked()) {
Status s = shared->SaveAtAndAfter(db_);
if (!s.ok()) {
fprintf(stderr, "Error enabling history tracing: %s\n",
s.ToString().c_str());
exit(1);
}
}
}

bool StressTest::VerifySecondaries() {
#ifndef ROCKSDB_LITE
if (FLAGS_test_secondary) {
Expand Down
2 changes: 2 additions & 0 deletions db_stress_tool/db_stress_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class StressTest {
// dependency with `SharedState`.
virtual void FinishInitDb(SharedState*);

void TrackExpectedState(SharedState* shared);

// Return false if verification fails.
bool VerifySecondaries();

Expand Down

0 comments on commit c7ce03d

Please sign in to comment.