FetchStage: pass prefetch intention to EloqIndexCursor

[xiexiaoy]

Summary by CodeRabbit

• New Features

  • Index-backed queries can opt into earlier record prefetching to improve fetch efficiency for some workloads.

• Performance Improvements

  • Improved coordination between index scans and fetch operations reduces redundant I/O and can speed index-backed reads.

• Chores

  • Minor formatting and non-functional cleanups.

[coderabbitai]

Walkthrough

Adds an opt-in prefetch hook to SortedDataInterface::Cursor, exposes IndexScan's internal cursor, enables FetchStage to call the hook for IndexScan-backed paths, implements prefetch gating in EloqIndexCursor, and includes minor local-variable/formatting tweaks.

Changes

Cohort / File(s)	Summary
Cursor API src/mongo/db/storage/sorted_data_interface.h	Adds virtual void enablePrefetchRecords() {} to SortedDataInterface::Cursor as an optional hook.
IndexScan accessor src/mongo/db/exec/index_scan.h, src/mongo/db/exec/index_scan.cpp	Adds SortedDataInterface::Cursor* getCursor() to expose the underlying index cursor.
Fetch stage src/mongo/db/exec/fetch.cpp	On first work (no retry), detects IndexScan child, calls getCursor() and enablePrefetchRecords() on the cursor before delegating work.
Eloq cursor implementation src/mongo/db/modules/eloq/src/eloq_index.cpp	Adds void enablePrefetchRecords() plus private _enablePrefetchRecords flag; resets flag in clear logic and gates prefetching on the flag.
Eloq record store usage src/mongo/db/modules/eloq/src/eloq_record_store.cpp	Introduces local endSpecified variable passed into indexScanOpen (behaviorally unchanged).
CountScan formatting src/mongo/db/exec/count_scan.cpp	Trivial formatting change: annotated anonymous-namespace closing brace (no functional change).

Sequence Diagram

sequenceDiagram
    participant Fetch as FetchStage
    participant Scan as IndexScan
    participant Cursor as SortedDataInterface::Cursor
    participant Eloq as EloqIndexCursor

    Fetch->>Scan: on first doWork(), detect IndexScan child
    Scan->>Cursor: getCursor()
    Fetch->>Cursor: enablePrefetchRecords()
    activate Cursor
    Cursor->>Eloq: enablePrefetchRecords() (override)
    Eloq->>Eloq: set _enablePrefetchRecords = true
    deactivate Cursor
    Fetch->>Scan: proceed to child doWork()
    loop cursor advancement
        Scan->>Cursor: advance index cursor
        alt _enablePrefetchRecords (rgba(0,128,0,0.5))
            Cursor->>Eloq: prefetch record data
        else not enabled (rgba(255,0,0,0.5))
            Eloq->>Eloq: skip prefetch
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Batch lookup primary table for secondary index scan #404: Modifies Eloq cursor prefetching and related index-scan/cursor interfaces; strongly related to these prefetch API and gating changes.

Suggested reviewers

• lzxddz

Poem

🐰 I hop to the index with a curious twitch,
I nudge sleepy cursors to fetch without hitch.
A flag and a whisper — records line the trail,
No hurried back-and-forth, just a smooth little scale.
🥕📚

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately describes the main objective of the changeset: enabling FetchStage to communicate prefetch intentions to EloqIndexCursor via a new enablePrefetchRecords() method.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/mongo/db/modules/eloq/src/eloq_index.cpp (1)

326-335: Count-scan path can hit MONGO_UNREACHABLE.
Line 330 skips prefetch, but _updateRecordPtr() still assumes prefetched records for STANDARD/UNIQUE indexes and will MONGO_UNREACHABLE when the vector is empty. Count scans only need RecordId, so avoid the lookup when _scanForCount is true.

✅ Proposed fix (guard in _updateRecordPtr)

@@
             case IndexCursorType::UNIQUE:
             case IndexCursorType::STANDARD: {
+                if (_scanForCount) {
+                    // CountScan only needs RecordId; avoid prefetched record lookup
+                    _recordPtr = nullptr;
+                    break;
+                }
                 // We dont have prefetch records for upsert operations
                 if (!_opCtx->isUpsert()) {
                     // Look up prefetched record by index

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/mongo/db/modules/eloq/src/eloq_index.cpp (1)

326-336: Missing _scanForCount handling in _updateRecordPtr() will cause assertion failure.

When _scanForCount is true, prefetching is correctly skipped here. However, _updateRecordPtr() (lines 527-574) is still called from _updatePosition() at line 355. For STANDARD/UNIQUE index types when not in upsert mode, _updateRecordPtr() enters the prefetch lookup branch (line 537) but finds empty _prefetchedRecords, eventually hitting MONGO_UNREACHABLE.

_updateRecordPtr() needs to check _scanForCount similarly to how it checks isUpsert():

Proposed fix

             case IndexCursorType::UNIQUE:
             case IndexCursorType::STANDARD: {
                 // We dont have prefetch records for upsert operations
-                if (!_opCtx->isUpsert()) {
+                if (!_opCtx->isUpsert() && !_scanForCount) {
                     // Look up prefetched record by index
                     // ... existing prefetch lookup code ...
                 } else {
-                    // For upsert operations, we dont prefetch records
+                    // For upsert or count operations, we dont prefetch records
                     _recordPtr = nullptr;
                 }
             } break;

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/mongo/db/modules/eloq/src/eloq_index.cpp (1)

656-661: Resetting _disablePrefetchRecords in _clearPrefetchedRecords() may negate the disablePrefetchRecords() call.

In CountScan::doWork, the sequence is:

1. _cursor->setEndPosition(...)
2. _cursor->disablePrefetchRecords() — sets _disablePrefetchRecords = true
3. _cursor->seek(...) — calls _seekCursor() → _clearPrefetchedRecords() → resets _disablePrefetchRecords = false

This means the flag is reset before the prefetch logic in _updatePosition() is reached, effectively making disablePrefetchRecords() a no-op.

Consider moving the flag reset out of _clearPrefetchedRecords(), or only reset it in reset() where a full cursor re-initialization is intended.

🐛 Proposed fix

 void _clearPrefetchedRecords() {
-    _disablePrefetchRecords = false;
     _prefetchedRecords.clear();
     _prefetchedBatchStartIdx = 0;
     _lastRecordsBatchCnt = 0;
 }

If you want to reset the flag only during full cursor reset, keep it in reset():

 void reset(const EloqIndex* idx,
            OperationContext* opCtx,
            bool forward,
            IndexCursorType cursorType) {
     // ... existing code ...
     
     // Clear prefetched records
     _clearPrefetchedRecords();
+    _disablePrefetchRecords = false;  // Only reset on full cursor reset
 }

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/mongo/db/exec/fetch.cpp`:
- Around line 87-92: The null-check currently uses FetchStage::_cursor but then
dereferences the index cursor local variable cursor; change the guard to check
the index cursor (SortedDataInterface::Cursor* cursor from IndexScan in the
child() block) instead of _cursor so you only call
cursor->enablePrefetchRecords() when cursor is non-null; locate the dynamic_cast
to IndexScan* (function child()) and replace the if condition that checks
_cursor with a check of cursor.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix null-check to guard the index cursor, not the record-store cursor.
Line 90 checks _cursor (FetchStage’s record-store cursor) but then dereferences cursor. This can skip prefetch enablement and can also null-deref if the index cursor is absent (e.g., after scan end/reset). Use cursor in the check.

🛠️ Proposed fix

-        if (IndexScan* stage = dynamic_cast<IndexScan*>(childStage.get()); stage) {
-            SortedDataInterface::Cursor* cursor = stage->getCursor();
-            if (_cursor) {
-                cursor->enablePrefetchRecords();
-            }
-        }
+        if (IndexScan* stage = dynamic_cast<IndexScan*>(childStage.get()); stage) {
+            SortedDataInterface::Cursor* cursor = stage->getCursor();
+            if (cursor) {
+                cursor->enablePrefetchRecords();
+            }
+        }

🤖 Prompt for AI Agents

In `@src/mongo/db/exec/fetch.cpp` around lines 87 - 92, The null-check currently
uses FetchStage::_cursor but then dereferences the index cursor local variable
cursor; change the guard to check the index cursor (SortedDataInterface::Cursor*
cursor from IndexScan in the child() block) instead of _cursor so you only call
cursor->enablePrefetchRecords() when cursor is non-null; locate the dynamic_cast
to IndexScan* (function child()) and replace the if condition that checks
_cursor with a check of cursor.

[liunyl]

So the current workflow is

1. scan return first tuple
2. fetch stage mark cursor as enable prefetch
3. fetch tuple read pk table for first index tuple
4. sk index scan then prefetch rest of the tuples?

[xiexiaoy]

So the current workflow is

1. scan return first tuple
2. fetch stage mark cursor as enable prefetch
3. fetch tuple read pk table for first index tuple
4. sk index scan then prefetch rest of the tuples?

Right.