Summary:
This PR adds automatic checks in the `PendingExpectedValue` class to make sure it's either committed or rolled back before being destructed.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/12244
Reviewed By: hx235
Differential Revision: D52853794
Pulled By: jowlyzhang
fbshipit-source-id: 1dcd7695f2c52b79695be0abe11e861047637dc4
Summary:
This PR fixes this type of stress test failure that could happen in either checkpoint or backup. Example failure messages are like this:
`Failure in a backup/restore operation with: Corruption: 0x00000000000001D5000000000000012B00000000000000FD exists in original db but not in restore`
`A checkpoint operation failed with: Corruption: 0x0000000000000365000000000000012B0000000000000067 exists in original db but not in checkpoint /...`
The internal task has an example test command to quickly reproduce this type of error.
The common symptom of these test failures are these expected keys do not exist in the original db either. The root cause is `TestCheckpoint` and `TestBackupRestore` both use the expected state as a proxy for the state of the original db when it comes to check a key's existence. 0758271d51/db_stress_tool/db_stress_test_base.cc (L1838)
This `ExpectedState::Exists` API returns true if a key has a pending write, such as a pending put. In usual case, this pending put should either soon materialize to an actual write when `PendingExpectedValue::Commit` is called to reflect a successful write to the DB, or test should be safely terminated if write to DB fails. All of which happens while a key is locked. So checkpoint and backup usually won't see the discrepancy between db and expected state caused by pending writes. However, the external file ingestion test currently has a path that will proceed the test after a failed ingestion caused by injected errors, leaving the pending put in the expected state. 0758271d51/db_stress_tool/no_batched_ops_stress.cc (L1577-L1589)
I think a proper and future proof fix for this is to explicitly rollback a pending state when a db write operation failed so that expected state do not diverge from db in the first place. I added a `PendingExpectedValue::Rollback` API so that we don't implicitly depend on thread termination to prevent test failures. Another place that could cause same divergence as external file ingestion is `PreloadDbAndReopenAsReadOnly`.
0758271d51/db_stress_tool/db_stress_test_base.cc (L616-L619)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/12227
Reviewed By: hx235
Differential Revision: D52705470
Pulled By: jowlyzhang
fbshipit-source-id: b21586b037caeeba29a2cff8c2fdc6f1d0bda9cf
Summary:
This patch remove the "stress" aspect from the WriteUnpreparedStressTest and leave it to be a unit test for some correctness testing w.r.t. snapshot functionality. I added some read-your-write verification to the transaction test in db_stress.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11424
Test Plan:
`./write_unprepared_transaction_test`
`./db_crashtest.py whitebox --txn`
`./db_crashtest.py blackbox --txn`
Reviewed By: hx235
Differential Revision: D45551521
Pulled By: jowlyzhang
fbshipit-source-id: 20c3d510eb4255b08ddd7b6c85bdb4945436f6e8
Summary:
**Context/Summary:**
https://github.com/facebook/rocksdb/pull/11424 made me realize there are a couple gaps in my `ExpectedValue` comments so I updated them, along with separating `ExpectedValue` into separate files so it's clearer that `ExpectedValue` can be used without updating `ExpectedState` (e.g, TestMultiGet() where we care about value base of expected value but not updating the ExpectedState).
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11456
Test Plan: CI
Reviewed By: jowlyzhang
Differential Revision: D45965070
Pulled By: hx235
fbshipit-source-id: dcee690c13b00a3119757ea9d43b646f9644e1a9
