diff --git a/control_plane/src/sketch_algebra/cost_model.rs b/control_plane/src/sketch_algebra/cost_model.rs index 0cf7ab21..fa4d4ff5 100644 --- a/control_plane/src/sketch_algebra/cost_model.rs +++ b/control_plane/src/sketch_algebra/cost_model.rs @@ -454,7 +454,7 @@ impl CostModel for ForcedFamilyCostModel { /// observed for the candidates on offer. /// /// This is the seam `data_plane`'s live-serving re-binding path -/// (`l4_lowering.rs`) needs: planning already decided a family + params +/// (`post_asap_planner.rs`) needs: planning already decided a family + params /// for a metric (that decision is what's actually registered in the /// `SketchStore`), so serving-time re-parsing the same query must /// reproduce EXACTLY that plan, not size a fresh one from a guessed @@ -469,7 +469,7 @@ impl CostModel for ForcedFamilyCostModel { /// (`find_candidates` finds nothing) is unchanged. /// /// **Fallback status (design-backend-plan-wire-format.md §5):** -/// `l4_lowering.rs` prefers reading planning's decision directly off an +/// `post_asap_planner.rs` prefers reading planning's decision directly off an /// installed `BackendPlan`'s materializations (no reconstruction needed /// there — `Materialization.kind`/`.params` already ARE the pair /// `observed` needs). This type's caller diff --git a/control_plane/src/sketch_algebra/lower.rs b/control_plane/src/sketch_algebra/lower.rs index 868a2fa6..e53825eb 100644 --- a/control_plane/src/sketch_algebra/lower.rs +++ b/control_plane/src/sketch_algebra/lower.rs @@ -68,7 +68,7 @@ pub fn bind_query_expr( /// instead of the default planning-time [`ControlPlaneCostModel`]. /// /// This is the seam serving-time re-binding needs: `data_plane`'s -/// live-serving path (`l4_lowering.rs`) must NOT re-derive a family/params +/// live-serving path (`post_asap_planner.rs`) must NOT re-derive family/params /// choice independently of what was actually planned — it looks up what's /// really registered in the `SketchStore` and hands in a cost model that /// echoes that back, so the resulting `SummaryNode` matches reality by diff --git a/data_plane/src/query_engines/asap_query_engine/engine.rs b/data_plane/src/query_engines/asap_query_engine/engine.rs index d370a667..db2a1eac 100644 --- a/data_plane/src/query_engines/asap_query_engine/engine.rs +++ b/data_plane/src/query_engines/asap_query_engine/engine.rs @@ -57,7 +57,7 @@ pub struct ASAPQueryEngine { archive_engine: Option>, /// BackendPlan wire format (design-backend-plan-wire-format.md). When - /// `Some`, `l4_lowering.rs`'s serving-time family/params lookup + /// `Some`, `post_asap_planner.rs`'s serving-time family/params lookup /// prefers reading the installed plan's materializations directly /// over reconstructing from `SketchStore` metadata /// (`ObservedFamilyCostModel`). `None` when not wired up (unit diff --git a/data_plane/src/query_engines/asap_query_engine/live_serve.rs b/data_plane/src/query_engines/asap_query_engine/live_serve.rs index b8db37be..ca3d0e3a 100644 --- a/data_plane/src/query_engines/asap_query_engine/live_serve.rs +++ b/data_plane/src/query_engines/asap_query_engine/live_serve.rs @@ -20,11 +20,11 @@ use control_plane::types_v2::AccuracyTarget; -use crate::query_engines::asap_query_engine::l4_readout::execute_l4_readout; +use crate::query_engines::asap_query_engine::post_asap_readout::execute_post_asap_readout; use crate::storage_engines::sketch_db::index::SketchStore; use crate::storage_engines::sketch_db::query::ASAPTierResult; -/// Fallback accuracy target used only when `l4_lowering.rs`'s +/// Fallback accuracy target used only when `post_asap_planner.rs`'s /// observed-family lookup finds nothing registered for the query's /// metric (in which case no family/params choice here can matter — the /// query can't be served either way). `data_plane` doesn't carry a @@ -66,7 +66,7 @@ pub fn summary_executor_live_enabled() -> bool { /// (ASAPController#163). That gate is gone — `Reduction` /// (ASAPController#165) lets `summary_executor.rs` resolve both halves of /// the ambiguity correctly on its own, so there is no longer a shape to -/// decline. See `L4ReadoutOutcome`'s doc for the full reasoning. +/// decline. See `PostAsapReadoutOutcome`'s doc for the full reasoning. pub fn try_serve_from_summary_executor( index: &SketchStore, query: &str, @@ -79,7 +79,7 @@ pub fn try_serve_from_summary_executor( return None; } - let outcome = match execute_l4_readout( + let outcome = match execute_post_asap_readout( index, query, t0_ms, diff --git a/data_plane/src/query_engines/asap_query_engine/mod.rs b/data_plane/src/query_engines/asap_query_engine/mod.rs index 736abb53..ba046bf9 100644 --- a/data_plane/src/query_engines/asap_query_engine/mod.rs +++ b/data_plane/src/query_engines/asap_query_engine/mod.rs @@ -10,9 +10,9 @@ //! the JSONL leg has been deleted). pub mod engine; -pub mod l4_lowering; -pub mod l4_readout; pub mod live_serve; +pub mod post_asap_planner; +pub mod post_asap_readout; pub mod summary_exec; pub mod summary_executor; diff --git a/data_plane/src/query_engines/asap_query_engine/l4_lowering.rs b/data_plane/src/query_engines/asap_query_engine/post_asap_planner.rs similarity index 93% rename from data_plane/src/query_engines/asap_query_engine/l4_lowering.rs rename to data_plane/src/query_engines/asap_query_engine/post_asap_planner.rs index b301df35..ab07579b 100644 --- a/data_plane/src/query_engines/asap_query_engine/l4_lowering.rs +++ b/data_plane/src/query_engines/asap_query_engine/post_asap_planner.rs @@ -1,7 +1,5 @@ //! PromQL string → `planner_types::post_asap::SummaryNode` bridge, shared by the actual -//! serving cutover (`live_serve.rs`, via `l4_readout.rs`). See -//! `data_plane/docs/l4node-plan-executor-design.md`'s "Rollout" section -//! for the general design. +//! serving cutover (`live_serve.rs`, via `post_asap_readout.rs`). //! //! `control_plane` runs in-process with `data_plane` in this deployment //! (see `data_plane/Cargo.toml`'s "Phase 9" comment), so this is a @@ -10,10 +8,10 @@ //! //! ## Serving time must not re-plan //! -//! `parse_query_expr_canonical` (L1→L2→L3) is safe to re-run at serving -//! time — it's a pure, deterministic canonicalization of the query text, -//! not a decision. Binding L3→L4 (which sketch family, what parameters) -//! is a genuine PLANNING decision, and planning already made it once, for +//! Parsing and canonicalization are safe to re-run at serving time: they are +//! pure, deterministic transformations of the query text. Selecting the +//! post-ASAP implementation (which summary family and parameters to use) is +//! a genuine planning decision, and planning already made it once, for //! real, when this metric's workload was planned — that decision is what //! `data_plane`'s ingest path actually registered in the `SketchStore` //! (`AggKind::Sketch { kind, config, .. }`). Serving time must reproduce @@ -27,7 +25,7 @@ //! chose strict equality over silently serving an answer under a looser //! guarantee than what was planned). //! -//! So before binding, [`lower_promql_to_l4node`] looks up what's actually +//! So before binding, [`plan_promql_to_post_asap`] looks up what's actually //! registered for the query's target metric and constructs an //! [`ObservedFamilyCostModel`] that echoes that back — the resulting //! `SummaryNode` matches reality by construction, not by a coincidental @@ -54,8 +52,8 @@ use crate::storage_engines::sketch_db::data::{AggKind, SketchConfig}; use crate::storage_engines::sketch_db::index::SketchStore; /// Why a query couldn't be answered through the `SummaryNode`/`SummaryExecutor` -/// path — covers both `lower_promql_to_l4node`'s own failure to produce a -/// tree, AND (via `l4_readout.rs`'s `execute_l4_readout`) a failure of +/// path — covers both `plan_promql_to_post_asap`'s own failure to produce a +/// tree, AND (via `post_asap_readout.rs`'s `execute_post_asap_readout`) a failure of /// `crate::query_engines::asap_query_engine::summary_exec::execute()` on a tree that DID lower successfully. /// None of these are errors in the alarming sense — every variant is an /// expected, frequent outcome for *some* fraction of live traffic; the @@ -77,10 +75,10 @@ pub enum LoweringSkip { /// calling into `control_plane`'s binder, not just deprioritized. RateShape, /// `bind_query_expr` itself failed (a genuine `BindingError`, e.g. - /// L3→L4 schema-derivation failure). + /// post-ASAP implementation/schema-derivation failure). Implement(String), /// `bind_query_expr` returned a `PhysicalExpr` variant other than - /// `Committed(L4Plan::Summary(_))`. Per `bind_query_expr`'s own doc + /// a committed post-ASAP summary. Per `bind_query_expr`'s own doc /// this shouldn't happen in practice (it never picks a Phase ε.1 /// placement), but the match is kept exhaustive and defensive rather /// than assuming. @@ -105,6 +103,10 @@ pub enum LoweringSkip { ExecuteFailed(String), } +// `L4Plan` and `PhysicalExpr` are backend compatibility/placement wrappers. +// The semantic tree returned by ASAPPlanner is `post_asap::SummaryNode`; this +// module does not claim or recreate an ASAPPlanner "L4" IR. + /// Map a registered sid's `(SketchKindHandle, SketchConfig)` — the /// durable record of what planning actually decided for this metric — to /// the `(SketchAlgorithm, SketchParams)` pair `ObservedFamilyCostModel` @@ -233,7 +235,7 @@ fn observed_family_for_metric_from_plan( /// shouldn't attempt (parse failure, `rate()`, or anything that doesn't /// realize to a concrete sketch/exact-agg binding) — see /// `LoweringSkip`'s variants. -pub fn lower_promql_to_l4node( +pub fn plan_promql_to_post_asap( index: &SketchStore, query: &str, accuracy: AccuracyTarget, @@ -314,7 +316,7 @@ mod tests { fn rate_query_is_skipped_before_binding() { let idx = empty_index(); let result = - lower_promql_to_l4node(&idx, "rate(http_requests_total[5m])", accuracy(), None); + plan_promql_to_post_asap(&idx, "rate(http_requests_total[5m])", accuracy(), None); assert!( matches!(result, Err(LoweringSkip::RateShape)), "expected RateShape, got {result:?}" @@ -325,7 +327,7 @@ mod tests { fn irate_query_is_skipped_before_binding() { let idx = empty_index(); let result = - lower_promql_to_l4node(&idx, "irate(http_requests_total[5m])", accuracy(), None); + plan_promql_to_post_asap(&idx, "irate(http_requests_total[5m])", accuracy(), None); assert!( matches!(result, Err(LoweringSkip::RateShape)), "expected RateShape, got {result:?}" @@ -335,7 +337,7 @@ mod tests { #[test] fn unparseable_query_is_skipped() { let idx = empty_index(); - let result = lower_promql_to_l4node(&idx, "this is not promql (((", accuracy(), None); + let result = plan_promql_to_post_asap(&idx, "this is not promql (((", accuracy(), None); assert!( matches!(result, Err(LoweringSkip::ParseFailed(_))), "expected ParseFailed, got {result:?}" @@ -355,7 +357,7 @@ mod tests { // expression stays one opaque `Logical` blob, which this module // surfaces as `NotRealized`. let idx = empty_index(); - let result = lower_promql_to_l4node(&idx, "http_requests_total", accuracy(), None); + let result = plan_promql_to_post_asap(&idx, "http_requests_total", accuracy(), None); assert!( matches!(result, Err(LoweringSkip::NotRealized)), "expected NotRealized, got {result:?}" @@ -376,7 +378,7 @@ mod tests { // falls back to the accuracy-driven default -- same outcome as // before this module started consulting the `SketchStore`. let idx = empty_index(); - let node = lower_promql_to_l4node( + let node = plan_promql_to_post_asap( &idx, "count_over_time(http_requests_total[5m])", accuracy(), @@ -398,7 +400,7 @@ mod tests { // one opaque `Logical` blob -- self-excludes via `NotRealized`, // no special-case detection needed for this shape specifically. let idx = empty_index(); - let result = lower_promql_to_l4node( + let result = plan_promql_to_post_asap( &idx, "topk(5, sum by (host) (rate(http_requests_total[5m])))", accuracy(), @@ -506,7 +508,7 @@ mod tests { let idx = SketchStore::new(); register_kll(&idx, "m"); let node = - lower_promql_to_l4node(&idx, "quantile_over_time(0.99, m[1m])", accuracy(), None) + plan_promql_to_post_asap(&idx, "quantile_over_time(0.99, m[1m])", accuracy(), None) .expect("should lower"); assert_eq!(bound_family(&node).0, SketchAlgorithm::Kll); } @@ -522,7 +524,7 @@ mod tests { let idx = SketchStore::new(); register_kll(&idx, "m"); let plan = plan_with_ddsketch_materialization("m"); - let node = lower_promql_to_l4node( + let node = plan_promql_to_post_asap( &idx, "quantile_over_time(0.99, m[1m])", accuracy(), @@ -545,7 +547,7 @@ mod tests { let idx = SketchStore::new(); register_kll(&idx, "m"); let plan = plan_with_ddsketch_materialization("some_other_metric"); - let node = lower_promql_to_l4node( + let node = plan_promql_to_post_asap( &idx, "quantile_over_time(0.99, m[1m])", accuracy(), diff --git a/data_plane/src/query_engines/asap_query_engine/l4_readout.rs b/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs similarity index 94% rename from data_plane/src/query_engines/asap_query_engine/l4_readout.rs rename to data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs index cd1a2c04..9fc8b6c7 100644 --- a/data_plane/src/query_engines/asap_query_engine/l4_readout.rs +++ b/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs @@ -1,14 +1,15 @@ //! `SummaryNode` lowering + execution + conversion into `ASAPTierResult`'s //! `(series, coverage)` shape — the core `live_serve.rs` (the serving -//! cutover) calls into. See -//! `data_plane/docs/l4node-plan-executor-design.md` for the design. +//! cutover) calls into. use std::collections::BTreeMap; use crate::query_engines::asap_query_engine::summary_exec::{execute, ExecOutcome}; use control_plane::types_v2::AccuracyTarget; -use crate::query_engines::asap_query_engine::l4_lowering::{lower_promql_to_l4node, LoweringSkip}; +use crate::query_engines::asap_query_engine::post_asap_planner::{ + plan_promql_to_post_asap, LoweringSkip, +}; use crate::query_engines::asap_query_engine::summary_executor::{ QueryExecutionContext, SummaryValue, }; @@ -51,17 +52,18 @@ pub type SeriesRows = Vec<(BTreeMap, Vec<(i64, f64)>)>; /// The flag would therefore be unconditionally `false` today; keeping it /// would mean keeping a heuristic that can only ever misfire (declining /// correct `PerEntity` answers) now that the real signal is available. -pub struct L4ReadoutOutcome { +pub struct PostAsapReadoutOutcome { pub series: SeriesRows, pub coverage: Option<(u64, u64)>, } -/// Lower `query`, execute it against `index` over `[t0_ms, t1_ms]`, and -/// convert the result into `L4ReadoutOutcome`. `Err` covers every reason +/// Ask ASAPPlanner for the post-ASAP representation of `query`, execute it +/// against `index` over `[t0_ms, t1_ms]`, and +/// convert the result into `PostAsapReadoutOutcome`. `Err` covers every reason /// this couldn't produce a trustworthy answer — see `LoweringSkip`'s /// variants; every one of them means "fall back to the legacy path," /// never "the legacy path is wrong." -pub fn execute_l4_readout( +pub fn execute_post_asap_readout( index: &SketchStore, query: &str, t0_ms: u64, @@ -69,8 +71,8 @@ pub fn execute_l4_readout( is_cumulative: bool, accuracy: AccuracyTarget, backend_plan: Option<&control_plane::backend_plan::BackendPlan>, -) -> Result { - let node = lower_promql_to_l4node(index, query, accuracy, backend_plan)?; +) -> Result { + let node = plan_promql_to_post_asap(index, query, accuracy, backend_plan)?; let ctx = QueryExecutionContext { index, @@ -87,7 +89,7 @@ pub fn execute_l4_readout( fold_coverage(&mut coverage, value.coverage()); series.extend(summary_value_to_series(group_key, value)); } - Ok(L4ReadoutOutcome { series, coverage }) + Ok(PostAsapReadoutOutcome { series, coverage }) } Ok(ExecOutcome::State(groups)) => { let mut coverage: Option<(u64, u64)> = None; @@ -99,7 +101,7 @@ pub fn execute_l4_readout( }; series.push((group_key.clone(), vec![(t1_ms as i64, value)])); } - Ok(L4ReadoutOutcome { series, coverage }) + Ok(PostAsapReadoutOutcome { series, coverage }) } Err(e) => Err(LoweringSkip::ExecuteFailed(format!("{e:?}"))), } @@ -246,7 +248,7 @@ mod tests { #[test] fn bare_range_function_keeps_one_series_per_entity() { let idx = ddsketch_fixture(); - let outcome = execute_l4_readout( + let outcome = execute_post_asap_readout( &idx, "quantile_over_time(0.99, latency_ms[1m])", 1_000, @@ -277,7 +279,7 @@ mod tests { let idx = SketchStore::new(); register_hll(&idx, 1, "svc-a", &["a", "b", "c"]); register_hll(&idx, 2, "svc-b", &["d", "e", "f"]); - let outcome = execute_l4_readout( + let outcome = execute_post_asap_readout( &idx, "count(unique_users)", 1_000, @@ -330,7 +332,7 @@ mod tests { (1_000, 2_000), Box::new(crate::precompute_engine::operators::SumAccumulator::with_sum(42.0)), ); - let outcome = execute_l4_readout( + let outcome = execute_post_asap_readout( &idx, "sum(bytes_total)", 1_000, diff --git a/data_plane/src/query_engines/asap_query_engine/summary_executor.rs b/data_plane/src/query_engines/asap_query_engine/summary_executor.rs index c78859f9..b648fc6a 100644 --- a/data_plane/src/query_engines/asap_query_engine/summary_executor.rs +++ b/data_plane/src/query_engines/asap_query_engine/summary_executor.rs @@ -265,7 +265,7 @@ pub enum SummaryExecutorError { UnsupportedFamily, /// Decode/merge failure surfaced from `delta_apply`/`asap_sketchlib`. Decode(String), - /// A `SummaryExpr::Logical` node — nothing committed at L4. Same + /// A `SummaryExpr::KeepPreAsap` node — no maintained summary was selected. Same /// meaning as today's "no candidate bound"; the caller fails over. Logical, /// A query shape not covered by this executor — see the module doc. @@ -836,7 +836,7 @@ fn project_group_key( } /// Group-key construction for `find_candidates`, shared by both the -/// `Sketch` and `ExactAgg` branches -- driven directly by L3/L4's own +/// `Sketch` and `ExactAgg` branches -- driven directly by the post-ASAP IR's /// `Reduction` (ASAPController#163/#164/#165), not inferred from whether /// `by` happens to be empty. /// @@ -845,7 +845,7 @@ fn project_group_key( /// `SummaryAgg`, an empty `by: Vec` was genuinely ambiguous -- /// it could mean either "no explicit grouping was even resolvable" (a /// bare per-series range function like `quantile_over_time(0.99, -/// http_latency_ms[10s])`, where L3/L4 planning has no reference to any +/// http_latency_ms[10s])`, where the post-ASAP plan has no reference to any /// label column at all) or "a real cross-series reduction with zero /// grouping columns" (`count(hll_metric)`, `sum(...)`-shaped). Those two /// cases need OPPOSITE group-key behavior and the old `by: &[ColumnId]` @@ -953,7 +953,7 @@ fn find_metric(node: &SummaryNode) -> Option { /// Walk a canonical `QueryExpr` down to its first `Scan { /// source: Source::TimeSeries { metric }, .. }` to recover the target -/// metric name. Shared with `l4_lowering.rs`'s observed-family lookup +/// metric name. Shared with `post_asap_planner.rs`'s observed-family lookup /// (serving time must know which metric to check the `SketchStore` /// against BEFORE binding — see that module's docs). pub(crate) fn find_metric_in_query_expr(qe: &QueryExpr) -> Option { diff --git a/data_plane/src/storage_engines/sketch_db/query/asap_tier_result.rs b/data_plane/src/storage_engines/sketch_db/query/asap_tier_result.rs index 47131ff7..8f2fb33d 100644 --- a/data_plane/src/storage_engines/sketch_db/query/asap_tier_result.rs +++ b/data_plane/src/storage_engines/sketch_db/query/asap_tier_result.rs @@ -1,5 +1,5 @@ //! `ASAPTierResult` — the per-series, per-window scalar-result shape -//! `SummaryExecutor` (via `live_serve.rs`/`l4_readout.rs`) fills in for +//! `SummaryExecutor` (via `live_serve.rs`/`post_asap_readout.rs`) fills in for //! the engine to adapt into `QueryResult`. //! //! This module used to also hold `SketchReducer`, the legacy per-Capability diff --git a/data_plane/src/storage_engines/sketch_db/query/mod.rs b/data_plane/src/storage_engines/sketch_db/query/mod.rs index ae01179a..e0b15c00 100644 --- a/data_plane/src/storage_engines/sketch_db/query/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/query/mod.rs @@ -29,7 +29,7 @@ //! * [`ASAPTierResult`] — per-series timestamped scalar samples //! matching the shape of [`crate::query_engines::query_result::QueryResult::Matrix`], //! filled in by `SummaryExecutor` (via -//! `asap_query_engine::live_serve`/`l4_readout`) — the sole +//! `asap_query_engine::live_serve`/`post_asap_readout`) — the sole //! sketch-serving path; the legacy `SketchReducer` this module used //! to also hold is retired (see `asap_tier_result.rs`'s doc). //! diff --git a/data_plane/tests/e2e_controller_plans_and_backend_serves.rs b/data_plane/tests/e2e_controller_plans_and_backend_serves.rs index de593b1d..8898c4e0 100644 --- a/data_plane/tests/e2e_controller_plans_and_backend_serves.rs +++ b/data_plane/tests/e2e_controller_plans_and_backend_serves.rs @@ -1115,7 +1115,7 @@ async fn controller_really_pushes_backend_plan_and_query_really_serves() { // ── 2. Confirm the backend really installed it (not just accepted the // POST) — read it back via GET, the same handle - // `l4_lowering.rs`'s serving-time lookup reads from. ── + // `post_asap_planner.rs`'s serving-time lookup reads from. ── let installed = get_backend_plan(&client, stack.backend_port).await; assert_eq!(installed["status"], "success"); assert_eq!(installed["plan_id"], 1);