-
Type:
Bug
-
Resolution: Unresolved
-
Priority:
Major - P3
-
None
-
Affects Version/s: None
-
Component/s: None
-
None
-
Query Execution
-
ALL
-
None
-
None
-
None
-
None
-
None
-
None
-
None
TLDR
there is a low chance if a response from a getMore comes back while detached and we try to refresh the opCtx, other fields might not be refreshed. like if an aggregate and getMore commands have different comments. or as the maxTimeMS is ticking down between getMore the acounting might not be right
Issue
When AsyncResultsMerger::_handleBatchResponse() schedules a delayed retry of a getMore, it
captures a copy of the original RemoteCommandRequest (request = cbData.request). On
re-dispatch it only reassigns the raw OperationContext pointer:
async_results_merger.cpp:1436-1437
auto refreshedRequest = request; refreshedRequest.opCtx = self->_opCtx;
It does not recompute the opCtx-derived fields of the request. RemoteCommandRequest bakes
timeout, deadline and timeoutCode in from the opCtx at construction time (see
RemoteCommandRequest::_updateTimeoutFromOpCtxDeadline, which copies
opCtx->getRemainingMaxTimeMillis(), opCtx->getDeadline() and opCtx->getTimeoutError()).
The same applies to any opCtx-driven cmdObj decoration (comment, API parameters, tracing) that was
serialized when the request was originally built.
When this is reachable (realistic conditions)
Each mongos getMore runs under a new OperationContext: between getMores the router cursor is
checked in and the ARM detached (cluster_cursor_manager.cpp checkInCursor ->
detachFromOperationContext), and on the next getMore it is checked out and reattached to the new
opCtx (checkOutCursor -> reattachToOperationContext). So "reattach to a different opCtx" is
the normal cursor flow.
A mongos getMore's opCtx carries a deadline (making the shard request's deadline non-default)
in one real flow: *non-tailable find/aggregate cursors whose originating command carried a
maxTimeMS* – either client-specified, or injected fleet-wide by the defaultMaxTimeMS read
default cluster parameter. The leftover budget is stashed on the router cursor
(cluster_cursor_manager.cpp, registration) and re-applied to each getMore's opCtx at
cluster_find.cpp (setUpOperationContextStateForGetMore, the
getLeftoverMaxTimeMicros() branch), then baked into the ARM's shard request at
async_results_merger.cpp _scheduleGetMoresForRemotes. No hand-crafted values are required –
a cluster with defaultMaxTimeMS set produces per-getMore deadlines automatically.
Not affected:
- Tailable/awaitData (change stream) getMores – their maxTimeMS is applied as the awaitData
timeout (waitForInsertsDeadline / a per-request maxTimeMS field), never as an opCtx
deadline. - maxTimeMS on the getMore command itself – the generic mongos path
(strategy.cpp) explicitly skips opGetMore when setting the deadline. - Multi-document transactions – no additional mongos opCtx deadline.
Consequence
If the retry is scheduled on the detached (_executor->sleepFor(...)) path – i.e. a getMore
error response arrives while the ARM is detached – and the ARM is then reattached to the next
getMore's opCtx before the backoff fires, the retry re-dispatches carrying the previous opCtx's
deadline/timeoutCode. Both opCtxs have deadlines; the stale one is earlier (its budget
started earlier and does not account for the wall-clock elapsed – including client think-time –
between the two getMores). The re-dispatched shard getMore can therefore hit MaxTimeMSExpired
prematurely, or immediately if the stale deadline has already elapsed. Severity is a spurious
timeout on the retry, not data corruption or a crash.
The attached/SubBaton path is not affected: detach routes the retry callback through the
!self->_opCtx early-return guard, and a subsequent reattach rebuilds the request fresh via
_scheduleGetMoresForRemotes(), so only the sleepFor path re-dispatches a stale captured
request.
Suggested fix
Rebuild the RemoteCommandRequest against the currently-attached OperationContext before
re-dispatch (the way _scheduleGetMoresForRemotes() constructs it), or otherwise recompute the
opCtx-derived timeout/deadline/timeoutCode (and opCtx-driven cmdObj decoration), rather
than copying the original request and patching only the opCtx pointer.
Notes
- Surfaced by the retry re-dispatch guard in
SERVER-128736; the copy-and-patch predates and is
unrelated to that fix's use-after-free correction. Filed separately per PR review. - A deterministic unit-test repro is on branch mjrb/arm-stale-deadline-repro (test
AsyncResultsMergerTest.RetryAfterReattachToDifferentOpCtxUsesStaleTimeout in
async_results_merger_test.cpp); it passes against master today, asserting the re-dispatched
request carries opCtx A's earlier deadline rather than opCtx B's. Full source is in a comment
below. - Affected file: src/mongo/s/query/exec/async_results_merger.cpp (retry re-dispatch in
_handleBatchResponse).
- related to
-
SERVER-128736 Rate limited getMore retry re-dispatched after the OperationContext is destroyed crashes
-
- Closed
-