Repository navigation
Conversation
Contributor
|
@philkunz thanks for the detailed write-up and the reproduction script! It helps us to have full motivation context and makes the PR made easier to follow. I’ve linked this to NODE-7893 and updated the title. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary of Changes
Enforce the getMore timeout contract in
GetMoreOperation, independent of whetherCSOT comes from the cursor, client, session default, or
withTransaction:maxTimeMSfrom getMore command options while retainingthe same client-side timeout context.
maxAwaitTimeMSas commandmaxTimeMSonly for a cursor that isboth tailable and awaitData. Change streams already supply both flags.
coverage for find/aggregation cursors inheriting both transaction and session
deadlines.
Notes for Reviewers
We could not open a NODE Jira ticket. GitHub issues are disabled for this
repository, so this PR includes the complete motivation and reproduction below.
Please associate a NODE ticket and add its scope to the title if required.
No new TODOs, dependencies, package metadata, build scripts, generated files, or
vendored specification fixtures are changed.
The CSOT specification for non-tailable cursors
requires retaining the remaining client timeout without appending
maxTimeMStogetMore. The correction preserves server-selection, checkout, round-trip and
transaction deadline enforcement; it does not disable CSOT or relax server
validation.
What is the motivation for this change?
A bounded transaction cannot read a normal cursor past its initial batch when
the deadline is inherited from the session. Even an ordinary 250-document read
then fails on the first getMore. This affects find and aggregation cursors and
occurs against MongoDB itself, without any application persistence abstraction.
For bug fixes
Current (incorrect) behavior:
withTransactionstores its deadline onClientSession.timeoutContext. A cursorwithout a local
timeoutMSsetsomitMaxTimeMSto false inAbstractCursor.cursorInit(). The operation inherits the session timeout and theconnection consequently appends the remaining budget to a normal getMore.
MongoDB rejects the command with code 2 (
BadValue):cannot set maxTimeMS on getMore command for a non-awaitData cursor.Expected behavior:
All documents are returned and the transaction commits within its client-side
deadline. A non-tailable getMore never carries
maxTimeMS; a tailable awaitDatacursor retains its explicit await timeout.
How to reproduce:
Use a disposable MongoDB 8.0.26 single-member replica set and set
MONGODB_URItoits connection string. Install the official driver with
pnpm add mongodb@7.6.0and run this driver-only CommonJS script:
The first getMore carries approximately 29998 ms and fails with the error above.
Removing the transaction's timeout makes the same read succeed. With this fix,
the read returns 250 documents and command monitoring shows a getMore without
maxTimeMS.Affected versions/environment:
16136bb34; the published 7.7.0 source retains the same omission condition.only for the disposable qualification fixture.
Tests
126 passing, 1 existing pending.
with a
withTransactiontimeout and a session-default timeout). These assertcomplete results, legal getMore commands, bounded initial commands and a
committed transactional write.
node_csot.test.ts: 39 passing, including stalled-operation andtransaction deadline behavior.
reproduction on the unchanged upstream-main build fails with the expected
server
BadValue.--skipLibCheck false, source/build/declarationextraction, native
tsd(39 type-test files), generated-declaration check,full source/test ESLint and changed-file Prettier all pass.
test/tsconfig.jsoncheck with--skipLibCheck falseexhaustsNode's default 4-GiB heap on both this branch and an independently installed
pristine upstream-main baseline. The driver's native public-type test workflow
passes; no typecheck or lint configuration is changed to mask this limitation.
All commands use pnpm. Scripts that nest npm were expanded into their documented
underlying commands rather than changing the repository's scripts. Integration
checks use a disposable MongoDB 8.0.26 fixture; no full topology/server-version
matrix is claimed.
Release Highlight
Left for the Node driver maintainers, as requested by the PR template.
Double check the following