Skip to content

Commit

Permalink
Fix flaky test asserting concurrent write thread's waiting time (#13241)
Browse files Browse the repository at this point in the history
Summary:
This test assertion was added in #13219. It checks the concurrent write thread's wait time is not longer than the file ingestion thread's write blocking time since the former entered the write thread after the blocking already started in the test. This test runs into flakiness like this:
```db/external_sst_file_basic_test.cc:300: Failure
Expected: (perf_context.file_ingestion_blocking_live_writes_nanos) > (write_thread_perf_context->write_thread_wait_nanos), actual: 166210 vs 279681
```
 In reality the write thread is yielding starting with a 1 micro period and then every 100 micros: https://github.com/facebook/rocksdb/blob/54b614de5bd3e26d332b85557d44bde86b2a2e87/db/write_thread.cc#L68-L70

So this 113 micros errors is within this margin
This fix the test with just removing this assertion. The other assertion `ASSERT_GT(write_thread_perf_context->write_thread_wait_nanos, 0)` should be sufficient for the test's purpose.

Pull Request resolved: #13241

Reviewed By: hx235

Differential Revision: D67526804

Pulled By: jowlyzhang

fbshipit-source-id: 23ee9771247e4c13444054a1e86ad9293902cb56
  • Loading branch information
jowlyzhang authored and facebook-github-bot committed Dec 20, 2024
1 parent 54b614d commit 19e4aba
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions db/external_sst_file_basic_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,6 @@ TEST_F(ExternalSSTFileBasicTest, Basic) {
}

write_thread.join();
ASSERT_GT(perf_context.file_ingestion_blocking_live_writes_nanos,
write_thread_perf_context->write_thread_wait_nanos);
SyncPoint::GetInstance()->DisableProcessing();

// Re-ingest the file just to check the perf context not enabled at and below
Expand Down

0 comments on commit 19e4aba

Please sign in to comment.