Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions control_plane/src/sketch_algebra/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion control_plane/src/sketch_algebra/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion data_plane/src/query_engines/asap_query_engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub struct ASAPQueryEngine {
archive_engine:
Option<Arc<dyn crate::query_engines::routing::query_engine_routing::QueryEngine>>,
/// 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
Expand Down
8 changes: 4 additions & 4 deletions data_plane/src/query_engines/asap_query_engine/live_serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions data_plane/src/query_engines/asap_query_engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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`
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:?}"
Expand All @@ -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:?}"
Expand All @@ -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:?}"
Expand All @@ -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:?}"
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down Expand Up @@ -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);
}
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -51,26 +52,27 @@ pub type SeriesRows = Vec<(BTreeMap<String, String>, 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,
t1_ms: u64,
is_cumulative: bool,
accuracy: AccuracyTarget,
backend_plan: Option<&control_plane::backend_plan::BackendPlan>,
) -> Result<L4ReadoutOutcome, LoweringSkip> {
let node = lower_promql_to_l4node(index, query, accuracy, backend_plan)?;
) -> Result<PostAsapReadoutOutcome, LoweringSkip> {
let node = plan_promql_to_post_asap(index, query, accuracy, backend_plan)?;

let ctx = QueryExecutionContext {
index,
Expand All @@ -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;
Expand All @@ -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:?}"))),
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
///
Expand All @@ -845,7 +845,7 @@ fn project_group_key(
/// `SummaryAgg`, an empty `by: Vec<ColumnId>` 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]`
Expand Down Expand Up @@ -953,7 +953,7 @@ fn find_metric(node: &SummaryNode) -> Option<String> {

/// 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<String> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion data_plane/src/storage_engines/sketch_db/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
//!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading