You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Retire PolicyFingerprint as StreamingConfig's routing key #785
PolicyFingerprint describes itself as legacy, in its own module doc:
//! Legacy routing wrapper for a deployed stored output.//!//! An explicit `PrecomputeMaterialization::stored_output_id` takes precedence.//! Otherwise the compiler allocates a deterministic default from the existing//! policy fields (including pane layout and cadence). This identifier is not//! semantic identity: `SummaryDefinitionId` hashes the versioned semantic//! definition, and several deployed outputs may share that definition.
Its two original jobs have already been split out: semantic identity belongs to SummaryDefinitionId, and explicit routing belongs to stored_output_id.
The migration is real but cannot finish on its current path
stored_output_id adoption is climbing through the stack:
But PolicyFingerprint occurrences do not fall. Counted across every open PR branch, they sit at 300-302 against main's 301, and several branches are at 302. No open PR deletes crates/asap_types/src/policy_fingerprint.rs.
The reason is a loop in PolicyFingerprint::from_config:
pubfnfrom_config(cfg:&PrecomputeMaterialization) -> Self{ifletSome(output) = cfg.stored_output_id{return output.fingerprint();// the explicit id becomes a fingerprint too}
...
}
Adopting an explicit id does not remove a fingerprint; it changes how that fingerprint is derived. The computation is being retired. The type is not, because it is still the routing key.
What actually blocks retirement
StreamingConfig keys its materializations by the fingerprint as a bare u64:
HashMap<u64,PrecomputeMaterialization>
StreamingConfig is what the control plane POSTs to the data plane as YAML (control_plane/src/backend_client.rs). So the key is part of a published configuration format, and changing it is a format change rather than an internal refactor.
Proposed work
Key StreamingConfig's materializations by StoredOutputId instead of a bare u64.
Require stored_output_id, so from_config's hashing fallback has no callers, then delete the fallback and the module.
Step 3 is the reason this is worth doing now rather than later: #782 made a schema bump one edit plus whatever its test reports, instead of a literal that some producers follow and others quietly do not.
Not in scope
SummaryDefinitionId stays. It is semantic identity, a different thing from routing, and several deployed outputs may legitimately share one definition.
Related
Separately worth checking while in this area: plan_id is computed with std::collections::hash_map::DefaultHasher (control_plane/src/physical/compiler.rs), whose algorithm the standard library does not guarantee across Rust releases, yet plan_id is compared across processes and contract tests reject a changed installed generation. A toolchain upgrade would change it.
StreamingConfig's materialization map carries #[serde(skip)]. It is a process-local index, rebuilt by from_precompute_plan when the config is decoded. So re-keying it is an internal refactor, not a change to the published configuration format, and step 3 above (bump the schema version) is not required.
Two further facts make the first two steps cheaper than I said:
StoredOutputId and PolicyFingerprint are the same u64 wearing two type names, with free conversions in both directions:
Both are #[serde(transparent)], so wherever either is serialized the bytes are identical.
And PolicyFingerprint never appears as a serde field on any struct. The only place its value reaches a published document is inside a formatted string:
That schema_id does travel in PrecomputePlan.schemas and is validated on install, but since the two types hold the same number, passing a StoredOutputId produces the identical string.
So the work is: re-key the map and give it a name that says what it holds, take StoredOutputId in state_schema_id, then make stored_output_id required so the hashing fallback in from_config has no callers and the module can go. No format change, no version bump, and nothing about SummaryDefinitionId moves.
PolicyFingerprintdescribes itself as legacy, in its own module doc:Its two original jobs have already been split out: semantic identity belongs to
SummaryDefinitionId, and explicit routing belongs tostored_output_id.The migration is real but cannot finish on its current path
stored_output_idadoption is climbing through the stack:stored_output_idoccurrencesmainissue-752(#761)test/physical-execution-e2e(#775)test/issue754-level3(#759)But
PolicyFingerprintoccurrences do not fall. Counted across every open PR branch, they sit at 300-302 againstmain's 301, and several branches are at 302. No open PR deletescrates/asap_types/src/policy_fingerprint.rs.The reason is a loop in
PolicyFingerprint::from_config:Adopting an explicit id does not remove a fingerprint; it changes how that fingerprint is derived. The computation is being retired. The type is not, because it is still the routing key.
What actually blocks retirement
StreamingConfigkeys its materializations by the fingerprint as a bareu64:StreamingConfigis what the control plane POSTs to the data plane as YAML (control_plane/src/backend_client.rs). So the key is part of a published configuration format, and changing it is a format change rather than an internal refactor.Proposed work
StreamingConfig's materializations byStoredOutputIdinstead of a bareu64.stored_output_id, sofrom_config's hashing fallback has no callers, then delete the fallback and the module.WORKLOAD_SNAPSHOT_VERSION, added in fix(control-plane): give the snapshot schema version one owner #782).Step 3 is the reason this is worth doing now rather than later: #782 made a schema bump one edit plus whatever its test reports, instead of a literal that some producers follow and others quietly do not.
Not in scope
SummaryDefinitionIdstays. It is semantic identity, a different thing from routing, and several deployed outputs may legitimately share one definition.Related
Separately worth checking while in this area:
plan_idis computed withstd::collections::hash_map::DefaultHasher(control_plane/src/physical/compiler.rs), whose algorithm the standard library does not guarantee across Rust releases, yetplan_idis compared across processes and contract tests reject a changed installed generation. A toolchain upgrade would change it.