Defer call to makeNotifier() in classic plan executor

XMLWordPrintableJSON

    • Type: Task
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: Internal Code
    • None
    • Query Execution
    • QE 2026-08-03, QE 2026-08-17
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      In the classic plan executor, `getNext()` and `getNextDocument()` calls route to the internal `_getNextImpl()` function.
      `_getNextImpl()` can built a notifier object for tailable + awaitData cursors. For tailable + awaitData cursors, the notifier is currently built unconditionally upon every entry into `_getNextImpl()`. The construction of the notifier can be deferred until the notifier is first needed to wait for inserts though. This removes a cycles from the hot path, and for tailable + awaitData cursors removes unnecessary notifier creations when the executor can still produce more results.

      Note that the classic plan executor also has a `getNextBatch()` implementation which created only a single notifier instance for an entire batch for tailable + awaitData cursors. This case now also defers the creation of the notifier until next, but it can only save the creation of a single notifier object per batch, in case the executor is able to produce more results without waiting.

      Reasoning from Claude Code about why deferring the creation of the notifier is safe:

      The thing that makes this look risky is the comment "Holding a shared pointer to the capped insert notifier is
      necessary for the notifierVersion to advance." That is literally true:
      RecordStoreBase::Capped::notifyWaitersIfNeeded() bumps the version only when hasWaiters() is true, and hasWaiters() is
      _cappedInsertNotifier.use_count() > 1 (record_store_base.cpp:152-160). So the version only advances while some reader
      holds an extra shared_ptr.

      The question is whether creation must happen before the scan to avoid a lost wakeup. Tracing the wait protocol
      (plan_insert_listener.cpp:91 + record_store.cpp:14-35), it does not, for these reasons:

      1. The version is never read at construction time. LocalCappedInsertNotifier only reads getVersion() inside prepareForWait(), which runs at EOF, not at construction. So when the object is created doesn't change which version snapshots get compared.
      2. The first EOF never blocks. _lastEOFVersion starts at ~0, so the first waitUntil() returns immediately and the loop re-scans. Any insert that landed during the productive scan (while no notifier was held, so the version didn't move) is picked up by that non-blocking re-scan pass — exactly as it is today. The version bump was never what caught those; the re-scan is.
      3. The blocking window is fully covered. Real blocking only happens on the second consecutive EOF. From the first EOF onward, the deferred code already holds the notifier (use_count > 1), so inserts bump the version and wake the waiter identically to today. The only window where deferral changes version-advancement behavior is the productive-scan window — and no one is blocked there, so no notification is needed.
      4. Cross-cursor notifications are unaffected. The version/notify only matters for a cursor that is currently waiting, and a waiting cursor always holds the notifier. A cursor that is actively scanning never needs to be notified (it will re-scan). Today's code already drops the notifier between getNext calls, so it does not continuously hold it across a batch anyway.

      One thing the deferral must preserve: create the notifier once, lazily, and reuse it across loop iterations within the
      same getNextImpl call. If you recreated it on each EOF, _lastEOFVersion would reset to ~0 every time and the cursor
      would spin without ever blocking. So keep the unique_ptr declared before the loop; just don't populate it until first
      needed.

            Assignee:
            Jan Steemann
            Reporter:
            Jan Steemann
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: