-
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).
- is related to
-
SERVER-133148 Avoid using semi-initialized opCtx when retrying getMores
-
- Closed
-
- related to
-
SERVER-133148 Avoid using semi-initialized opCtx when retrying getMores
-
- Closed
-