-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5736: Expose atClusterTime parameter in snapshot sessions #1874
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
base: main
Are you sure you want to change the base?
Conversation
|
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
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
Exposes snapshot sessions’ atClusterTime to consumers via a new public SnapshotTime option and a GetSnapshotTime extension method, and updates unified/spec tests to validate the behavior.
Changes:
- Added
SnapshotTimetoClientSessionOptionsand propagated it throughCoreSessionOptions/CoreSession. - Introduced
GetSnapshotTimeextension method for retrieving the snapshot cluster time from a session. - Extended unified test runner and session spec tests to cover snapshot-time scenarios.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedTestOperationFactory.cs | Registers a new unified operation for retrieving snapshot time. |
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedGetSnapshotOperation.cs | Adds a unified test operation/builder to read snapshot time and save it as an entity. |
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedEntityMap.cs | Adds parsing of snapshotTime in sessionOptions for unified tests. |
| tests/MongoDB.Driver.Tests/Specifications/sessions/SessionsProseTests.cs | Adds prose tests for SnapshotTime validation and error behavior. |
| src/MongoDB.Driver/MongoClient.cs | Adds validation preventing SnapshotTime usage when Snapshot=false. |
| src/MongoDB.Driver/IClientSessionExtensions.cs | Adds public GetSnapshotTime extension method; renames internal extensions type. |
| src/MongoDB.Driver/Core/Bindings/CoreSessionOptions.cs | Adds SnapshotTime to core options and exposes it via a property. |
| src/MongoDB.Driver/Core/Bindings/CoreSession.cs | Plumbs SnapshotTime from options into the core session. |
| src/MongoDB.Driver/ClientSessionOptions.cs | Adds public SnapshotTime and maps it into CoreSessionOptions. |
| src/MongoDB.Driver/ClientSessionHandle.cs | Exposes SnapshotTime from the core session. |
| specifications/sessions/tests/snapshot-sessions.yml | Adds new spec test cases exercising snapshotTime; includes an unclear comment. |
| specifications/sessions/tests/snapshot-sessions.json | JSON form of the added spec test cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedGetSnapshotTimeOperation.cs
Show resolved
Hide resolved
tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedGetSnapshotOperation.cs
Outdated
Show resolved
Hide resolved
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
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static BsonTimestamp GetSnapshotTime(this IClientSession session) | ||
| { | ||
| var clientSessionHandle = (ClientSessionHandle)session; | ||
| return clientSessionHandle.WrappedCoreSession.IsSnapshot ? | ||
| clientSessionHandle.SnapshotTime | ||
| : throw new InvalidOperationException("Cannot retrieve snapshot time from a non-snapshot session."); |
Copilot
AI
Feb 10, 2026
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.
This public extension method hard-casts IClientSession to ClientSessionHandle, which can surface an InvalidCastException for callers with a different IClientSession implementation (or proxies/mocks). Since the PR description indicates this should be an extension on IClientSessionHandle, prefer changing the signature to GetSnapshotTime(this IClientSessionHandle session) and avoid the concrete cast; alternatively use pattern matching and throw a more intentional exception type/message when the session isn't a driver session handle.
| public static BsonTimestamp GetSnapshotTime(this IClientSession session) | |
| { | |
| var clientSessionHandle = (ClientSessionHandle)session; | |
| return clientSessionHandle.WrappedCoreSession.IsSnapshot ? | |
| clientSessionHandle.SnapshotTime | |
| : throw new InvalidOperationException("Cannot retrieve snapshot time from a non-snapshot session."); | |
| /// <exception cref="NotSupportedException">Thrown when the session is not a <see cref="ClientSessionHandle"/>.</exception> | |
| public static BsonTimestamp GetSnapshotTime(this IClientSession session) | |
| { | |
| if (session is ClientSessionHandle clientSessionHandle) | |
| { | |
| return clientSessionHandle.WrappedCoreSession.IsSnapshot | |
| ? clientSessionHandle.SnapshotTime | |
| : throw new InvalidOperationException("Cannot retrieve snapshot time from a non-snapshot session."); | |
| } | |
| throw new NotSupportedException("GetSnapshotTime is only supported for sessions of type ClientSessionHandle."); |
This PR exposes
atClusterTimeto developers. In the context of this PRatClusterTimehas been renamed toSnapshotTimein our public API, to be consistent with our current public terminology.The changes to the public API are:
SnapshotTimetoClientSessionOptions.SnapshotTimetoCoreSessionOptionsand added relative constructor.GetSnapshotTimeonIClientSessionHandle. This has been added as an extension method to avoid having breaking changes.Example use: