Repository navigation
refactor(planner)!: summary identity and dedup move to the runtime's SDS - #760
Merged
Merged
Conversation
The Planner says what the plan needs: which nodes are stored, when each is built, its pane, and how long that node's output must be kept. Remove StoredOutput.computation_id (the SHA-256 that mirrored the runtime's definition id) and the step that gave every output of one computation the largest retention; each StoredOutput.retention_ms is now its own node's requirement. Drop the sha2 dependency. The stand-in runtime test dedupes panes with its own cache key and keeps a shared pane for the largest retention among the outputs it serves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
User decision (2026-10-19): "make sure SDS is in runtime asapquery-backend, not asapplanner."
The Planner says what the plan needs: which nodes are stored, when each is built, its pane window, and how long that node's output must be kept. The runtime's stored-summary system (SDS) owns how: summary identity and definition ids, deduplication of identical stored outputs across queries, store layout, and eviction. The
plan_installablehandoff added SDS concerns tocrates/planner/src/installable.rs. This PR moves them out.Summary identity and dedup move to the runtime's SDS. ASAPQuery-backend has this on its SDS feature branches (
data_plane/src/sds.rs, not yet on main). It hashes the sub-DAG and treats offset-onlyTimeShifts as transparent. When the backend re-pins to this Planner version, it must keep that hashing and use it. It must also keep each definition for the maxretention_msof the outputs that share it.What
StoredOutput.computation_idis removed. That was the hex SHA-256 of what an output computes, and it copied the backend's DefinitionId scheme. Its hashing (computation_hash) and the planner'ssha2dependency are removed too.StoredOutput.retention_msis now that node's own requirement. The Planner no longer gives every output of one computation the largest retention among them. Its doc says that a runtime which stores identical outputs once must keep them for the largest requirement among them, and that this is the runtime's choice.StoredOutputBuild::QueryRunandpane_ofdocs.Wire format change:
plan.jsonno longer hasstored_outputs[].computation_id.StoredOutputisdeny_unknown_fields, so an older plan file that still has the field is refused.How
runtime_inputs_ofbuilds eachStoredOutputfrom its node only:node_id,built_at,pane, andretention_ms. The retention is Stage 3'spane_retention_msfor the node's chain, or else the pane's reach. The hashing pass and the max-per-computation pass are deleted.The stand-in runtime test (
crates/planner/tests/runtime_install.rs) now uses its own cache key,pane_cache_key. The key is built from the stored node's and itsScan's operators and output schemas with node ids removed, plus the pane width and the pane end. Offsets are left out, so panes of one chain and of an offset twin that cover the same window share an entry. The key is valid only for the shapes in these tests, where the stored node reads its pane'sTimeRangedirectly, and the test asserts that. A cached pane keeps the largestretention_msamong the outputs it serves. That is the runtime-side rule this PR hands to the SDS.Other SDS concepts checked in the Planner tree
I checked
crates/planner,executor,types,plan-selection,physical-asap-optimizationandlogical-asap-optimizationfor stored-summary definitions, identities, store layout, eviction and dedup. Nothing else needs to move:physical_planner/compiled.rssays "without storage identity",candidates.rssays "Stored outputs have no storage identity", andtypes/src/physical/summary_window.rssays it does not "identify a runtime library, process, placement, shard layout, storage".precompute.rs's "identities which are implicit in a stored summary instance" refers to population and pane columns in the executor's rows, not to a store.storage_iocost model in plan-selection prices storage requests. It is a cost fact, not SDS.Selection::pane_retention_ms(one retention per chain), materialization, timing and pane windows are plan facts, and they stay.Deduprelational operator, CSE, or label-matcher dedup.Key code interfaces added / modified
crates/planner/src/installable.rscrates/planner/Cargo.toml: dependencysha2removed.Usage
From
an_offset_chain_keeps_its_panes_for_its_window_plus_the_offset(crates/integration-tests/tests/runtime_handoff.rs), run at this head:Before this PR
The Planner hashed every stored output into a
computation_idthat copied the backend's DefinitionId. It also overwrote each output'sretention_mswith the largest one among outputs sharing that id, which treated those outputs as a single pane store. In the twin above, all 12 outputs hadretention_ms = 4_200_000. Summary identity and the retention merge, both runtime storage decisions, were made in the Planner.After this PR
Each
StoredOutputdescribes only its own node: what is stored, when it is built, from which pane, and for how long. In the twin, the unshifted query's six outputs have 3,600,000 ms and the offset query's six have 4,200,000 ms. Identity, dedup across queries, store layout and eviction belong to the runtime's SDS (data_plane/src/sds.rsin ASAPQuery-backend).Follow-up (backend re-pin): keep
sds.rs's sub-DAG hashing with offset-onlyTimeShifts transparent, use it to dedupe stored outputs, keep each definition for the maxretention_msof its outputs, and stop readingcomputation_id.Verification
cargo nextest run -p asap-planner --test runtime_install --lib: 8 passed. The stand-in runtime runs the following on its own cache key, and every run's answers equal the query-time reference:an_hour_and_its_offset_twin_both_equal_the_reference(formerlypanes_of_one_computation_build_once_across_queries): 12 stored outputs, 7 panes built, both answers equal the reference;query_time_results_kept_across_evaluations_are_built_in_query_runs: 5 panes, then 1;cargo nextest run -p asap-integration-tests --test runtime_handoff: 8 passed.stored_panes_carry_stage3_pane_retentionkeeps 7,200,000 ms for the range-evaluation case.an_offset_chain_keeps_its_panes_for_its_window_plus_the_offsetnow checks each query's own outputs (3,600,000 / 4,200,000 ms) instead of a shared computation_id and a shared max.cargo fmt --all -- --check,cargo check -p asap-planner --tests --locked, andcargo clippy -p asap-planner -p asap-integration-tests --all-targets --all-features --locked -- -D warnings: clean.