[ragnorc]

What & why

This draft RFC proposes that the v2 planner price storage round trips as well as rows. On object storage, the rows-only cost model picks plans built from dependent lookups (indexed hops, id_lookup slices). Each of those waits on the one before. The plans it passes over read a table in parallel, and on object storage they win.

The RFC adds one term to today's row costs: the request waves on an alternative's critical path, priced by a storage profile chosen from the snapshot's storage kind and recorded in the plan. A wave is a set of requests issued together, costing about one round trip. The local profile charges nothing for a wave, so local and DST plans are exactly today's plans, and the GQT corpus with them. The object-store profile charges about 20 ms per wave, which matches requests timed from inside a hosted server's region. The first two decisions to use the model are expand mode (indexed or CSR, at plan time and at run time) and destination access path (id_lookup or hash_join). Results never change.

The motivating measurement uses GQT --measure --model s3-like on DST, with 200,000 Person nodes in 50 fragments and a selective Company { slug } binding, warm:

Plan	Requests	Waves	Simulated time
Selective binding declared last: scan, CSR, hash_join	104	3	53 ms
Selective binding declared first: key lookup, indexed hop, id_lookup	313	9	161 ms

On a local filesystem the order reverses: 0.03 s against 0.16 s. So no rows-only constant serves both stores. Cardinality-driven anchor selection, Kuzu-style, would slow hosted reads until the cost model can see waves.

Backing issue / RFC

• Is an RFC PR: docs/rfcs/2026-10-13-request-aware-plan-cost.md (status draft)

Checklist

• Change is focused (one RFC and its registry row)
• Tests added/updated for behavior changes (N/A, no code)
• Public docs updated if user-facing surface changed (N/A)
• Reviewed against docs/dev/invariants.md. The RFC's Invariants section covers 7, 9, 11 and 13 and the deny-list items on hidden statistics, cloud-only correctness and fragment-count preconditions.

Local verification

• python3 scripts/check-docs.py: Documentation OK.
• typos on the changed files: ok.

Notes for reviewers

@azimafroozeh, this builds directly on the index-facts RFC and the way gather and Assumptions record planner inputs, so your read would help most on:

• Unresolved questions 1–4. Where the object-store constants come from; one profile or several; warm-state pricing; whether to cost the persisted CSR artifact.
• Recording the profile. It is a typed Assumptions field, because the storage kind cannot be derived from the root-relative dataset pins. Its constants ride on ExpandCostInputs, so the run-time re-decisions price with the same model.
• Units. The RFC treats today's constants (CSR_BUILD_FACTOR, HASH_JOIN_RATIO) as per-row time ratios, and treats an indexed probe row and a scanned row as one unit. The local profile then reproduces today's decisions exactly.

The open correlated-block strategy PR #973 is related. Its S3 request evidence and summed-rows pricing would read the same profile in a later change. Anchor selection, cache-aware CSR pricing, and correcting AntiJoin's unconditional csr_cached are explicitly out of scope.

[cursor]

Review: request-aware plan cost RFC

Recommendation: accept after the four optional items below are addressed or explicitly deferred.

Why this RFC

The v2 planner picks operators by row count alone. On object storage, the plan with fewer rows can be 3x slower because its operators wait in series on dependent lookups. The RFC adds one term -- serial request-wave depth times a storage-profile latency -- to the existing row-cost formula, then sets the local-storage profile to zero so every local and DST plan is unchanged. The problem is real, the evidence is checked in, and the proposal is the smallest change that closes the gap.

What it does

• Defines a storage profile (latency, parallelism, byte cost) chosen once from the root URI at gather time and recorded in Assumptions.
• Extends the cost formula: cost = row_work (unchanged) + serial_depth × latency_units + bytes × byte_units.
• Applies the new term to two existing decisions: choose_expand_mode (indexed scan vs CSR) and choose_access_path (id_lookup vs hash_join).
• The local profile sets latency_units = 0 and byte_units = 0, so local plans are provably today's plans. That is a testable invariant, not an approximation.

How it aligns

• Invariant 7 (physical acceleration is derived): the profile changes operator choice, not query semantics. Both alternatives already have identical results.
• Invariant 9 (typed structures): the profile is a typed Assumptions field, not a string or ambient read.
• Deny-list: cost-blind plan choice / hidden statistics: every input of every priced decision is recorded in the plan and printed by explain.
• Deny-list: cloud-only correctness: this is cost, not correctness. One code path, one model, local profile is the zero case.
• RFC 0047 equal-inputs-equal-plan: the profile is a recorded input; a replay uses the plan's recorded profile, not the ambient storage.

Alignment is clean. No invariant is under strain.

What I verified

Check	Result
id_lookup 256-row serial slices	Matches scan/input.rs:108: .clamp(1, 256), sequential for offset in (0..) loop
EXPAND_OUTPUT_ROWS = 256 (not 8,192)	Matches expand_stream.rs:34; the 8,192 in the request-model table is the upstream input batch, not the hop output chunk
csr_cached set unconditionally on AntiJoin	Confirmed at optimizer.rs:3006: self.csr_cached = true regardless of whether the block builds a CSR
HASH_JOIN_RATIO = 8, CSR_BUILD_FACTOR = 1.5	Exact in cost.rs:92, 153
StorageKind and storage_kind_for_uri	Exist in omnigraph-storage/src/lib.rs:1148-1152, 2202-2217; shared-memory:// is Local under the dst feature
Assumptions has no existing storage-profile field	Confirmed at physical.rs:121-140; adding one is the same pattern as memory_limit and gate_policy
BOUND_PLAN_VERSION is currently 8	bound.rs:24
PR #973 (correlated-block strategy) is open	Confirmed; the RFC's explicit scoping is valid
RFC registry row is correctly formatted	One new row, sorted by date, correct columns
check-docs.py and typos pass per the PR description	Noted; no code changes to lint

Findings

All optional. None block the draft or its progression to accepted.

1. csr_cached accuracy before step 2. The RFC correctly identifies that AntiJoin unconditionally sets csr_cached even when its block does not build the CSR (optimizer.rs:3006). On object storage the CSR-build cost is the main lever the model pulls, so an inaccurate csr_cached would overcount free CSR paths. The RFC scopes the fix out ("would change local plans, so it is a separate fix"). Suggestion: cross-reference this as a prerequisite or co-requisite of step 2, with an issue number, so the dependency is tracked. If step 2 ships before the fix, the model may pick CSR for an expand that follows an AntiJoin whose block never built the CSR -- the CSR will be built on the fly, which is correct but not free.

2. Clarify the byte term per operator. The cost formula includes bytes × byte_units, and the CSR row mentions "plus its bytes, priced per query," but the other operators' rows do not spell out their byte term. The table scan, indexed scan, id_lookup, and hash_join rows show serial depth only. A reader should be able to reconstruct the full cost of each alternative from the table alone. Consider adding a Bytes (warm) column or a note that the byte term is zero for every operator except the CSR build in this RFC.

3. The 8,192-row batch in the indexed-Expand model. The request-model table says "per input batch of at most 8,192 rows." In the engine, the input is whatever the upstream producer emits (Lance's default is 8,192), but the hop itself processes in EXPAND_OUTPUT_ROWS = 256-row chunks (expand_stream.rs:34). The 8,192 is the input batch granularity at which the walk loop advances, not the scan granularity. A parenthetical "(the upstream producer's batch; each hop's scans are chunked at 256)" would prevent misreading.

4. Calibration reproducibility. The latency_units constant is "20 ms divided by the measured cost of one indexed-probe row on the reference host." The RFC plans to check in calibration records with the benchmark fixtures (rollout step 4). That step should also document the reference host's hardware and the benchmark command, so a future maintainer can reproduce the ratio. The unresolved-questions section already covers the shape of this; this is a note that the specific calibration artifact matters.

Tradeoffs and liability

• What this deprioritizes: cache awareness. The RFC explicitly prices the CSR as a cold build, even though a warm server usually has it cached. This is the right call for a deterministic plan: process-level cache state is not a plan input and would break the equal-inputs-equal-plan rule. The cost is that the first query after a restart may choose the CSR path and pay the build, but the build is correct and the query returns the same rows.
• What this does not address: join ordering, anchor selection, and late-materialization cost awareness. All are explicitly scoped out and can adopt the recorded profile later without a new RFC. This is a clean boundary.
• Reversibility: setting two constants to zero restores today's plans everywhere, without a format change. The plan-version bump is the only irreversible part, and it follows the existing regeneration protocol.
• Ongoing liability: one new field in Assumptions, one new field in ExpandCostInputs, two modified cost comparisons, and a profile-selection function. The surface is small. The phased rollout (plumbing, then expand mode, then access path, then calibration) further limits risk per step.

Summary

This is a well-motivated, narrowly scoped draft RFC with concrete measurements, clean invariant alignment, and a reversible design. The local-profile-is-zero property is the strongest design choice: it eliminates regression risk for every existing test and user. The four optional items above are clarifications, not design objections. No blocking issue.

[ragnorc]

I addressed the review in 99ba93a, after merging main:

1. csr_cached on AntiJoin. The correction is now a co-requisite of step 2. Rollout step 2 lands with it, and Compatibility names it as the rollout's one local plan change, with its own GQT updates. I confirmed the unconditional set on current main (optimizer.rs, the AntiJoin arm of Lowering::lower). No issue is filed yet; it can be filed when step 2 starts, or now if that helps track it.

2. Bytes per operator. The request-model table has a "Bytes priced" column:

   • full scans and hash_join builds use their projected bytes, the latter from build_side_bytes;
   • the CSR build uses the src/dst column bytes;
   • an indexed hop uses the unindexed fragments' columns;
   • dependent reads (key-filtered scan, id_lookup, indexed take) price zero.

   A new paragraph says that understates them, since the measured id_lookup read 4.0 MB against a full scan's 4.8 MB. The error leans toward today's choice.

3. 8,192 vs 256. I checked single_hop.rs (prepare → scan_neighbor_map). An indexed hop issues one Lance scan per input batch and orientation, over all of the batch's distinct source ids; the keys are not split. EXPAND_OUTPUT_ROWS (256) bounds only the output chunks of pairs, which issue no requests. So the row now says that, instead of "each hop's scans are chunked at 256". It also says the input batch is at most the session's 8,192 rows and smaller when the producer reaches its byte target first.

4. Calibration. Rollout step 4 now requires the record to name the host's CPU, cores, memory, OS and storage, the Rust and Lance versions, the exact benchmark command and fixture, and the measured per-row cost. Evidence refers to it.

I also added a "Known limitation: average-degree estimates" section, following the hub-root note in #973's review. It now has Unresolved question 5: a declared run-time switch from id_lookup to hash_join, proposed as a follow-up.

check-docs.py and typos pass.

[cursor]

Review: request-aware plan cost RFC, head be7bbd25

Outcome: request changes. There is one blocking item: the worked example's arithmetic. It is a fix to the document, not an objection to the design. The design is sound, and the motivating measurement reproduces at this head (see Validation).

This is a top-level comment because this account cannot submit a formal review state. Treat it as "Request changes". Each inline item links to its exact line at be7bbd25.

Why, what, how, impact

• Why. On object storage, the v2 planner can pick a plan that waits on storage many times in series. Row counts do not show this cost, so the plan with fewer rows can be about 3x slower.
• What. Each priced alternative gets one more term: serial request waves × per-request latency, plus bytes × byte cost. A storage profile, chosen from the root URI at gather time and recorded in Assumptions, supplies the constants.
• How. The local profile sets both constants to zero, so local and DST plans stay the same. The object-store profile charges about 20 ms per wave. The first users are the Expand mode choice and the destination access path (id_lookup or hash_join).
• Impact. Results do not change. Only operator choice changes, and only on S3 and Azure roots. Plans get one typed field, explain gets new rows, and the bound plan version goes up.

Earlier findings (review of 2026-10-14): all resolved in 99ba93a2

1. The csr_cached correction is now a co-requisite of step 2 (Rollout, Compatibility). Resolved.
2. The request model has a "Bytes priced" column, and the text says dependent reads price zero bytes. Resolved. Optional item O3 asks where those bytes come from.
3. The indexed hop is now described as one Lance scan per input batch and orientation. I confirmed this in single_hop.rs prepare → graph.rs scan_neighbor_map: one key IN (keys) scan per orientation, with the orientations run one after another. EXPAND_OUTPUT_ROWS bounds only the output chunks. Resolved.
4. The calibration record now names the host, the toolchain and Lance versions, and the command. Resolved.

The new "average-degree" section and Unresolved question 5 are accurate.

Blocking

B1. The worked example counts waves only, but the formula also keeps the row term. With the RFC's own inputs, the claimed flips hold only if latency_units is very large. (RFC L219–227, L162–173)

Here is the arithmetic, using today's cost.rs on the 200,000-row graph:

• Expand $c → $p. The direction is In, so src is Company with 200 rows (expand_statistics). The fanout is 200,000 / 200 = 1,000, and the frontier is 1.
  • Indexed: 1 × 1 × 1,000 × 1 = 1,000 row units, in 3 waves.
  • CSR: 1.5 × 200,000 = 300,000 row units, in 2 waves, plus about 3 MB.
  • The CSR wins only if latency_units > 299,000 + 3 MB × byte_units.
• Access path over 1,005 ids.
  • id_lookup: 8 × 1,005 = 8,040 row units, in 4 waves.
  • hash_join: 200,000 row units, in 2 + 1 waves, plus about 4.8 MB.
  • hash_join wins only if latency_units > 191,960 + 4.8 MB × byte_units, or > 95,980 without the "+1" (see O1).

With latency_units = 20 ms / (cost of one indexed-probe row), the Expand flip needs one row to cost less than about 67 ns on the reference host. The RFC does not show this, and its DST evidence cannot show it. GQT --measure charges only store requests on a paused clock (measure.rs Model). So the 53 ms against 161 ms leaves out the CPU cost of the 300,000-unit CSR build and the 200,000-row hash build, which is exactly the work the row term prices.

As a rough check, the RFC's own local numbers (0.03 s against 0.16 s over 500,000 edges) suggest about 150–200 ns per row unit. That gives latency_units of roughly 100,000–130,000. At that value, the access-path flip holds without the "+1", but the Expand flip does not. These are one-shot CLI timings, not a measurement, so treat this only as a sign that the risk is real.

The row constants cannot change to close the gap: the RFC promises that the local profile reproduces today's plans. So latency_units is the only lever.

Requested change. In "The model", write out each alternative's full cost: the row, wave and byte terms. State the latency_units range in which the claimed plans hold. Tie the Evidence acceptance threshold (at most 6 waves and 110 ms) to that range. Then say what happens if calibration (step 4) lands below it: for example, steps 2 and 3 do not ship, or the request model is revisited. As written, "the model reproduces the measured order" is not supported by the model's own formula.

Optional (non-blocking)

O1. The HashJoin "+1" wave is asymmetric. (L193) Both alternatives wait for the probe's first batch. hash_join.rs reads that batch before it executes the build. lookup_candidates also starts at the first input batch, and then awaits each slice of up to 256 ids in turn. If only hash_join pays the +1, the margin in B1 is cut in half. Remove the +1, or say what it models.

O2. Say which edge types the CSR price covers. (L195) One CSR build covers every edge type the plan traverses, not only the type of the Expand being decided. plan_edge_types collects every Expand member in any mode, including AntiJoin inner trees. GraphIndex::build then scans those types one after another. State that Σ_types sums over this set and that the planner computes it from the logical plan before lowering. Otherwise a query over several edge types under-prices the CSR. Today's row term prices only one type's |E|.

O3. The byte terms need statistics the planner does not read today. (L46–47, L198–200) At this head, gather records whole-file bytes only for Expand destination node tables (query_table_statistics: file_bytes: destination.then_some(size)). It loads column bytes only for a borderline hash-join destination (column_statistics_needed). Edge tables and non-destination scans get neither, yet the CSR, table-scan and unindexed-fragment byte terms need them. There are two ways to get them:

• Use manifest file sizes. This costs no I/O, but it overstates src/dst bytes when an edge type has properties.
• Widen calculate_data_stats. Lance 11 (dataset/statistics.rs, collect_current_data_stats) opens every fragment's data files, so this adds plan-time I/O on object storage. It also changes the DST storage-cost golden for operations that plan reads.

Name the choice, and correct "adds no statistic beyond the fragment facts the planner already opens".

O4. The parallelism N is not a fixed 64. (L148–151, L400–404) Two things lower it:

• Lance 11 ObjectStore::io_parallelism() reads LANCE_IO_THREADS each time it is called (lance-io object_store.rs). The Lance performance guide advises lowering it for cross-region access.
• The pipelined scan caps read-ahead at 64 MiB (engine/scan.rs L42, L274). Lance pages of 8–32 MiB therefore give 2–8 requests in flight, not 64.

Record N from the store at gather, as the expand ceilings are recorded, or say why a fixed 64 is accepted. Unresolved question 1 can carry this.

O5. Some references are stale against main. (L210–211, L254–256) #973 merged as eacfce6d after this branch last merged main. On main (0b5e17fa):

• Only a correlated-strategy AntiJoin sets csr_cached; a key-set AntiJoin does not.
• BOUND_PLAN_VERSION is 10.
• perf(planner): run a bounded correlated block once as a key-set anti-join #973's anti_join_strategy is a third cost decision that uses rows only.

At the next merge, update "Today every AntiJoin sets csr_cached", the "correlated-block strategy of PR #973" wording, and the code anchor on L22.

O6. Test both sides, and check in the instrument early. (L357–372) The acceptance checks cover only the shape that improves. Add one object-store shape where the plan must not flip, for example a frontier of one id_lookup slice against a large table. The measured fixture is not in the repository, so the motivating numbers cannot be rerun from the tree (invariant 13). Check it in with step 1. The temporary case under Validation reproduces the numbers and could serve as a start.

Tradeoffs and software liability

• Correctness. No result changes. Each affected decision chooses between alternatives that are already declared and equivalent. The profile is a recorded plan input, so the plan stays a function of its recorded inputs (RFC 0047), and replay uses the recorded profile. I found no strain on any invariant.
• Liability added. One Assumptions field, an ExpandCostInputs extension, explain rows, a harness header and a plan-version bump. Above all, one global exchange rate (latency_units) that every future priced decision will share. A recalibration moves every object-store decision at once. That is acceptable because plans record their profile, but the calibration record becomes a maintained artifact.
• Liability removed. The rows-only choice that the measurements show is wrong on object storage, without a separate cloud model or code path.
• After five more changes like this. Anchor selection, join order, the key-set anti-join and late materialization would all read the same profile. That is the right shape: one formula. The cost is that each request-model row must track operator details (slice size, scheduler caps, scan buffer). Consider one planner test per row that fails when the operator's shape changes, so the model cannot drift silently.
• Deprioritized. The model prices one query's critical path on an idle store. Under concurrent load, total requests (Lance's AIMD throttle, GET cost) also matter. In the measured case the chosen plan also issues fewer requests (104 against 313), so both goals agree there. The RFC should say this objective out loud.

Validation

All of this was run on a clean, detached worktree at be7bbd25e7c205b27b3fe4d953e95b665b15b0d0. I compared it with 99ba93a2 and with main at 0b5e17fa.

• python3 scripts/check-docs.py (pinned docs venv) → Documentation OK (293 Markdown files checked).

• typos 1.50.1 (the CI pin) → exit 0.

• bash scripts/check-agents-md.sh → OK.

• CI run 38822917578 at this exact head → success. The Rust jobs skip because the change is docs-only.

• Reproduction. I wrote a temporary case outside the repository with the RFC's graph: Person 200,000 rows in 50 commits (50 fragments), WorksAt 200,000 edges with uniform companies, Company 200 rows, # traversal: auto, DST with seed 0. I ran it with omnigraph-gqt <case> --measure --model s3-like, built at this head. Result: ok, 1,095 matches (the RFC's fixture had 1,005, so its generator seed differs).

  Warm step	Plan asserted (--- expect plan)	Requests	Sim ms	RFC
  $c last	expand $p $c: mode csr	104	52.9	104, 53 ms
  $c first	expand $c $p: mode indexed_scan, scan Person as $p: access id_lookup	314	178.9	313, 161 ms

  The extra ~17 ms on the $c-first step is one more id_lookup slice: ⌈1,095 / 256⌉ = 5 slices, against 4 for 1,005 ids. The case was not added to the tree.

• Code inspection at this head confirms: the cost formulas and constants; the 256-id serial id_lookup slices; the hash join's probe-first ordering; the indexed hop's per-batch, per-orientation scans; the serial CSR build per edge type; storage_kind_for_uri (with shared-memory:// as Local under dst); the Assumptions fields; and that issue_659_typed_edge_alternation.gqt is the only case that declares both environments and asserts a mode. The S3 traversal test (s3_storage.rs) pins Traversal::Csr, so the profile does not affect it.

• Lance 11.0.0 source: filtered_read.rs (fragment_readahead defaults to io_parallelism × 2), lance-io object_store.rs (64 for cloud, 8 for local, LANCE_IO_THREADS), and dataset/statistics.rs. Upstream pages read in full: object stores, performance, observability, read/write, DataFusion, and BTREE.

• Limits. No implementation exists to test, so no before/after regression proof applies: this PR fixes no bug. I did not measure the per-row CPU cost. B1 rests on the RFC's formula and constants, not on a timing. The performance claims stay unverified outside DST's storage-only model.