-
Notifications
You must be signed in to change notification settings - Fork 66
PSMDB-1078: fix failing tests in storage_inmemory_kv_engine_test (v8.0) #1593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PSMDB-1078: fix failing tests in storage_inmemory_kv_engine_test (v8.0) #1593
Conversation
... otherwise all tests which rely on timestamps will fail because the table logging will be enabled - see WiredTigerUtil::useTableLogging.
…test If the StorageEngineImpl is not created some of the tests fail due to missing the _mdb_catalog which is created in the StorageEngineImpl constructor.
Update RollingBackToLastStable and CommitBehindStable test cases to handle ephemeral storage engine behavior where the last stable recovery timestamp is set immediately when setStableTimestamp is called, unlike non ephemeral storage engine that require a checkpoint to advance the timestamp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the in-memory storage engine test harness to properly wrap the KVEngine in a StorageEngineImpl and adapts related tests to handle ephemeral storage engine behavior differences.
- Wraps WiredTiger in-memory KVEngine in StorageEngineImpl for proper test setup
- Adds conditional checks for ephemeral engines in timestamp-related tests
- Implements retry logic for handling ObjectIsBusy errors during ident drops
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mongo/db/storage/kv/kv_engine_test_harness.cpp | Adds retry loop for ObjectIsBusy errors and conditional assertions for ephemeral engine timestamp behavior |
| src/mongo/db/storage/inmemory/inmemory_kv_engine_test.cpp | Refactors test harness to wrap KVEngine in StorageEngineImpl with replica set configuration |
| src/mongo/db/storage/inmemory/SConscript | Adds storage_engine_impl dependency for the test build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…y from dropIdent The TemporaryRecordStoreSimple test can fail when dropIdent() returns ErrorCodes::ObjectIsBusy, which occurs when the ident is temporarily in use. This is a transient condition that should be retried. This change adds retry logic to the test, retrying dropIdent() until it succeeds when ObjectIsBusy is returned. This matches the pattern used in production code (e.g. WiredTigerKVEngine::dropIdentForImport).
ae79b7d to
875d657
Compare
igorsol
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ktrushin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.