ExportXMLWordPrintableJSON

    • Type: Improvement
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      Follow-up to SERVER-134598 (Refactor ARM to clarify retry behavior).

      Background

      SERVER-133148 fixed a bug where AsyncResultsMerger (ARM) retried a getMore from inside the network/backoff-timer callback using an OperationContext that had been reattached but whose per-operation state (IncrementalFeatureRollout egress metadata, maxTimeMS) was not yet restored. The fix made dispatch happen only in _scheduleGetMores(), reached only via nextEvent() from the operation thread after restoration completes.

      However, the safety of that path still rests on an implicit contract: "only the operation thread calls scheduleGetMores / nextEvent, and only after the opCtx is fully restored." The only structural check is invariant(_opCtx), i.e. "_opCtx != nullptr. As SERVER-133148 showed, {_opCtx} != null is a necessary but not sufficient proxy for "safe to dispatch from this opCtx" — the opCtx can be reattached before all relevant state is set up (a bit later down in the getMore command execution path).

      What SERVER-134598 did

      SERVER-134598 introduced a single named ARM-level predicate, _isReadyToDispatch(lk), and routed the Undispatched -> Dispatched dispatch transition through it (with a tassert) instead of an inline invariant(_opCtx). Today that helper still just returns {_opCtx} != nullptr — behavior-preserving — but it is now a single chokepoint with a TODO SERVER-XXXXX marker pointing here.

      Work for this ticket

      Strengthen what "ready to dispatch" means so it no longer relies on the {_opCtx} != null proxy:

      • Track opCtx-restoration readiness explicitly. For example, reattachToOperationContext() could leave the ARM in an OpCtxAttached state that is not yet ReadyToDispatch, with a separate signal (set at the precise point in the getMore command execution path where IFRContext egress metadata is cached and maxTimeMS is restored) opening the gate.
      • Have _isReadyToDispatch(lk) return true only when that explicit readiness holds, not merely when {_opCtx} is non-null.
      • Add tests that a reattach-before-restoration window cannot dispatch a getMore (deterministic, mirroring the SERVER-133148 repro shape).

      Scope notes

      • This is a behavior-adjacent change on the getMore command runner side (the restoration-completion point), not on RemoteCursorData — a different axis from the per-remote state machine introduced in SERVER-134598.
      • Must preserve the "retry defers until reattach" contract currently enforced by the _scheduleGetMores opCtx invariant.

      Related: SERVER-133148, SERVER-134598.

            Assignee:
            Charlie Swanson
            Reporter:
            Charlie Swanson
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: