ExportXMLWordPrintableJSON

    • Type: Bug
    • Resolution: Won't Do
    • Priority: Minor - P4
    • None
    • Affects Version/s: None
    • Component/s: None
    • DB Integration & Observability
    • ALL
    • 200
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      Origin

      Adjacent-risk hunt from the BF-46466 postmortem — SUSPECTED, NOT YET CONFIRMED. Confidence: medium. Related: [BF-46466], SERVER-133148.

      Summary

      CallbackCanceledDoesNotClobberNonRetargetingError (line 1455) and CallbackCanceledDoesNotClobberRetargetingError (line 1503) in src/mongo/s/query/exec/establish_cursors_test.cpp arm the AsyncRequestsSender retry via _subBaton->waitUntil(_subExecutor->now() + delay, token) (src/mongo/s/async_requests_sender.cpp:517-526) with setBackoffDelayForTesting = 1 ms (1459/1507) and no armRetryScheduledForTesting-style barrier.

      Each test's only synchronization is the ordering of its two onCommand callbacks followed by a comment and future.default_timed_get(). Determinism relies entirely on the mock clock not advancing during the test window, so the 1 ms waitUntil timer cannot fire before the second onCommand swallows the terminal error and stopRetrying() cancels the pending timer. No barrier guards against a future clock advance, and the rest of the file (1456-1620) is already on the fix pattern (failpoint barriers + worker self-drive) — these two tests are the stragglers.

      Scope

      The two CallbackCanceled* tests in establish_cursors_test.cpp:1455-1543. The aggravator is process-global and applies to the whole binary: the precise ClockSourceMock is a ClockSourceMockImpl delegate (clock_source_mock.cpp:88-113) that is never reset between tests in this fixture family (ShardingTestFixture constructs via ServiceContext::make, not makeServiceContext(); sharding_mongos_test_fixture.cpp:99-103), and an earlier log-suppressor test (1423-1438) already advances it +2 s via a stack-local ClockSourceMock (delegating to the same global). A later advance during this window fires the 1 ms timer before the second onCommand and flips which payload the test sees.

      Acceptance Criteria

      • The two tests synchronize on the retry-arm state via a failpoint barrier (e.g. armRetryScheduledForTesting relative-count wait), not on implicit "clock didn't move" timing.
      • The tests are robust to a small mock-clock advance between the two {{onCommand}}s (no payload flip).
      • (Stretch, shared with the clock-reset ticket) the global mock clock is reset per test in this fixture family, removing the +2 s baseline-drift aggravator.

      Verification (confirm/deny)

      Insert {{getMockClockSource()->advance(Milliseconds

      {2}

      )}} between the two onCommand}}s. Not {{getBaton()->run(preciseClock) — a solo run would park on a max-deadline nothing advances, becoming a second runner or blocking forever. With the 2 ms advance, the 1 ms retry timer fires via the worker's own parked run_until, and the second onCommand absorbs it, flipping which payload the assertion sees -> confirms the missing barrier. If the assertion still holds, the timer is on the network alarm set not the DefaultBaton _timers map — but SubBaton forwards waitUntil to the parent (baton.cpp:102-108 -> default_baton.cpp:179-186), so this deny is unlikely.

      Investigation

      • Confirm whether either test is already flaky in CI history (the BF-46466 BFG list did not nominate this binary, so it may be latent only).
      • Decide between a local failpoint barrier vs. the fixture-wide clock-reset (postmortem Open Question 5).

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

              Created:
              Updated:
              Resolved: