-
Type:
Bug
-
Resolution: Unresolved
-
Priority:
Minor - P4
-
None
-
Affects Version/s: None
-
Component/s: None
-
None
-
Query Execution
-
ALL
-
-
None
-
None
-
None
-
None
-
None
-
None
-
None
The proposed solution to SERVER-135600 introduces a minor race condition for the sake of implementation simplicity. This is considered a minor bug because it only impacts $merge aggregations, which are already sensitive to data loss/corruption on the event of any in-server error (SERVER-45209). To fix this, we should let queued operations check query-memory load shedding on their own thread while waiting for an execution ticket.
Background
Query memory load shedding sheds memory-heavy operations at query-execution checkpoints, which run on the operation's own thread (queryMemoryCheckLoadShedding). An operation waiting for an execution ticket reaches no checkpoint, so SERVER-135600 added a sweep on the RSS monitor thread (sweepQueuedOperationsForMemoryPressure). It finds operations queued for a read ticket and kills them via ServiceContext::killOperation().
Deciding on another thread has costs:
- A race between the decision and the kill. The sweep decides under ClientLock, but ticket grant and lock acquisition don't take ClientLock. The op can be granted its ticket, reach its write phase and take its IX lock between the sweep's candidacy check and markKilled(). It can then be killed with a retryable QueryMemoryLimitExceeded after a partial write. The window is very narrow, but nothing rules it out, and a kill can't be withdrawn once set.
- Cross-thread state. LoadSheddingOpState's eligibility flag and dt clock had to become atomics. The sweep reads tracked bytes from CurOp's published copy instead of the tracker, uses the monitor's PRNG, and relies on Locker::wasGlobalLockTakenInModeConflictingWithWrites() being safe to read from another thread.
- Responsiveness tied to the monitor interval. Queued ops are only evaluated every queryMemoryRssMonitorIntervalMillis.
Proposal
While waiting for a ticket, have the queued operation itself periodically call queryMemoryCheckLoadShedding(opCtx). If it decides to shed, the op marks itself killed and throws out of the wait, so it never receives a ticket and cannot write. This removes the race and the cross-thread machinery, and lets us delete the monitor-thread sweep.
What's needed:
- A periodic wake in OrderedTicketSemaphore (kPrioritizeFewestAdmissions, used by the low-priority read/write pools; see execution_control_init.cpp). Today it waits in waitForConditionOrInterruptUntil until signalled, interrupted or the deadline, with no periodic wake. These pools are where execution control deprioritizes long-running queries, i.e. our main shedding targets. Use a timed wait with jitter (as UnorderedTicketSemaphore does: 500ms ±20%) that keeps the waiter's place in _waitQueue.
- A "while queued" callback hook called from both semaphores' wait loops on each wake. UnorderedTicketSemaphore already wakes every ~500ms but only calls the generic checkForInterrupt(). Admission code can't depend on query/memory-tracking libraries, so the hook should be registered, following the existing TicketHolder callbacks (StartQueueingCallback, WaitedAcquisitionCallback, …). Query memory load shedding would register one that runs queryMemoryCheckLoadShedding.
- Call the hook outside the semaphore mutex. OrderedTicketSemaphore::acquire currently holds _mutex across the wait and the following checkForInterrupt(). The callback does CurOp reads, logging and OTel counter updates, and must not run under a global admission lock. Drop the mutex around the call while staying in the wait queue, and handle being woken (awake) during the callback.
Once this lands, remove from query_memory_load_shedding.cpp: sweepQueuedOperationsForMemoryPressure, isQueuedReader, the monitor's sweep call, and the atomics in LoadSheddingOpState. The waitingForTicket log field can be kept by passing a flag through the hook.
Notes / risks
- Wake cost: periodic wakeups for every low-priority waiter. Jitter limits a thundering herd. The interval should stay under kMaxShedEvalDt (1000ms), which caps the dt charged per evaluation.
- The hook should be cheap when load shedding is disabled (one relaxed load in queryMemoryCheckLoadShedding).
- Write-ticket waiters can stay excluded: shed-eligible ops are reads, and the write-intent latch still protects writes.
- Owned by admission/execution control, so needs review from that team.
Testing
- Unit tests for both semaphore types (kCompeting and kPrioritizeFewestAdmissions): an eligible, memory-tracked waiter on a full pool is shed by its own thread within one wake interval under forced memory pressure, never receives a ticket, and running ticket holders are untouched.
- Port the existing sweep tests and query_memory_queued_operation_shedding.js to the new mechanism, including a deprioritized (low-priority pool) waiter.
- is caused by
-
SERVER-135600 Queued operations should participate in query memory load shedding
-
- Closed
-
- is related to
-
SERVER-45209 Allow $merge in aggregations in multi-document transactions
-
- Backlog
-