-
Notifications
You must be signed in to change notification settings - Fork 66
PSMDB-1078: fix failing tests in storage_inmemory_kv_engine_test #1592
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 #1592
Conversation
... otherwise all tests which rely on timestamps will fail because the table logging will be enabled - see WiredTigerUtil::useTableLogging.
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 KVEngine test harness to handle ephemeral storage engines (e.g., in-memory WiredTiger) differently when testing last stable recovery timestamps. The changes address a behavioral difference where ephemeral engines set the last stable timestamp immediately upon calling setStableTimestamp(), while persistent engines only update it after a checkpoint.
Key changes:
- Added conditional logic in test assertions to handle ephemeral vs. persistent storage engine behavior
- Refactored in-memory test harness to properly initialize StorageEngine via StorageEngineImpl
- Simplified test setup by replacing global settings calls with explicit local variables
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/mongo/db/storage/kv/kv_engine_test_harness.cpp | Updated two tests to conditionally assert different behavior for ephemeral engines when checking last stable recovery timestamp |
| src/mongo/db/storage/inmemory/inmemory_kv_engine_test.cpp | Refactored test harness helper to initialize engine via StorageEngineImpl and use explicit replica set configuration instead of global settings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…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.
0ce8049 to
688c452
Compare
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.