Conversation
This was referenced Oct 3, 2026
zzylol
force-pushed
the
stack/528-11-deployment-inputs
branch
from
October 3, 2026 04:08
ff53a21 to
ff11d5c
Compare
zzylol
force-pushed
the
stack/528-12-latency
branch
from
October 3, 2026 04:08
ec66d76 to
cc8794b
Compare
This was referenced Oct 3, 2026
zzylol
marked this pull request as draft
October 3, 2026 19:28
Contributor
Author
|
Parked as draft: PR priorities changed (see #528). Order is now (A) finish #511 operator sharing, (B) the #572 crate/module reorganization, (C) #509 end-to-end stages. This PR sits on the old 🤖 Generated with Claude Code |
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.
Problem: query latency bounds are never checked against summary lifecycle alternatives
#509 §3 Plan selection:
#509 §Goal lists latency requirements as part of the query workload, and Example 1 (Stage 3) applies them: "Its cost model estimates Q2's latency against the 100 ms bound; for example, an exact top 10 rebuilt from one million series at every refresh may miss it." #509 §Stages and their decisions also requires that every pruned candidate carries a reason.
Before this PR, the workload already carried the bound, but nothing read it during planning:
CostModelreturns abstractCost/CostRatevalues and had no latency hook. So for this workload:quantile_over_time(0.99, lat[5m])a deployment whose ephemeral KLL takes 250 ms per read (rebuilt from raw samples) and whose continuously maintained KLL takes 50 ms had no way to say so. The ephemeral alternative stayed selectable whenever it was cheaper, and the 100 ms bound had no effect on the plan.
Scope. This PR covers the latency-bound part of #509 §3 for summary lifecycle alternatives (part 1 of #526). It leaves out: a latency check on raw recomputation (#556), and recording reasons for valid candidates that lose selection (part 2 of #526).
Proposed method
All changes are in the lifecycle stage, which chooses a summary's materialization lifecycle and feeds selection (
crates/asap-aware-mapping/src/summary_maintenance_lifecycle.rs).CostModel::summary_read_latency_ms(summary, lifecycle) -> Option<f64>. It is the deployment's estimate of response latency for readingsummaryunder one physical lifecycle. The default returnsNone(no estimate).workload_factsalready walks the workload entries that consume a summary. It now also keeps the minimumExplicitMaxMsbound over those entries, in the new fieldSummaryMaintenanceWorkloadFacts.latency_bound_ms.Unspecifiedentries add nothing. If no entry has a bound, the field isNoneand no latency check runs.enumerate_with_profile, afteralternatives_forbuilds the alternatives for one summary, and only when a bound exists, call the hook for each alternative's lifecycle:>= 0and> bound: if the alternative has no rejection yet, setrejection = ExceedsLatencyBoundand push the assumption"estimated response latency {latency_ms} ms exceeds {bound_ms} ms bound".>= 0and<= bound: no change.None, NaN, infinite or negative: keep the alternative and push the assumption"latency bound {bound_ms} ms unchecked: no estimate".selectable()requiresrejection.is_none()and a known cost), so selection moves to the next legal lifecycle. An existing rejection is never overwritten, so the first reason stays.The check is per lifecycle alternative, not per logical candidate, because the same summary can be fast when maintained and slow when rebuilt per read.
Key code interfaces
crates/asap-aware-mapping/src/cost_model.rscrates/asap-aware-mapping/src/summary_maintenance_lifecycle.rsThe result is visible on the existing public type:
Fields
CostModel::summary_read_latency_mssummary&OperatorNodeSummaryAggwhose read is estimatedlifecycle&SummaryMaintenanceLifecycleEphemeral,Prepared { activate_at, retire_at },Shared { retention }orContinuouslyMaintainedOption<f64>None= no estimate. Only finite values>= 0are compared; anything else is treated as no estimate.Implemented by the deployment's cost model. The default (and
DefaultCostModel) returnsNone.SummaryMaintenanceLifecycleRejection::ExceedsLatencyBound: the deployment's estimate for this lifecycle is larger than the strictest bound among the summary's consumers. Serialized as"exceeds_latency_bound"(rename_all = "snake_case"). Set only by the latency check, and only on an alternative that had no rejection.SummaryMaintenanceWorkloadFacts.latency_bound_ms(private):Option<f64>, the minimumLatencyRequirement::ExplicitMaxMsover the workload entries inworkload_entry_indicesfor this summary.Nonewhen every consumer isUnspecified. Set byworkload_facts.SummaryMaintenanceLifecycleAlternativefields as used here:rejection:Nonemeans legal;Some(ExceedsLatencyBound)is the new reason.assumptions: gets one latency string per checked alternative (either the "exceeds" text with the estimate, or the "unchecked: no estimate" text).total_cost,summary_maintenance_lifecycle: unchanged.Examples
Slow ephemeral loses to fast maintained. Test
latency_bound_rejects_slow_ephemeral_summary_and_keeps_fast_maintained_oneincrates/planner/tests/summary_sharing.rs:quantile_over_time(0.99, lat[5m]), ε = 0.01,response_latency = ExplicitMaxMs(100.0). Test cost modelFixedCosts { build: 1.0, raw_per_read: 1_000.0, latency_estimates: true }, whose hook returns 250 ms forEphemeraland 50 ms for every other lifecycle.Ephemeral(250 > 100) is rejected. The other lifecycles (50 ≤ 100) are left as they are.Ephemeralwithrejection == Some(ExceedsLatencyBound)andContinuouslyMaintainedwithrejection == None.No estimate keeps the candidate. Test
missing_latency_estimate_keeps_candidate_and_records_unchecked_reason, same file:CHEAP_SUMMARY(latency_estimates: false, so the hook returnsNone).rejection == Noneand an assumption containing"latency bound 100 ms unchecked".Unspecified)ExceedsLatencyBound+ "exceeds" assumptionUnsupportedByRuntime)None, NaN, ∞ or negativeThe existing tests that build
FixedCostssetlatency_estimates: false, so their results do not change.Out of scope
DefaultCostModelreturnsNone, so without a deployment model the bound is recorded as unchecked.Stack and validation
Legacy physical stack: … ← #553 ← #554 ← #555 ← #556 … · Base: #553 (
stack/528-11-deployment-inputs) · Next: #555 · Tracker: #528Implements the latency part of #526. Selection-loss explanations remain a separate follow-up.
Validation:
CARGO_TARGET_DIR=/mydata/cargo-target-412 cargo test --locked -p asap-plannercargo fmt --all -- --check🤖 Generated with Claude Code