From 1dcf299fbc8c1f86d37e934317d271ac164ee000 Mon Sep 17 00:00:00 2001 From: zz_y Date: Sat, 12 Sep 2026 13:17:08 -0600 Subject: [PATCH 1/8] Maintain shared current-series quantile and TopK state --- control_plane/src/main.rs | 1 + control_plane/src/physical/compiler.rs | 84 ++- control_plane/src/physical/current_series.rs | 143 +++++ control_plane/src/physical/mod.rs | 2 + control_plane/src/physical/workload_cost.rs | 33 ++ crates/asap_types/src/query_plan.rs | 1 + .../src/query_plan/current_series.rs | 49 ++ crates/asap_types/src/query_plan/logical.rs | 26 +- .../drivers/ingest/prometheus_remote_write.rs | 10 +- data_plane/src/drivers/query/servers/http.rs | 9 + .../query_engines/asap_query_engine/engine.rs | 42 +- .../asap_query_engine/logical_dag.rs | 13 + .../sketch_db/current_series.rs | 537 ++++++++++++++++++ .../storage_engines/sketch_db/index/mod.rs | 1 + .../src/storage_engines/sketch_db/mod.rs | 2 + .../asapquery_compatibility_process_e2e.rs | 3 + .../tests/support/current_series_process.rs | 305 ++++++++++ .../current-series-aggregations.md | 63 ++ .../query-engine/query-engine.md | 3 + 19 files changed, 1321 insertions(+), 6 deletions(-) create mode 100644 control_plane/src/physical/current_series.rs create mode 100644 crates/asap_types/src/query_plan/current_series.rs create mode 100644 data_plane/src/storage_engines/sketch_db/current_series.rs create mode 100644 data_plane/tests/support/current_series_process.rs create mode 100644 docs/developer_docs/query-engine/current-series-aggregations.md diff --git a/control_plane/src/main.rs b/control_plane/src/main.rs index d0f0bc0f..6480d5bb 100644 --- a/control_plane/src/main.rs +++ b/control_plane/src/main.rs @@ -973,6 +973,7 @@ fn compile_physical_plan_request( }; let planning_request = physical::compiler::PlanningRequest { + current_series: false, logical_selection, query_workload: None, queries, diff --git a/control_plane/src/physical/compiler.rs b/control_plane/src/physical/compiler.rs index 1fb5fada..51300a45 100644 --- a/control_plane/src/physical/compiler.rs +++ b/control_plane/src/physical/compiler.rs @@ -139,6 +139,8 @@ pub struct LifecyclePlanningInput { #[derive(Debug, Clone, Default)] pub struct PlanningRequest { + /// Exact current-value state is a separately priced physical alternative. + pub current_series: bool, /// Diagnostic projections of the original Planner search; never consumed by selection. pub logical_selection: Vec, /// Enable a composable DAG with SummaryStore materializations and Prometheus exact subtrees. @@ -681,6 +683,7 @@ impl BackendLocalPlanningSnapshot { // Composable lowering residualizes unsafe leaves individually; retain Planner siblings. Ok(( PlanningRequest { + current_series: false, logical_selection, hybrid_execution: true, materialization_policy: None, @@ -915,6 +918,14 @@ impl PhysicalCompiler { environment: DeploymentEnvironment, metricsql: bool, ) -> Result { + if request.current_series + && (metricsql + || environment.target != PhysicalDeploymentTarget::BackendLocalRemoteWrite) + { + return Err(CompileError::Snapshot( + "current-series maintenance requires backend-local PromQL deployment".into(), + )); + } if request.hybrid_execution && environment.target != PhysicalDeploymentTarget::BackendLocalRemoteWrite { @@ -1421,11 +1432,12 @@ impl PhysicalCompiler { } } - let plan_id = if request.hybrid_execution { + let plan_id = if request.hybrid_execution || request.current_series { use std::hash::{Hash, Hasher}; let mut hash = std::collections::hash_map::DefaultHasher::new(); stable_workload_plan_id(&plan_materializations, &request.queries).hash(&mut hash); "typed-local-residual-v3-counter-index".hash(&mut hash); + request.current_series.hash(&mut hash); request.materialization_policy.hash(&mut hash); for query in &request.queries { format!("{:?}", query.post_asap).hash(&mut hash); @@ -1597,6 +1609,18 @@ impl PhysicalCompiler { if metricsql { entry.language = crate::query_plan::QueryLanguage::MetricsQl; } + if request.current_series && !metricsql { + if let Some(operator) = super::current_series::operator(&request, query)? { + entry.root = crate::query_plan::QueryNodeId(0); + entry.nodes = BTreeMap::from([( + entry.root, + crate::query_plan::QueryPlanNode::Logical { + operator, + inputs: vec![], + }, + )]); + } + } let catalog_key = QueryPlan::catalog_key(entry.language, &canonical); if query_entries.insert(catalog_key, entry).is_some() { return Err(CompileError::Query { @@ -3240,6 +3264,63 @@ fn stable_workload_plan_id( mod tests { use super::*; + /// Multiple current-value quantiles and TopK limits share one maintained population. + #[test] + fn current_series_quantiles_and_topk_have_a_shared_executable_candidate() { + let mut snapshot: BackendLocalPlanningSnapshot = serde_json::from_str(include_str!( + "../../../docs/examples/asapquery-planning-snapshot.json" + )) + .unwrap(); + snapshot.snapshot_version = 2; + let template = snapshot.query_workload.repeating_queries.as_ref().unwrap()[0].clone(); + let queries = [ + "quantile by (job) (0.5, a)", + "quantile by (job) (0.9, a)", + "quantile by (job) (0.95, a)", + "quantile by (job) (0.99, a)", + "topk by (job) (1, a)", + "topk by (job) (5, a)", + ]; + snapshot.query_workload.repeating_queries = Some( + queries + .iter() + .map(|q| { + let mut entry = template.clone(); + entry.query = planner_types::workload::Query((*q).into()); + entry + }) + .collect(), + ); + let (request, environment) = snapshot.planning_request().unwrap(); + let plans: Vec<_> = super::super::workload_cost::with_exact_alternative(request) + .unwrap() + .into_iter() + .filter_map(|r| PhysicalCompiler.compile(r, environment.clone()).ok()) + .collect(); + let plan = plans.iter().find(|plan| plan.query_plan.entries.values().all(|entry| + entry.nodes.values().any(|node| matches!(node, crate::query_plan::QueryPlanNode::Logical { + operator: crate::query_plan::logical::LogicalOperator::CurrentSeries { .. }, .. + })))).expect("no shared current-series candidate"); + let mut populations = BTreeSet::new(); + for entry in plan.query_plan.entries.values() { + for node in entry.nodes.values() { + if let crate::query_plan::QueryPlanNode::Logical { + operator: + crate::query_plan::logical::LogicalOperator::CurrentSeries { + population, .. + }, + .. + } = node + { + assert_eq!(population.max_k, 5); + assert!(population.quantiles); + populations.insert(population.key()); + } + } + } + assert_eq!(populations.len(), 1); + } + #[test] fn installed_partition_must_match_the_bound_dag_reduction() { let mut env = environment(10_000); @@ -3727,6 +3808,7 @@ mod tests { evidence_by_query.insert(query_id.to_string(), evidence); } Ok(PlanningRequest { + current_series: false, logical_selection: Vec::new(), hybrid_execution: false, materialization_policy: None, diff --git a/control_plane/src/physical/current_series.rs b/control_plane/src/physical/current_series.rs new file mode 100644 index 00000000..d05d8593 --- /dev/null +++ b/control_plane/src/physical/current_series.rs @@ -0,0 +1,143 @@ +//! Bind exact current-value aggregations without treating historical samples as a population. +use super::compiler::{CompileError, PlanningQuery, PlanningRequest}; +use asap_types::query_plan::{ + current_series::{SeriesPopulation, SeriesReadout}, + logical::{Grouping, LabelMatch, LabelMatcher, LogicalOperator}, +}; +use promql_parser::{ + label::MatchOp, + parser::{self, Expr, LabelModifier}, +}; + +fn parse(query: &str) -> Option<(SeriesPopulation, SeriesReadout)> { + let Expr::Aggregate(a) = parser::parse(query).ok()? else { + return None; + }; + let Expr::VectorSelector(selector) = a.expr.as_ref() else { + return None; + }; + if selector.offset.is_some() + || selector.at.is_some() + || !selector.matchers.or_matchers.is_empty() + { + return None; + } + let Expr::NumberLiteral(parameter) = a.param.as_deref()? else { + return None; + }; + if !parameter.val.is_finite() { + return None; + } + let readout = match a.op.to_string().as_str() { + "quantile" => SeriesReadout::Quantile { q: parameter.val }, + "topk" => SeriesReadout::TopK { + k: (parameter.val as i64).max(0) as u64, + }, + _ => return None, + }; + let mut grouping = match &a.modifier { + None => Grouping { + labels: vec![], + without: false, + }, + Some(LabelModifier::Include(labels)) => Grouping { + labels: labels.labels.clone(), + without: false, + }, + Some(LabelModifier::Exclude(labels)) => Grouping { + labels: labels.labels.clone(), + without: true, + }, + }; + grouping.labels.sort(); + grouping.labels.dedup(); + let mut matchers: Vec<_> = selector + .matchers + .matchers + .iter() + .map(|m| LabelMatcher { + name: m.name.clone(), + value: m.value.clone(), + operation: match m.op { + MatchOp::Equal => LabelMatch::Equal, + MatchOp::NotEqual => LabelMatch::NotEqual, + MatchOp::Re(_) => LabelMatch::Regex, + MatchOp::NotRe(_) => LabelMatch::NotRegex, + }, + }) + .collect(); + matchers.sort_by_key(|m| { + ( + m.name.clone(), + m.value.clone(), + format!("{:?}", m.operation), + ) + }); + Some(( + SeriesPopulation { + metric: selector.name.clone()?, + matchers, + grouping, + lookback_ms: 300_000, + max_input_lag_ms: 60_000, + max_series: 100_000, + max_bytes: 64 * 1024 * 1024, + max_k: 0, + quantiles: false, + }, + readout, + )) +} + +pub(super) fn supported(request: &PlanningRequest) -> bool { + request + .queries + .iter() + .any(|q| parse(&q.query_string).is_some()) +} + +pub(super) fn operator( + request: &PlanningRequest, + query: &PlanningQuery, +) -> Result, CompileError> { + let Some((mut population, readout)) = parse(&query.query_string) else { + return Ok(None); + }; + // Compare source/group semantics before adding workload-wide resource bounds. + for other in &request.queries { + if let Some((other_population, other_readout)) = parse(&other.query_string) { + let mut identity = population.clone(); + identity.max_k = 0; + identity.quantiles = false; + if identity == other_population { + match other_readout { + SeriesReadout::TopK { k } => population.max_k = population.max_k.max(k), + SeriesReadout::Quantile { .. } => population.quantiles = true, + } + } + } + } + let populations: std::collections::BTreeSet<_> = request + .queries + .iter() + .filter_map(|q| parse(&q.query_string).map(|(p, _)| p.key())) + .collect(); + population.max_bytes = request + .retained_summary_memory_budget_bytes + .unwrap_or(population.max_bytes) + .min(1_073_741_824) + / populations.len().max(1) as u64; + population.max_series = population + .max_series + .min((population.max_bytes / 1024) as usize); + population.max_input_lag_ms = request + .source_sample_interval_ms + .unwrap_or(60_000) + .saturating_add(request.query_staleness_margin_ms) + .clamp(1, 300_000); + population.validate()?; + Ok(Some(LogicalOperator::CurrentSeries { + population, + readout, + })) +} diff --git a/control_plane/src/physical/mod.rs b/control_plane/src/physical/mod.rs index 896594da..afcc253c 100644 --- a/control_plane/src/physical/mod.rs +++ b/control_plane/src/physical/mod.rs @@ -48,3 +48,5 @@ pub use allocator::SketchAllocator; pub use plan::{CostEstimate, ExecutionMode, PipelineStage, PlanNode, PlanSummary}; pub mod publication; + +pub(crate) mod current_series; diff --git a/control_plane/src/physical/workload_cost.rs b/control_plane/src/physical/workload_cost.rs index ecf98038..cc118ce4 100644 --- a/control_plane/src/physical/workload_cost.rs +++ b/control_plane/src/physical/workload_cost.rs @@ -233,6 +233,24 @@ pub fn manifest( // Typed local scans require retained input and ingest/update work even // when no precomputed summary is installed. Deduplicate by source. for node in entry.nodes.values() { + if let crate::query_plan::QueryPlanNode::Logical { + operator: + crate::query_plan::logical::LogicalOperator::CurrentSeries { population, .. }, + .. + } = node + { + let key = population.key(); + for phase in ["build", "update", "residency", "retire"] { + add( + format!("current-series:{key}:{phase}"), + json!({"population": population, "phase": phase}), + "horizon", + 1.0, + ); + } + let source = json!({"source": planner_types::pre_asap::Source::TimeSeries { metric: population.metric.clone() }, "location": "backend", "ingest": plan.precompute_plan.ingest}); + add(format!("source:{source}"), source, "horizon", 1.0); + } if matches!( node, crate::query_plan::QueryPlanNode::Logical { @@ -661,6 +679,21 @@ fn select_with_frontend( /// exact backend. Additional Planner-produced forests can use `select` directly. pub fn with_exact_alternative( request: PlanningRequest, +) -> Result, CompileError> { + let current = !request.current_series && super::current_series::supported(&request); + let mut alternatives = materialization_alternatives(request)?; + if current { + let mut maintained = alternatives.last().expect("exact alternative").clone(); + maintained.current_series = true; + maintained.hybrid_execution = false; + maintained.materialization_policy = None; + alternatives.push(maintained); + } + Ok(alternatives) +} + +fn materialization_alternatives( + request: PlanningRequest, ) -> Result, CompileError> { let mut exact = request.clone(); exact.hybrid_execution = false; diff --git a/crates/asap_types/src/query_plan.rs b/crates/asap_types/src/query_plan.rs index 8b0c48e8..8c08dabe 100644 --- a/crates/asap_types/src/query_plan.rs +++ b/crates/asap_types/src/query_plan.rs @@ -5,6 +5,7 @@ //! node IDs. Serving executes this graph without reconstructing Planner IR or //! searching for compatible materializations. +pub mod current_series; pub mod logical; use std::collections::{BTreeMap, BTreeSet}; diff --git a/crates/asap_types/src/query_plan/current_series.rs b/crates/asap_types/src/query_plan/current_series.rs new file mode 100644 index 00000000..d8acee5f --- /dev/null +++ b/crates/asap_types/src/query_plan/current_series.rs @@ -0,0 +1,49 @@ +//! Maintained current-value populations, shared independently of q and k. +use super::{ + logical::{Grouping, LabelMatcher}, + QueryPlanError, +}; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +pub struct SeriesPopulation { + pub metric: String, + pub matchers: Vec, + pub grouping: Grouping, + pub lookback_ms: u64, + pub max_input_lag_ms: u64, + pub max_series: usize, + pub max_bytes: u64, + pub max_k: u64, + pub quantiles: bool, +} +impl SeriesPopulation { + pub fn key(&self) -> String { + serde_json::to_string(self).expect("serializable current-series population") + } + pub fn validate(&self) -> Result<(), QueryPlanError> { + if self.metric.is_empty() + || self.lookback_ms != 300_000 + || self.max_input_lag_ms == 0 + || self.max_input_lag_ms > self.lookback_ms + || self.max_series == 0 + || self.max_series > 100_000 + || self.max_bytes == 0 + || self.max_bytes > 1_073_741_824 + || self.max_k > self.max_series as u64 + { + return Err(QueryPlanError::Invalid( + "invalid current-series population bounds".into(), + )); + } + Ok(()) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] +pub enum SeriesReadout { + Quantile { q: f64 }, + TopK { k: u64 }, +} diff --git a/crates/asap_types/src/query_plan/logical.rs b/crates/asap_types/src/query_plan/logical.rs index 5c3f6e13..0615b7e8 100644 --- a/crates/asap_types/src/query_plan/logical.rs +++ b/crates/asap_types/src/query_plan/logical.rs @@ -9,6 +9,10 @@ fn invalid(message: impl Into) -> QueryPlanError { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] pub enum LogicalOperator { + CurrentSeries { + population: super::current_series::SeriesPopulation, + readout: super::current_series::SeriesReadout, + }, /// A maximal exact scalar/vector subtree evaluated by Prometheus. ExactSubquery { query: String, @@ -116,8 +120,28 @@ pub enum TemporalOperation { impl LogicalOperator { pub fn validate(&self, inputs: usize) -> Result<(), QueryPlanError> { + if let Self::CurrentSeries { + population, + readout, + } = self + { + population.validate()?; + match readout { + super::current_series::SeriesReadout::Quantile { q } + if !q.is_finite() || !population.quantiles => + { + return Err(invalid( + "quantile readout requires finite q and a quantile population", + )) + } + super::current_series::SeriesReadout::TopK { k } if *k > population.max_k => { + return Err(invalid("TopK readout exceeds shared population capacity")) + } + _ => {} + } + } let expected = match self { - Self::Scan { .. } | Self::ExactSubquery { .. } => 0, + Self::Scan { .. } | Self::ExactSubquery { .. } | Self::CurrentSeries { .. } => 0, Self::CandidateExactSubquery { .. } => 1, Self::Binary { .. } | Self::HistogramQuantile => 2, _ => 1, diff --git a/data_plane/src/drivers/ingest/prometheus_remote_write.rs b/data_plane/src/drivers/ingest/prometheus_remote_write.rs index b37d4c79..33531a27 100644 --- a/data_plane/src/drivers/ingest/prometheus_remote_write.rs +++ b/data_plane/src/drivers/ingest/prometheus_remote_write.rs @@ -463,6 +463,14 @@ impl PrometheusRemoteWriteReceiver { ))) })?; + self.inner + .ingest + .sketch_index + .current_series + .lock() + .expect("current-series state poisoned") + .ingest(&physical_plan.query_plan, &new_samples); + // Commit dedup mutation only after the entire routed batch was // reserved successfully. A rejected/backpressured request must not // advance event time or erase retry history. @@ -541,7 +549,7 @@ impl DedupState { } } -fn canonicalize_request( +pub(crate) fn canonicalize_request( request: &WriteRequest, config: &PrometheusRemoteWriteConfig, ) -> Result, RemoteWriteError> { diff --git a/data_plane/src/drivers/query/servers/http.rs b/data_plane/src/drivers/query/servers/http.rs index a6d63925..7fba4b1e 100644 --- a/data_plane/src/drivers/query/servers/http.rs +++ b/data_plane/src/drivers/query/servers/http.rs @@ -2160,6 +2160,15 @@ async fn handle_metrics(State(state): State) -> impl IntoResponse { let mut buffer = Vec::new(); prometheus::Encoder::encode(&encoder, &metric_families, &mut buffer) .unwrap_or_else(|e| tracing::error!("Failed to encode metrics: {}", e)); + let (populations, builds) = state + .sketch_index + .current_series + .lock() + .expect("current-series state poisoned") + .stats(); + buffer.extend_from_slice(format!( + "# TYPE asap_current_series_populations gauge\nasap_current_series_populations {populations}\n# TYPE asap_current_series_cache_builds_total counter\nasap_current_series_cache_builds_total {builds}\n" + ).as_bytes()); if let Some(receiver) = state.remote_write.as_ref() { use std::sync::atomic::Ordering; let stats = receiver.stats(); 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 ccd3a246..4fad687c 100644 --- a/data_plane/src/query_engines/asap_query_engine/engine.rs +++ b/data_plane/src/query_engines/asap_query_engine/engine.rs @@ -275,8 +275,43 @@ impl ASAPQueryEngine { .sketch_index .as_ref() .map(|index| index.summary_update_revision()); - let result = - super::logical_dag::execute_installed(entry, leaves, at, |root, evaluation_ms| { + let result = super::logical_dag::execute_installed( + entry, + leaves, + at, + |root, evaluation_ms| { + if let Some(asap_types::query_plan::QueryPlanNode::Logical { + operator: + asap_types::query_plan::logical::LogicalOperator::CurrentSeries { + population, + readout, + }, + .. + }) = entry.nodes.get(&root) + { + let index = self.sketch_index.as_ref().ok_or_else(|| { + EngineError::capability_miss("current_series", "summary store unavailable") + })?; + let values = index + .current_series + .lock() + .expect("current-series state poisoned") + .read( + ( + physical.query_plan.plan_id, + physical.query_plan.plan_version, + ), + population, + readout, + evaluation_ms, + ) + .map_err(|error| EngineError::capability_miss("current_series", error))?; + use crate::query_engines::query_result::{InstantVectorElement, QueryResult}; + return Ok(QueryResult::vector(values.into_iter().map(|(labels,value)| { + InstantVectorElement::new(crate::storage_engines::types::KeyByLabelValues::new_with_labels(labels.values().cloned().collect()), value) + .with_label_keys_override(labels.into_keys().collect()) + }).collect(), evaluation_ms)); + } let mut subtree = entry.clone(); subtree.root = root; let reachable = subtree.topological_order().map_err(|e| { @@ -383,7 +418,8 @@ impl ASAPQueryEngine { evaluation_ms, false, )) - }); + }, + ); let current = self .sketch_index .as_ref() diff --git a/data_plane/src/query_engines/asap_query_engine/logical_dag.rs b/data_plane/src/query_engines/asap_query_engine/logical_dag.rs index 93c12407..b4e2436f 100644 --- a/data_plane/src/query_engines/asap_query_engine/logical_dag.rs +++ b/data_plane/src/query_engines/asap_query_engine/logical_dag.rs @@ -182,6 +182,16 @@ impl Result> Evaluator<' .clone(); let value = match node { QueryPlanNode::Scalar { value } => Value::Scalar(value), + QueryPlanNode::Logical { + operator: LogicalOperator::CurrentSeries { .. }, + .. + } => { + self.stats.summary_readout_evaluations += 1; + from_result((self.callback)( + id, + u64::try_from(at).map_err(|_| miss("negative current-series timestamp"))?, + )?)? + } QueryPlanNode::Logical { operator, inputs } => { if matches!( operator, @@ -239,6 +249,9 @@ impl Result> Evaluator<' | LogicalOperator::CandidateExactSubquery { .. } => { Err(miss("Prometheus exact leaf was not prepared")) } + LogicalOperator::CurrentSeries { .. } => Err(miss( + "current-series leaf must use its installed node identity", + )), LogicalOperator::Scan { .. } => { Err(miss("local raw Scan is forbidden in deployed plans")) } diff --git a/data_plane/src/storage_engines/sketch_db/current_series.rs b/data_plane/src/storage_engines/sketch_db/current_series.rs new file mode 100644 index 00000000..545cc29b --- /dev/null +++ b/data_plane/src/storage_engines/sketch_db/current_series.rs @@ -0,0 +1,537 @@ +//! Bounded current-value state. Never pools a series' old samples into a quantile. +use crate::drivers::ingest::prometheus_remote_write::CanonicalSample; +use asap_types::query_plan::{ + current_series::{SeriesPopulation, SeriesReadout}, + logical::{LabelMatch, LogicalOperator}, + QueryPlan, QueryPlanNode, +}; +use std::collections::{BTreeMap, BTreeSet}; + +type Labels = BTreeMap; +pub type Vector = Vec<(Labels, f64)>; +#[derive(Debug, Clone)] +struct Ranked { + value: f64, + labels: Labels, +} +impl PartialEq for Ranked { + fn eq(&self, other: &Self) -> bool { + self.cmp(other).is_eq() + } +} +impl Eq for Ranked {} +impl PartialOrd for Ranked { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for Ranked { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.value + .total_cmp(&other.value) + .then(self.labels.cmp(&other.labels)) + } +} +struct Member { + timestamp: i64, + value: Option, + group: Labels, + bytes: u64, +} +#[derive(Default)] +struct Group { + ordered: BTreeSet, + cached: Option<(Vec, Vector)>, +} +struct Population { + definition: SeriesPopulation, + members: BTreeMap, + expiry: BTreeSet<(i64, Labels)>, + groups: BTreeMap, + bytes: u64, + unavailable: bool, + cache_builds: u64, + last_read: i64, + matchers: Vec, +} +impl Population { + fn new(definition: SeriesPopulation) -> Result { + definition.validate().map_err(|e| e.to_string())?; + let mut matchers = vec![]; + for matcher in &definition.matchers { + use promql_parser::parser::token::{T_EQL, T_EQL_REGEX, T_NEQ, T_NEQ_REGEX}; + let token = match matcher.operation { + LabelMatch::Equal => T_EQL, + LabelMatch::NotEqual => T_NEQ, + LabelMatch::Regex => T_EQL_REGEX, + LabelMatch::NotRegex => T_NEQ_REGEX, + }; + matchers.push(promql_parser::label::Matcher::new_matcher( + token, + matcher.name.clone(), + matcher.value.clone(), + )?); + } + Ok(Self { + definition, + members: BTreeMap::new(), + expiry: BTreeSet::new(), + groups: BTreeMap::new(), + bytes: 0, + unavailable: false, + cache_builds: 0, + last_read: i64::MIN, + matchers, + }) + } + fn remove(&mut self, labels: &Labels) { + if let Some(old) = self.members.remove(labels) { + self.expiry.remove(&(old.timestamp, labels.clone())); + self.bytes -= old.bytes; + if let Some(value) = old.value { + if let Some(group) = self.groups.get_mut(&old.group) { + group.ordered.remove(&Ranked { + value, + labels: labels.clone(), + }); + group.cached = None; + if group.ordered.is_empty() { + self.groups.remove(&old.group); + } + } + } + } + } + fn expire(&mut self, cutoff: i64) { + while let Some((timestamp, labels)) = self.expiry.first().cloned() { + if timestamp >= cutoff { + break; + } + self.remove(&labels); + } + } + fn update(&mut self, sample: &CanonicalSample, cutoff: i64) { + if self.unavailable + || sample.metric.as_ref() != self.definition.metric + || sample.timestamp_ms < cutoff + { + return; + } + let mut labels: Labels = sample + .labels + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); + labels.insert("__name__".into(), sample.metric.to_string()); + if !self.matchers.iter().all(|matcher| { + matcher.is_match(labels.get(&matcher.name).map(String::as_str).unwrap_or("")) + }) { + return; + } + if self + .members + .get(&labels) + .is_some_and(|old| old.timestamp >= sample.timestamp_ms) + { + return; + } + let group: Labels = labels + .iter() + .filter(|(key, _)| { + if self.definition.grouping.without { + key.as_str() != "__name__" && !self.definition.grouping.labels.contains(key) + } else { + self.definition.grouping.labels.contains(key) + } + }) + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); + self.remove(&labels); + // Bound the retained keys, tree nodes, and shared readout caches together. + let bytes = 1024 + + labels + .iter() + .map(|(k, v)| (k.len() + v.len()) as u64 * 16) + .sum::(); + if self.members.len() >= self.definition.max_series + || self.bytes.saturating_add(bytes) > self.definition.max_bytes + { + self.unavailable = true; + self.members.clear(); + self.groups.clear(); + self.expiry.clear(); + self.bytes = 0; + return; + } + self.bytes += bytes; + self.expiry.insert((sample.timestamp_ms, labels.clone())); + self.members.insert( + labels.clone(), + Member { + timestamp: sample.timestamp_ms, + value: sample.value, + group: group.clone(), + bytes, + }, + ); + if let Some(value) = sample.value { + let state = self.groups.entry(group).or_default(); + state.ordered.insert(Ranked { value, labels }); + state.cached = None; + } + } + fn read(&mut self, readout: &SeriesReadout) -> Vector { + let mut result = vec![]; + for (labels, group) in &mut self.groups { + if group.cached.is_none() { + let values = if self.definition.quantiles { + group.ordered.iter().map(|r| r.value).collect() + } else { + vec![] + }; + let top = group + .ordered + .iter() + .rev() + .take(self.definition.max_k as usize) + .map(|r| (r.labels.clone(), r.value)) + .collect(); + group.cached = Some((values, top)); + self.cache_builds += 1; + } + let (values, top) = group.cached.as_ref().unwrap(); + match readout { + SeriesReadout::Quantile { q } => { + let value = if *q < 0. { + f64::NEG_INFINITY + } else if *q > 1. { + f64::INFINITY + } else { + let rank = q * (values.len() - 1) as f64; + let lo = rank.floor() as usize; + let hi = (lo + 1).min(values.len() - 1); + let weight = rank - lo as f64; + values[lo] * (1. - weight) + values[hi] * weight + }; + result.push((labels.clone(), value)); + } + SeriesReadout::TopK { k } => result.extend(top.iter().take(*k as usize).cloned()), + } + } + result + } +} + +#[derive(Default)] +pub struct CurrentSeriesStore { + generation: Option<(u64, u64)>, + populations: BTreeMap, + first: Option, + watermark: Option, +} +impl CurrentSeriesStore { + /// Called only after the complete Remote Write batch was admitted successfully. + pub fn ingest(&mut self, plan: &QueryPlan, samples: &[CanonicalSample]) { + let generation = (plan.plan_id, plan.plan_version); + if self.generation != Some(generation) { + *self = Self::default(); + self.generation = Some(generation); + for entry in plan.entries.values() { + for node in entry.nodes.values() { + if let QueryPlanNode::Logical { + operator: LogicalOperator::CurrentSeries { population, .. }, + .. + } = node + { + if let Ok(state) = Population::new(population.clone()) { + self.populations.entry(population.key()).or_insert(state); + } + } + } + } + } + if self.populations.is_empty() || samples.is_empty() { + return; + } + let mut timestamps: Vec<_> = samples.iter().map(|s| s.timestamp_ms).collect(); + timestamps.sort_unstable(); + timestamps.dedup(); + let max_lag = self + .populations + .values() + .map(|p| p.definition.max_input_lag_ms) + .min() + .unwrap() as i64; + for timestamp in timestamps { + if self + .watermark + .is_some_and(|w| timestamp > w.saturating_add(max_lag)) + { + // A gap cannot prove that every still-live series was observed. + self.first = Some(timestamp); + } + self.first.get_or_insert(timestamp); + self.watermark = Some(self.watermark.unwrap_or(timestamp).max(timestamp)); + } + let watermark = self.watermark.unwrap(); + for population in self.populations.values_mut() { + let cutoff = watermark.saturating_sub(population.definition.lookback_ms as i64); + population.expire(cutoff); + for sample in samples { + population.update(sample, cutoff); + } + } + } + pub fn read( + &mut self, + generation: (u64, u64), + definition: &SeriesPopulation, + readout: &SeriesReadout, + at: u64, + ) -> Result { + if self.generation != Some(generation) { + return Err("current-series generation is not ingested".into()); + } + let at = i64::try_from(at).map_err(|_| "invalid evaluation timestamp")?; + let watermark = self.watermark.ok_or("current-series state is cold")?; + if at < watermark { + return Err("current-series state cannot answer historical evaluations".into()); + } + if at > watermark.saturating_add(definition.max_input_lag_ms as i64) { + return Err("current-series input is behind evaluation time".into()); + } + if at.saturating_sub(definition.lookback_ms as i64) + < self.first.ok_or("current-series state is cold")? + { + return Err("current-series lookback is not covered yet".into()); + } + let population = self + .populations + .get_mut(&definition.key()) + .ok_or("current-series population is not installed")?; + if population.unavailable { + return Err("current-series population exceeded its resource budget".into()); + } + if at < population.last_read { + return Err("current-series evaluation precedes already expired state".into()); + } + population.last_read = at; + population.expire(at.saturating_sub(definition.lookback_ms as i64)); + Ok(population.read(readout)) + } + pub fn stats(&self) -> (usize, u64) { + ( + self.populations.len(), + self.populations.values().map(|p| p.cache_builds).sum(), + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::drivers::ingest::prometheus_remote_write::{ + canonicalize_request, Label, PrometheusRemoteWriteConfig, Sample, TimeSeries, WriteRequest, + STALE_NAN_BITS, + }; + use asap_types::query_plan::{ + FallbackPolicy, InstantExecution, QueryLanguage, QueryNodeId, QueryPlanEntry, + }; + fn definition() -> SeriesPopulation { + SeriesPopulation { + metric: "a".into(), + matchers: vec![], + grouping: asap_types::query_plan::logical::Grouping { + labels: vec!["job".into()], + without: false, + }, + lookback_ms: 300_000, + max_input_lag_ms: 60_000, + max_series: 100, + max_bytes: 1_000_000, + max_k: 3, + quantiles: true, + } + } + fn plan(p: &SeriesPopulation) -> QueryPlan { + let mut plan = QueryPlan::empty(); + plan.plan_id = 7; + plan.plan_version = 1; + plan.entries.insert( + "test".into(), + QueryPlanEntry { + language: QueryLanguage::PromQl, + query_id: "test".into(), + canonical_query: "quantile by (job) (0.5, a)".into(), + fixed_evaluation: None, + root: QueryNodeId(0), + nodes: BTreeMap::from([( + QueryNodeId(0), + QueryPlanNode::Logical { + operator: LogicalOperator::CurrentSeries { + population: p.clone(), + readout: SeriesReadout::Quantile { q: 0.5 }, + }, + inputs: vec![], + }, + )]), + instant: InstantExecution { + lookback_ms: 300_000, + full_history: false, + cumulative_readout: true, + }, + fallback: FallbackPolicy::ExactBackend, + }, + ); + plan + } + fn sample(pod: &str, job: &str, timestamp: i64, value: Option) -> CanonicalSample { + canonicalize_request( + &WriteRequest { + timeseries: vec![TimeSeries { + labels: [("__name__", "a"), ("pod", pod), ("job", job)] + .into_iter() + .map(|(name, value)| Label { + name: name.into(), + value: value.into(), + }) + .collect(), + samples: vec![Sample { + timestamp, + value: value.unwrap_or(f64::from_bits(STALE_NAN_BITS)), + }], + exemplars: vec![], + histograms: vec![], + }], + }, + &PrometheusRemoteWriteConfig::default(), + ) + .unwrap() + .remove(0) + } + fn warm(store: &mut CurrentSeriesStore, plan: &QueryPlan) { + for t in (0..=300_000).step_by(60_000) { + store.ingest( + plan, + &[ + sample("x", "api", t, Some(1.)), + sample("y", "api", t, Some(9.)), + sample("z", "api", t, Some(5.)), + sample("w", "db", t, Some(50.)), + ], + ); + } + } + /// Four quantiles reuse one distribution, and smaller k reads the shared maximum-k prefix. + #[test] + fn quantiles_and_topk_share_state_and_promote_after_updates_and_staleness() { + let p = definition(); + let plan = plan(&p); + let mut store = CurrentSeriesStore::default(); + warm(&mut store, &plan); + for (q, expected) in [(0.5, 5.), (0.9, 8.2), (0.95, 8.6), (0.99, 8.92)] { + let result = store + .read((7, 1), &p, &SeriesReadout::Quantile { q }, 300_000) + .unwrap(); + assert!((result[0].1 - expected).abs() < 1e-10); + assert_eq!(result[1].1, 50.); + } + for percentile in 1..100 { + let q = percentile as f64 / 100.; + let result = store + .read((7, 1), &p, &SeriesReadout::Quantile { q }, 300_000) + .unwrap(); + assert!((result[0].1 - (1. + 8. * q)).abs() < 1e-10); + } + let small = store + .read((7, 1), &p, &SeriesReadout::TopK { k: 1 }, 300_000) + .unwrap(); + let big = store + .read((7, 1), &p, &SeriesReadout::TopK { k: 3 }, 300_000) + .unwrap(); + assert_eq!(small[0].0["pod"], "y"); + assert_eq!(big[0], small[0]); + assert_eq!(store.stats(), (1, 2)); + store.ingest(&plan, &[sample("y", "api", 301_000, Some(-5.))]); + assert_eq!( + store + .read((7, 1), &p, &SeriesReadout::TopK { k: 1 }, 301_000) + .unwrap()[0] + .0["pod"], + "z" + ); + store.ingest(&plan, &[sample("z", "api", 302_000, None)]); + assert_eq!( + store + .read((7, 1), &p, &SeriesReadout::TopK { k: 1 }, 302_000) + .unwrap()[0] + .0["pod"], + "x" + ); + // Out-of-order old values must not resurrect the stale series. + store.ingest(&plan, &[sample("z", "api", 301_000, Some(100.))]); + assert_eq!( + store + .read((7, 1), &p, &SeriesReadout::TopK { k: 1 }, 302_000) + .unwrap()[0] + .0["pod"], + "x" + ); + } + /// Cold state, gaps, old generations and historical timestamps cannot masquerade as complete populations. + #[test] + fn coverage_expiration_generation_and_capacity_fail_closed() { + let p = definition(); + let plan = plan(&p); + let mut store = CurrentSeriesStore::default(); + store.ingest(&plan, &[sample("x", "api", 0, Some(1.))]); + assert!(store + .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 0) + .is_err()); + warm(&mut store, &plan); + assert!(store + .read((7, 2), &p, &SeriesReadout::Quantile { q: 0.5 }, 300_000) + .is_err()); + assert!(store + .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 299_000) + .is_err()); + for t in (360_000..=600_000).step_by(60_000) { + store.ingest(&plan, &[sample("y", "api", t, Some(9.))]); + } + assert_eq!( + store + .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 600_000) + .unwrap() + .len(), + 2 + ); // inclusive lookback boundary + assert_eq!( + store + .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 600_001) + .unwrap() + .len(), + 1 + ); + assert!(store + .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 600_000) + .is_err()); + store.ingest(&plan, &[sample("y", "api", 900_000, Some(9.))]); + assert!(store + .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 900_000) + .is_err()); + let mut bounded = p.clone(); + bounded.max_series = 3; + let plan = super::tests::plan(&bounded); + let mut store = CurrentSeriesStore::default(); + warm(&mut store, &plan); + assert!(store + .read( + (7, 1), + &bounded, + &SeriesReadout::Quantile { q: 0.5 }, + 300_000 + ) + .unwrap_err() + .contains("budget")); + } +} diff --git a/data_plane/src/storage_engines/sketch_db/index/mod.rs b/data_plane/src/storage_engines/sketch_db/index/mod.rs index 9d3fb440..d5610f23 100644 --- a/data_plane/src/storage_engines/sketch_db/index/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/index/mod.rs @@ -631,6 +631,7 @@ impl Drop for StateMutation<'_> { #[derive(Default)] pub struct SketchStore { + pub current_series: std::sync::Mutex, /// Held through each state append; completion takes the exclusive guard. completed_windows: RwLock>, completion_flush_before: std::sync::atomic::AtomicU64, diff --git a/data_plane/src/storage_engines/sketch_db/mod.rs b/data_plane/src/storage_engines/sketch_db/mod.rs index 428886af..93ac2ef4 100644 --- a/data_plane/src/storage_engines/sketch_db/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/mod.rs @@ -56,3 +56,5 @@ pub use sds::{ DataDescriptor, DataDescriptorId, SdsBinding, SummaryDescriptor, SummaryDescriptorId, SummaryDescriptorRegistry, SummaryOperator, }; + +pub mod current_series; diff --git a/data_plane/tests/asapquery_compatibility_process_e2e.rs b/data_plane/tests/asapquery_compatibility_process_e2e.rs index 9a532a34..0f891faa 100644 --- a/data_plane/tests/asapquery_compatibility_process_e2e.rs +++ b/data_plane/tests/asapquery_compatibility_process_e2e.rs @@ -28,6 +28,9 @@ mod durable_summary_process; #[path = "support/immutable_maintenance_process.rs"] mod immutable_maintenance_process; +#[path = "support/current_series_process.rs"] +mod current_series_process; + struct ChildGuard(Child); impl Drop for ChildGuard { diff --git a/data_plane/tests/support/current_series_process.rs b/data_plane/tests/support/current_series_process.rs new file mode 100644 index 00000000..08a5efa8 --- /dev/null +++ b/data_plane/tests/support/current_series_process.rs @@ -0,0 +1,305 @@ +use super::*; +use control_plane::physical::{ + compiler::{ + BackendLocalPlanningSnapshot, PhysicalCompiler, BACKEND_REVISION, PLANNER_REVISION, + }, + workload_cost::{self, WorkloadCostEvidence, WorkloadQuote}, +}; + +/// Remote Write updates one current population; quantiles and different TopK limits share it. +#[tokio::test] +async fn current_series_quantiles_topk_share_and_replace_values() { + let calls = Arc::new(std::sync::atomic::AtomicU64::new(0)); + let observed = calls.clone(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let fallback = format!("http://{}", listener.local_addr().unwrap()); + let task = tokio::spawn(async move { + axum::serve(listener,Router::new() + .route("/-/healthy",get(||async {"healthy"})) + .route("/api/v1/query",get(move || {let observed=observed.clone();async move { + observed.fetch_add(1,std::sync::atomic::Ordering::Relaxed); + Json(serde_json::json!({"status":"success","data":{"resultType":"vector","result":[{"metric":{"fallback":"true"},"value":[0,"999"]}]}})) + }}))).await.unwrap(); + }); + let native = std::env::var("ASAP_CURRENT_SERIES_PROMETHEUS_URL").ok(); + let fallback = native.clone().unwrap_or(fallback); + let mut snapshot: BackendLocalPlanningSnapshot = serde_json::from_str(include_str!( + "../../../docs/examples/asapquery-planning-snapshot.json" + )) + .unwrap(); + snapshot.snapshot_version = 2; + snapshot.implementation.source_sample_interval_ms = Some(60_000); + let template = snapshot.query_workload.repeating_queries.as_ref().unwrap()[0].clone(); + let mut queries = vec![]; + for q in [0.5, 0.9, 0.95, 0.99] { + queries.push(format!("quantile({q}, a)")); + queries.push(format!("quantile by (job) ({q}, a)")); + } + for k in [1, 2, 3] { + queries.push(format!("topk({k}, a)")); + queries.push(format!("topk by (job) ({k}, a)")); + } + snapshot.query_workload.repeating_queries = Some( + queries + .iter() + .map(|q| { + let mut e = template.clone(); + e.query = planner_types::workload::Query(q.clone()); + e + }) + .collect(), + ); + let (request, env) = snapshot.clone().planning_request().unwrap(); + let candidates = workload_cost::with_exact_alternative(request).unwrap(); + let quotes = candidates + .into_iter() + .filter_map(|candidate| { + let plan = PhysicalCompiler + .compile(candidate.clone(), env.clone()) + .ok()?; + let warm = candidate.current_series; + let manifest = workload_cost::manifest(&plan, &candidate.queries).unwrap(); + Some(WorkloadQuote { + unit_costs: manifest + .components + .keys() + .map(|key| (key.clone(), if warm { 1. } else { 1e12 })) + .collect(), + manifest, + executable: true, + }) + }) + .collect(); + snapshot.workload_cost_evidence = Some(WorkloadCostEvidence { + backend_revision: BACKEND_REVISION.into(), + planner_revision: PLANNER_REVISION.into(), + data_snapshot_id: "current-series-process-test".into(), + model_version: "synthetic-test-only".into(), + observed_at_unix_ms: env.observed_at_unix_ms, + valid_for_ms: env.max_evidence_age_ms, + quotes, + }); + let planned = snapshot.clone().compile().unwrap(); + assert!(planned + .query_plan + .entries + .values() + .all(|e| serde_json::to_string(e).unwrap().contains("current_series"))); + let output = tempfile::tempdir().unwrap(); + let path = output.path().join("snapshot.json"); + std::fs::write(&path, serde_json::to_vec(&snapshot).unwrap()).unwrap(); + let port = unused_port(); + let mut child = ChildGuard( + Command::new(env!("CARGO_BIN_EXE_data_plane")) + .args([ + "--profile", + "asapquery", + "--forward-unsupported-queries", + "--planning-snapshot", + ]) + .arg(&path) + .args([ + "--prometheus-server", + &fallback, + "--http-port", + &port.to_string(), + "--output-dir", + ]) + .arg(output.path()) + .stdout(Stdio::null()) + .stderr(Stdio::inherit()) + .spawn() + .unwrap(), + ); + let client = reqwest::Client::new(); + let base = format!("http://127.0.0.1:{port}"); + wait_until_ready(&client, &format!("{base}/api/v1/health"), &mut child.0).await; + let end = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as i64; + let times: Vec<_> = (0..=5).map(|i| end - 300_000 + i * 60_000).collect(); + let wire = WriteRequest { + timeseries: [ + ("x", "api", 1.), + ("y", "api", 9.), + ("z", "api", 5.), + ("w", "db", 50.), + ] + .into_iter() + .map(|(pod, job, value)| { + series_with_labels( + "a", + &[("pod", pod), ("job", job)], + ×.iter().map(|t| (*t, value)).collect::>(), + ) + }) + .collect(), + }; + if let Some(url) = &native { + assert_eq!(remote_write(&client, url, &wire).await, 204); + } + assert_eq!(remote_write(&client, &base, &wire).await, 204); + async fn query(client: &reqwest::Client, base: &str, q: &str, at: i64) -> Value { + client + .get(format!("{base}/api/v1/query")) + .query(&[ + ("query", q.to_string()), + ("time", format!("{:.3}", at as f64 / 1000.)), + ]) + .send() + .await + .unwrap() + .json() + .await + .unwrap() + } + async fn compare_native( + client: &reqwest::Client, + native: &Option, + q: &str, + at: i64, + actual: &Value, + ) { + if let Some(url) = native { + let expected = query(client, url, q, at).await; + let normalize = |body: &Value| { + let mut rows: Vec<_> = body["data"]["result"] + .as_array() + .unwrap() + .iter() + .map(|row| { + ( + serde_json::to_string(&row["metric"]).unwrap(), + row["value"][1].as_str().unwrap().parse::().unwrap(), + ) + }) + .collect(); + rows.sort_by(|a, b| a.0.cmp(&b.0)); + rows + }; + let a = normalize(actual); + let b = normalize(&expected); + assert_eq!(a.len(), b.len(), "{q}: {actual} vs {expected}"); + for (a, b) in a.iter().zip(&b) { + assert_eq!(a.0, b.0); + assert!((a.1 - b.1).abs() < 1e-8, "{q}: {a:?} vs {b:?}"); + } + } + } + for (q, global, grouped) in [ + (0.5, 7., 5.), + (0.9, 37.7, 8.2), + (0.95, 43.85, 8.6), + (0.99, 48.77, 8.92), + ] { + for (text, expected) in [ + (format!("quantile({q}, a)"), global), + (format!("quantile by (job) ({q}, a)"), grouped), + ] { + let body = query(&client, &base, &text, end).await; + assert!(is_warm(&body), "{text}: {body}"); + compare_native(&client, &native, &text, end, &body).await; + let value = body["data"]["result"][0]["value"][1] + .as_str() + .unwrap() + .parse::() + .unwrap(); + assert!((value - expected).abs() < 1e-8, "{text}: {body}"); + } + } + for k in [1, 2, 3] { + let global = query(&client, &base, &format!("topk({k}, a)"), end).await; + assert!(is_warm(&global), "{global}"); + compare_native(&client, &native, &format!("topk({k}, a)"), end, &global).await; + assert_eq!(global["data"]["result"].as_array().unwrap().len(), k); + let body = query(&client, &base, &format!("topk by (job) ({k}, a)"), end).await; + assert!(is_warm(&body), "{body}"); + compare_native( + &client, + &native, + &format!("topk by (job) ({k}, a)"), + end, + &body, + ) + .await; + assert_eq!( + body["data"]["result"].as_array().unwrap().len(), + k + 1, + "{body}" + ); + } + let metrics = client + .get(format!("{base}/metrics")) + .send() + .await + .unwrap() + .text() + .await + .unwrap(); + assert!( + metrics.contains("asap_current_series_populations 2\n"), + "{metrics}" + ); + assert!( + metrics.contains("asap_current_series_cache_builds_total 3\n"), + "{metrics}" + ); + assert_eq!(calls.load(std::sync::atomic::Ordering::Relaxed), 0); + // A decreasing value and a stale marker must promote a formerly excluded series. + for (pod, value, offset, expected) in [ + ("y", -5., 1000, "z"), + ( + "z", + f64::from_bits(data_plane::drivers::ingest::prometheus_remote_write::STALE_NAN_BITS), + 2000, + "x", + ), + ] { + let update = WriteRequest { + timeseries: vec![series_with_labels( + "a", + &[("pod", pod), ("job", "api")], + &[(end + offset, value)], + )], + }; + if let Some(url) = &native { + assert_eq!(remote_write(&client, url, &update).await, 204); + } + assert_eq!(remote_write(&client, &base, &update).await, 204); + let body = query(&client, &base, "topk by (job) (1, a)", end + offset).await; + assert!(is_warm(&body), "{body}"); + assert_eq!( + body["data"]["result"][0]["metric"]["pod"], expected, + "{body}" + ); + compare_native( + &client, + &native, + "topk by (job) (1, a)", + end + offset, + &body, + ) + .await; + let body = query(&client, &base, "quantile by (job) (0.5, a)", end + offset).await; + assert!(is_warm(&body), "{body}"); + compare_native( + &client, + &native, + "quantile by (job) (0.5, a)", + end + offset, + &body, + ) + .await; + } + // Historical reads cannot use a state already updated beyond their evaluation time. + let body = query(&client, &base, "quantile(0.5, a)", end).await; + assert!(!is_warm(&body), "historical read must fall back: {body}"); + if native.is_none() { + assert_eq!( + body["data"]["result"][0]["metric"]["fallback"], "true", + "{body}" + ); + } + task.abort(); +} diff --git a/docs/developer_docs/query-engine/current-series-aggregations.md b/docs/developer_docs/query-engine/current-series-aggregations.md new file mode 100644 index 00000000..8a2622d1 --- /dev/null +++ b/docs/developer_docs/query-engine/current-series-aggregations.md @@ -0,0 +1,63 @@ +# Shared current-series quantiles and TopK + +Backend-local PromQL workload compilation can export a maintained current-value +alternative for `quantile(q, metric)` and `topk(k, metric)`, including `by` and +`without` grouping and selector label matchers. Parameters must be finite scalar +literals. Selector offsets, `@`, nested input expressions and MetricsQL use the +existing alternatives; they are not admitted by this implementation. + +For example, put p50, p90, p95, p99 and Top1/Top5 in one workload. Matching source, +selector and grouping contracts produce one `CurrentSeries` population with +`quantiles: true` and `max_k: 5`. Each registered query keeps its own readout. +Different sources, filters and groupings remain distinct. This state is exact: +it retains each series' current value in a shared ordered population, rather than +inserting all historical observations into a quantile sketch. Its memory grows +with series cardinality, even when only Top1 is requested. Those excluded series +are required to promote the correct replacement when a winner decreases or expires. + +Accepted Remote Write batches update the state atomically under its lock. A newer +sample replaces the old value; stale markers remove the value. Older updates do +not resurrect a newer stale marker. Per-group readout arrays are shared until that +group changes. TopK-only populations cache just the largest registered k results; +quantile consumers also share an ordered value array. + +Deployment uses complete workload quotes. The manifest deduplicates population +build, update, residency and retirement components across consumers and prices +individual readouts separately. Exporting a candidate does not establish a speedup. +A native exact alternative remains available for cost selection and execution fallback. + +## Runtime coverage + +- This implementation serves current evaluations, using Prometheus' default + five-minute selector lookback. It requires a complete Remote Write feed for + each registered metric; the producer must not omit matching series. +- It waits for five minutes of observed input coverage. Event-time gaps exceeding + the installed input-lag bound restart this warmup. The bound comes from the + declared sample interval plus staleness margin (60 seconds when unspecified). +- Input lag, historical evaluations, reads before already-expired state, an + unobserved generation or exceeded resource bounds cause native fallback. +- State is in memory. Restart and generation replacement require warmup again; + historical range queries continue to use native execution. +- Populations divide the configured retained-summary memory budget and cap series + cardinality. Bounds include conservative space for labels, trees and caches. + The existing Remote Write adapter accepts finite sample values and stale markers. + +`/metrics` exposes `asap_current_series_populations` and +`asap_current_series_cache_builds_total` to verify reuse. These describe the active +in-memory population generation; they are not window-sketch materialization counts. + +## Validation + +Run the compiler regression and state tests, then the process acceptance test: + +```sh +cargo +1.98.0 test --locked -p control_plane --lib current_series_quantiles_and_topk +cargo +1.98.0 test --locked -p data_plane --lib current_series +cargo +1.98.0 test --locked -p data_plane --test asapquery_compatibility_process_e2e current_series_quantiles_topk +``` + +For differential validation, set `ASAP_CURRENT_SERIES_PROMETHEUS_URL` to a **fresh** +Prometheus instance with `--web.enable-remote-write-receiver`, then run the process +test. It writes the same samples to both services and compares values and labels +for quantiles and TopK, including value replacement and staleness. Test quotes are +synthetic and must not be used as performance evidence. diff --git a/docs/developer_docs/query-engine/query-engine.md b/docs/developer_docs/query-engine/query-engine.md index bde6f15d..0254787f 100644 --- a/docs/developer_docs/query-engine/query-engine.md +++ b/docs/developer_docs/query-engine/query-engine.md @@ -138,3 +138,6 @@ remote failures remain errors and are not successful empty vectors. - Partial/stale/gapped summary state must not return a successful complete `QueryResponse`. - A plan swap during execution must not mix identities in one response. + +For maintained instant-vector quantiles and shared maximum-k TopK state, see +[current-series aggregations](current-series-aggregations.md). From d419b232d7d80b52af6193af6765941d945d7eed Mon Sep 17 00:00:00 2001 From: zz_y Date: Sat, 12 Sep 2026 13:45:02 -0600 Subject: [PATCH 2/8] Lower shared current-series state from Planner-selected IR --- Cargo.lock | 10 +- Cargo.toml | 10 +- control_plane/src/main.rs | 1 - control_plane/src/physical/compiler.rs | 89 +++++++--- control_plane/src/physical/current_series.rs | 164 +++++++----------- control_plane/src/physical/workload_cost.rs | 26 ++- .../tests/support/current_series_process.rs | 12 +- .../current-series-aggregations.md | 9 + 8 files changed, 178 insertions(+), 143 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c11b4ad4..2163e7c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -373,7 +373,7 @@ dependencies = [ [[package]] name = "asap-aware-mapping" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=3be523fa0f06a905188e42cbe482d06aa843ba5d#3be523fa0f06a905188e42cbe482d06aa843ba5d" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" dependencies = [ "asap-types", "serde", @@ -384,7 +384,7 @@ dependencies = [ [[package]] name = "asap-frontend-promql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=3be523fa0f06a905188e42cbe482d06aa843ba5d#3be523fa0f06a905188e42cbe482d06aa843ba5d" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" dependencies = [ "asap-types", "promql-parser 0.10.0 (git+https://github.com/ProjectASAP/promql-parser?rev=9fede7eecca923c9882fe256484d00d37f8706cb)", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=3be523fa0f06a905188e42cbe482d06aa843ba5d#3be523fa0f06a905188e42cbe482d06aa843ba5d" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -416,12 +416,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=3be523fa0f06a905188e42cbe482d06aa843ba5d#3be523fa0f06a905188e42cbe482d06aa843ba5d" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=3be523fa0f06a905188e42cbe482d06aa843ba5d#3be523fa0f06a905188e42cbe482d06aa843ba5d" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 75fec181..4a71a3ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,12 +18,12 @@ version = "0.1.0" asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", branch = "main" } [workspace.dependencies] -# Keep Planner frontends, selection, and IR on the same immutable main revision. +# Keep Planner frontends, selection, and IR on the same immutable revision (current-series Planner PR). # Alias upstream asap-types because this workspace also defines asap_types. -planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "3be523fa0f06a905188e42cbe482d06aa843ba5d" } -asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "3be523fa0f06a905188e42cbe482d06aa843ba5d" } -asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "3be523fa0f06a905188e42cbe482d06aa843ba5d" } -asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "3be523fa0f06a905188e42cbe482d06aa843ba5d" } +planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } +asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } +asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } +asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } # Shared external deps (used by 2+ crates) serde = { version = "1.0", features = ["derive"] } diff --git a/control_plane/src/main.rs b/control_plane/src/main.rs index 6480d5bb..d0f0bc0f 100644 --- a/control_plane/src/main.rs +++ b/control_plane/src/main.rs @@ -973,7 +973,6 @@ fn compile_physical_plan_request( }; let planning_request = physical::compiler::PlanningRequest { - current_series: false, logical_selection, query_workload: None, queries, diff --git a/control_plane/src/physical/compiler.rs b/control_plane/src/physical/compiler.rs index 51300a45..de1eef20 100644 --- a/control_plane/src/physical/compiler.rs +++ b/control_plane/src/physical/compiler.rs @@ -139,8 +139,6 @@ pub struct LifecyclePlanningInput { #[derive(Debug, Clone, Default)] pub struct PlanningRequest { - /// Exact current-value state is a separately priced physical alternative. - pub current_series: bool, /// Diagnostic projections of the original Planner search; never consumed by selection. pub logical_selection: Vec, /// Enable a composable DAG with SummaryStore materializations and Prometheus exact subtrees. @@ -683,7 +681,6 @@ impl BackendLocalPlanningSnapshot { // Composable lowering residualizes unsafe leaves individually; retain Planner siblings. Ok(( PlanningRequest { - current_series: false, logical_selection, hybrid_execution: true, materialization_policy: None, @@ -918,7 +915,7 @@ impl PhysicalCompiler { environment: DeploymentEnvironment, metricsql: bool, ) -> Result { - if request.current_series + if super::current_series::supported(&request) && (metricsql || environment.target != PhysicalDeploymentTarget::BackendLocalRemoteWrite) { @@ -1075,7 +1072,7 @@ impl PhysicalCompiler { .collect::>(); // An exact native fallback has no maintained state and must not // depend on evidence for unused window/state implementations. - if selected.is_empty() { + if selected.is_empty() && super::current_series::operator(&request, query)?.is_none() { continue; } let executable = @@ -1085,6 +1082,9 @@ impl PhysicalCompiler { reason: format!("invalid executable subDAG: {error}"), })?; executable_dags[query_index] = Some(executable); + if selected.is_empty() { + continue; + } validate_lifecycle_input(&query.query_id, &query.lifecycle)?; if environment.target == PhysicalDeploymentTarget::DistributedCollectors && selected.iter().any(|state| { @@ -1432,12 +1432,12 @@ impl PhysicalCompiler { } } - let plan_id = if request.hybrid_execution || request.current_series { + let plan_id = if request.hybrid_execution || super::current_series::supported(&request) { use std::hash::{Hash, Hasher}; let mut hash = std::collections::hash_map::DefaultHasher::new(); stable_workload_plan_id(&plan_materializations, &request.queries).hash(&mut hash); "typed-local-residual-v3-counter-index".hash(&mut hash); - request.current_series.hash(&mut hash); + super::current_series::supported(&request).hash(&mut hash); request.materialization_policy.hash(&mut hash); for query in &request.queries { format!("{:?}", query.post_asap).hash(&mut hash); @@ -1565,7 +1565,31 @@ impl PhysicalCompiler { full_history: false, cumulative_readout: true, }; - let mut entry = if request.hybrid_execution { + let mut entry = if let Some(operator) = + super::current_series::operator(&request, query)? + { + let root = crate::query_plan::QueryNodeId(0); + let compiled = executable_dags[query_index] + .as_ref() + .expect("compiled Planner DAG"); + query_node_bindings.insert((query_index, compiled.dag.root), root); + Ok(crate::query_plan::QueryPlanEntry { + language: crate::query_plan::QueryLanguage::PromQl, + query_id: query.query_id.clone(), + canonical_query: canonical.clone(), + fixed_evaluation: None, + root, + nodes: BTreeMap::from([( + root, + crate::query_plan::QueryPlanNode::Logical { + operator, + inputs: vec![], + }, + )]), + instant, + fallback: FallbackPolicy::ExactBackend, + }) + } else if request.hybrid_execution { crate::query_plan::compile_bound_composable_mapped( query.query_id.clone(), canonical.clone(), @@ -1609,18 +1633,6 @@ impl PhysicalCompiler { if metricsql { entry.language = crate::query_plan::QueryLanguage::MetricsQl; } - if request.current_series && !metricsql { - if let Some(operator) = super::current_series::operator(&request, query)? { - entry.root = crate::query_plan::QueryNodeId(0); - entry.nodes = BTreeMap::from([( - entry.root, - crate::query_plan::QueryPlanNode::Logical { - operator, - inputs: vec![], - }, - )]); - } - } let catalog_key = QueryPlan::catalog_key(entry.language, &canonical); if query_entries.insert(catalog_key, entry).is_some() { return Err(CompileError::Query { @@ -3319,6 +3331,42 @@ mod tests { } } assert_eq!(populations.len(), 1); + let installed = serde_json::to_string(&plan.precompute_plan.executable_dags).unwrap(); + assert!( + installed.contains("MaintainCurrentSeries"), + "shared state must originate in the installed Planner DAG" + ); + } + + // Physical lowering follows selected Planner IR, independent of catalog text. + #[test] + fn current_series_lowering_uses_selected_ir_not_query_text() { + let mut request = request("ir", "quantile(0.5, a)"); + let root = Rc::new( + crate::query_parser::parse_query_expr_canonical( + "quantile(0.5, a)", + AccuracyTarget::Exact, + ) + .unwrap(), + ); + let strategy = asap_aware_mapping::current_series::CurrentSeriesStrategy::new( + std::slice::from_ref(&root), + ); + request.queries[0].post_asap = strategy.candidate(&root).unwrap(); + let before = super::super::current_series::operator(&request, &request.queries[0]) + .unwrap() + .unwrap(); + request.queries[0].query_string = "quantile(0.99, b)".into(); + let after = super::super::current_series::operator(&request, &request.queries[0]) + .unwrap() + .unwrap(); + assert_eq!(before, after); + request.queries[0].post_asap = crate::planner_selection::keep_pre_asap(&root).unwrap(); + assert!( + super::super::current_series::operator(&request, &request.queries[0]) + .unwrap() + .is_none() + ); } #[test] @@ -3808,7 +3856,6 @@ mod tests { evidence_by_query.insert(query_id.to_string(), evidence); } Ok(PlanningRequest { - current_series: false, logical_selection: Vec::new(), hybrid_execution: false, materialization_policy: None, diff --git a/control_plane/src/physical/current_series.rs b/control_plane/src/physical/current_series.rs index d05d8593..3dbcb574 100644 --- a/control_plane/src/physical/current_series.rs +++ b/control_plane/src/physical/current_series.rs @@ -1,141 +1,93 @@ -//! Bind exact current-value aggregations without treating historical samples as a population. +//! Lower Planner-selected current-series operators; never discover query rewrites here. use super::compiler::{CompileError, PlanningQuery, PlanningRequest}; use asap_types::query_plan::{ current_series::{SeriesPopulation, SeriesReadout}, logical::{Grouping, LabelMatch, LabelMatcher, LogicalOperator}, }; -use promql_parser::{ - label::MatchOp, - parser::{self, Expr, LabelModifier}, -}; +use planner_types::post_asap::{current_series::*, SummaryExpr, SummaryNode, ValueOperation}; -fn parse(query: &str) -> Option<(SeriesPopulation, SeriesReadout)> { - let Expr::Aggregate(a) = parser::parse(query).ok()? else { - return None; - }; - let Expr::VectorSelector(selector) = a.expr.as_ref() else { - return None; - }; - if selector.offset.is_some() - || selector.at.is_some() - || !selector.matchers.or_matchers.is_empty() - { - return None; - } - let Expr::NumberLiteral(parameter) = a.param.as_deref()? else { +fn selected(node: &SummaryNode) -> Option<(&CurrentSeriesPopulation, &CurrentSeriesReadout)> { + let SummaryExpr::ValueOperation { + child, + operation: ValueOperation::ReadCurrentSeries { readout }, + .. + } = &node.expr + else { return None; }; - if !parameter.val.is_finite() { + let SummaryExpr::ValueOperation { + operation: ValueOperation::MaintainCurrentSeries { population }, + .. + } = &child.expr + else { return None; - } - let readout = match a.op.to_string().as_str() { - "quantile" => SeriesReadout::Quantile { q: parameter.val }, - "topk" => SeriesReadout::TopK { - k: (parameter.val as i64).max(0) as u64, - }, - _ => return None, - }; - let mut grouping = match &a.modifier { - None => Grouping { - labels: vec![], - without: false, - }, - Some(LabelModifier::Include(labels)) => Grouping { - labels: labels.labels.clone(), - without: false, - }, - Some(LabelModifier::Exclude(labels)) => Grouping { - labels: labels.labels.clone(), - without: true, - }, }; - grouping.labels.sort(); - grouping.labels.dedup(); - let mut matchers: Vec<_> = selector - .matchers - .matchers - .iter() - .map(|m| LabelMatcher { - name: m.name.clone(), - value: m.value.clone(), - operation: match m.op { - MatchOp::Equal => LabelMatch::Equal, - MatchOp::NotEqual => LabelMatch::NotEqual, - MatchOp::Re(_) => LabelMatch::Regex, - MatchOp::NotRe(_) => LabelMatch::NotRegex, - }, - }) - .collect(); - matchers.sort_by_key(|m| { - ( - m.name.clone(), - m.value.clone(), - format!("{:?}", m.operation), - ) - }); - Some(( - SeriesPopulation { - metric: selector.name.clone()?, - matchers, - grouping, - lookback_ms: 300_000, - max_input_lag_ms: 60_000, - max_series: 100_000, - max_bytes: 64 * 1024 * 1024, - max_k: 0, - quantiles: false, - }, - readout, - )) + Some((population, readout)) } pub(super) fn supported(request: &PlanningRequest) -> bool { request .queries .iter() - .any(|q| parse(&q.query_string).is_some()) + .any(|q| selected(&q.post_asap).is_some()) } pub(super) fn operator( request: &PlanningRequest, query: &PlanningQuery, ) -> Result, CompileError> { - let Some((mut population, readout)) = parse(&query.query_string) else { + let Some((spec, readout)) = selected(&query.post_asap) else { return Ok(None); }; - // Compare source/group semantics before adding workload-wide resource bounds. - for other in &request.queries { - if let Some((other_population, other_readout)) = parse(&other.query_string) { - let mut identity = population.clone(); - identity.max_k = 0; - identity.quantiles = false; - if identity == other_population { - match other_readout { - SeriesReadout::TopK { k } => population.max_k = population.max_k.max(k), - SeriesReadout::Quantile { .. } => population.quantiles = true, - } - } - } - } let populations: std::collections::BTreeSet<_> = request .queries .iter() - .filter_map(|q| parse(&q.query_string).map(|(p, _)| p.key())) + .filter_map(|q| { + selected(&q.post_asap) + .map(|(p, _)| serde_json::to_string(p).expect("typed population serializes")) + }) .collect(); - population.max_bytes = request + let max_bytes = request .retained_summary_memory_budget_bytes - .unwrap_or(population.max_bytes) + .unwrap_or(64 * 1024 * 1024) .min(1_073_741_824) / populations.len().max(1) as u64; - population.max_series = population - .max_series - .min((population.max_bytes / 1024) as usize); - population.max_input_lag_ms = request - .source_sample_interval_ms - .unwrap_or(60_000) - .saturating_add(request.query_staleness_margin_ms) - .clamp(1, 300_000); + let population = SeriesPopulation { + metric: spec.metric.clone(), + matchers: spec + .matchers + .iter() + .map(|m| LabelMatcher { + name: m.label.clone(), + value: m.value.clone(), + operation: match m.operation { + CurrentSeriesMatch::Equal => LabelMatch::Equal, + CurrentSeriesMatch::NotEqual => LabelMatch::NotEqual, + CurrentSeriesMatch::Regex => LabelMatch::Regex, + CurrentSeriesMatch::NotRegex => LabelMatch::NotRegex, + }, + }) + .collect(), + grouping: Grouping { + labels: spec.grouping.clone(), + without: spec.without, + }, + lookback_ms: spec.lookback_ms, + max_k: spec.max_k as u64, + quantiles: spec.quantiles, + max_bytes, + max_series: 100_000.min((max_bytes / 1024) as usize), + max_input_lag_ms: request + .source_sample_interval_ms + .unwrap_or(60_000) + .saturating_add(request.query_staleness_margin_ms) + .clamp(1, 300_000), + }; population.validate()?; + let readout = match readout { + CurrentSeriesReadout::Quantile { q } => SeriesReadout::Quantile { q: *q }, + CurrentSeriesReadout::TopK { k } => SeriesReadout::TopK { k: *k as u64 }, + }; Ok(Some(LogicalOperator::CurrentSeries { population, readout, diff --git a/control_plane/src/physical/workload_cost.rs b/control_plane/src/physical/workload_cost.rs index cc118ce4..cbf91ef5 100644 --- a/control_plane/src/physical/workload_cost.rs +++ b/control_plane/src/physical/workload_cost.rs @@ -680,11 +680,29 @@ fn select_with_frontend( pub fn with_exact_alternative( request: PlanningRequest, ) -> Result, CompileError> { - let current = !request.current_series && super::current_series::supported(&request); + let already_selected = super::current_series::supported(&request); let mut alternatives = materialization_alternatives(request)?; - if current { - let mut maintained = alternatives.last().expect("exact alternative").clone(); - maintained.current_series = true; + if already_selected { + return Ok(alternatives); + } + let mut maintained = alternatives.last().expect("exact alternative").clone(); + let roots: Vec<_> = maintained + .queries + .iter() + .map(|q| match &q.post_asap.expr { + planner_types::post_asap::SummaryExpr::KeepPreAsap(root) => std::rc::Rc::clone(root), + _ => unreachable!("native alternative retains canonical roots"), + }) + .collect(); + let strategy = asap_aware_mapping::current_series::CurrentSeriesStrategy::new(&roots); + let mut changed = false; + for (query, root) in maintained.queries.iter_mut().zip(&roots) { + if let Some(candidate) = strategy.candidate(root) { + query.post_asap = candidate; + changed = true; + } + } + if changed { maintained.hybrid_execution = false; maintained.materialization_policy = None; alternatives.push(maintained); diff --git a/data_plane/tests/support/current_series_process.rs b/data_plane/tests/support/current_series_process.rs index 08a5efa8..56f1377f 100644 --- a/data_plane/tests/support/current_series_process.rs +++ b/data_plane/tests/support/current_series_process.rs @@ -57,7 +57,17 @@ async fn current_series_quantiles_topk_share_and_replace_values() { let plan = PhysicalCompiler .compile(candidate.clone(), env.clone()) .ok()?; - let warm = candidate.current_series; + let warm = + candidate.queries.iter().all(|query| { + matches!( + &query.post_asap.expr, + planner_types::post_asap::SummaryExpr::ValueOperation { + operation: + planner_types::post_asap::ValueOperation::ReadCurrentSeries { .. }, + .. + } + ) + }); let manifest = workload_cost::manifest(&plan, &candidate.queries).unwrap(); Some(WorkloadQuote { unit_costs: manifest diff --git a/docs/developer_docs/query-engine/current-series-aggregations.md b/docs/developer_docs/query-engine/current-series-aggregations.md index 8a2622d1..f78793ba 100644 --- a/docs/developer_docs/query-engine/current-series-aggregations.md +++ b/docs/developer_docs/query-engine/current-series-aggregations.md @@ -6,6 +6,15 @@ alternative for `quantile(q, metric)` and `topk(k, metric)`, including `by` and literals. Selector offsets, `@`, nested input expressions and MetricsQL use the existing alternatives; they are not admitted by this implementation. +ASAPPlanner owns this transformation through the opt-in `CurrentSeriesStrategy` +over canonical IR. It emits `MaintainCurrentSeries` at maintenance time and +`ReadCurrentSeries` at read time, with source/filter/group identity, quantile +consumers and the maximum requested k in its typed contract. Compatible producers +are shared by Planner CSE. The backend consumes these nodes, binds resource and +input-lag limits, and retains the Planner DAG in the installed plan. It does not +recognize this optimization by reparsing the query text. Other deployments must +opt in only when they can implement and price this maintenance contract. + For example, put p50, p90, p95, p99 and Top1/Top5 in one workload. Matching source, selector and grouping contracts produce one `CurrentSeries` population with `quantiles: true` and `max_k: 5`. Each registered query keeps its own readout. From 12fb169ec619f5955f516cfd39fe14a1f0f4d6bb Mon Sep 17 00:00:00 2001 From: zz_y Date: Sat, 12 Sep 2026 13:45:02 -0600 Subject: [PATCH 3/8] Record compiler rule-boundary review findings --- .../compiler-rule-boundary-review.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/developer_docs/query-engine/compiler-rule-boundary-review.md diff --git a/docs/developer_docs/query-engine/compiler-rule-boundary-review.md b/docs/developer_docs/query-engine/compiler-rule-boundary-review.md new file mode 100644 index 00000000..62f47a49 --- /dev/null +++ b/docs/developer_docs/query-engine/compiler-rule-boundary-review.md @@ -0,0 +1,28 @@ +# Backend compiler rule-boundary review + +Scope: ASAPQuery-backend main `877ea128`, plus PR #700 (`1dcf299f`) and its current correction. Reviewed implementation and callers, not documentation claims. Findings below are code-level boundary violations or coupling; no untested claim of production wrong answers. + +## Findings + +1. **[P1] Legacy binding rewrites Rate into Increase before Planner selection.** `control_plane/src/physical/post_asap/lower.rs:171-214` calls `rewrite_rate_to_increase` before `select_summary`. This erases Rate from the selected logical IR and relies on downstream knowledge to recover division by the window. The caller in `control_plane/src/main.rs:1135-1162` remains reachable through the legacy algebra/typed-stage path. Share a physical counter implementation while preserving the logical Rate node/readout; if a logical rate-to-increase/divide transformation is needed, Planner must emit both operations. This is distinct from the main physical compiler's `physical_materialization_family`, which only maps the stored family and retains the selected readout. + +2. **[P1] Legacy metric-name policy invents aggregation semantics.** `control_plane/src/physical/workload_planner.rs:23-38,126-143,180-282` maps `top_endpoint_qps` to TopK and constructs a synthetic per-endpoint Count aggregate. It also chooses Quantile/Cardinality/Frequency from names rather than solely from the canonical query. `main.rs:1161` and `main.rs:1958` still call this adapter. Planner checks the fabricated intent's legality, not whether it describes the submitted query. Restrict such examples to fixtures, or require an explicit typed workload intent; never choose a statistic from a metric name. + +3. **[P2] Native residual lowering reruns Planner to rediscover the selected operator from text.** `control_plane/src/query_plan/logical.rs:537-620` parses original subexpressions and calls `select_summary_default` to construct an equality witness; `selected_aggregate_operator` then extracts the operation from that text. It fails closed on missing/ambiguous witnesses, which protects against blindly choosing an unrelated expression, but a valid selected DAG can become unlowerable when selection policy or representation changes. Carry sufficient explicit readout/operator information in Planner IR and lower that directly. The observed cross-series quantile failure in the old implementation was “native residual substitution requires an exact selected value”. + +4. **[P2] CandidateTopK can bypass its selected values child.** `control_plane/src/query_plan.rs:515-603` uses the original top-level PromQL aggregate's input as an `ExternalExact` request when `logical_source` is present, instead of lowering `values`. It retains Planner k/group/completeness and derives membership identity from the selected candidate, but does not establish equivalence between the generated expression and the selected values DAG. A future Planner rewrite of that child would not be faithfully reflected. Lower the selected exact child or carry a verified native-fragment binding in the input contract. + +5. **[P2] Legacy binding owns an unconditional TimeRange/Aggregate swap.** `control_plane/src/physical/post_asap/lower.rs:120-141` turns `TimeRange(Aggregate(X))` into `Aggregate(TimeRange(X))` for any aggregate at that position. This is a logical tree rewrite, with no local legality condition for the measure or grouping. Move any required canonicalization and its legality proof to Planner/frontend; the backend should consume the canonical shape. Reachable through the same legacy binding entry as finding 1; no production failure was reproduced in this audit. + +## PR #700 correction + +The original backend `current_series.rs` parsed PromQL and independently assembled a maintained-state alternative. The correction moves recognition and workload sharing into Planner's `CurrentSeriesStrategy`, adds typed maintenance/readout operators, removes `PlanningRequest.current_series`, and lowers the selected operators directly. Regression tests check that the installed Planner DAG contains the producer and that changing catalog text does not change physical operator binding. + +## Within the backend's responsibility + +- Complete workload quote comparison, placement, memory limits, input-lag bounds and runtime capability filtering. +- Choosing supported physical window implementations and subsets of retained materializations while preserving exact residual execution. +- Mapping Count/Sum or Rate/Increase to compatible physical storage while retaining the original logical readout. +- Explicit exact fallback for unsupported operators/accuracy/ERP evidence. A fallback may reduce acceleration coverage but is not itself a new logical rewrite rule. + +The other findings are review follow-ups, not silently bundled fixes. Retiring the legacy binding path and removing text-based residual reconstruction should be separate, focused changes with compatibility tests. From a9b4f58a6f9e02a6cf2d64d5780ad7ad3933d46d Mon Sep 17 00:00:00 2001 From: zz_y Date: Sat, 12 Sep 2026 13:51:18 -0600 Subject: [PATCH 4/8] Remove unused enumeration from merged process quote helper --- data_plane/tests/asapquery_compatibility_process_e2e.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/data_plane/tests/asapquery_compatibility_process_e2e.rs b/data_plane/tests/asapquery_compatibility_process_e2e.rs index f79a994e..f9023709 100644 --- a/data_plane/tests/asapquery_compatibility_process_e2e.rs +++ b/data_plane/tests/asapquery_compatibility_process_e2e.rs @@ -51,8 +51,7 @@ fn quote_snapshot_for_frontend_test( let quotes = workload_cost::with_exact_alternative(request) .unwrap() .into_iter() - .enumerate() - .filter_map(|(_index, candidate)| { + .filter_map(|candidate| { let plan = if metricsql { PhysicalCompiler.compile_metricsql(candidate.clone(), environment.clone()) } else { From 1f1228dee10f0b6d92b42b892c1c3c73b7504994 Mon Sep 17 00:00:00 2001 From: zz_y Date: Sat, 12 Sep 2026 14:16:49 -0600 Subject: [PATCH 5/8] Fix sliding snapshot reads and unsupported minimum fallback --- control_plane/src/clickhouse.rs | 2 + control_plane/src/physical/compiler.rs | 65 +++++- control_plane/src/query_plan.rs | 1 + control_plane/src/query_plan/logical.rs | 1 + crates/asap_types/src/plan_publication.rs | 8 + crates/asap_types/src/query_plan.rs | 9 + .../drivers/ingest/prometheus_remote_write.rs | 1 + .../accelerator.rs | 1 + .../asap_query_engine/exact_subqueries.rs | 1 + .../asap_query_engine/live_serve.rs | 1 + .../asap_query_engine/post_asap_readout.rs | 4 + .../asap_query_engine/summary_executor.rs | 194 +++++++++++------- .../asapquery_compatibility_process_e2e.rs | 1 + data_plane/tests/support/physical_fixture.rs | 1 + .../query-engine/issue-701-code-review.md | 68 ++++++ 15 files changed, 275 insertions(+), 83 deletions(-) create mode 100644 docs/developer_docs/query-engine/issue-701-code-review.md diff --git a/control_plane/src/clickhouse.rs b/control_plane/src/clickhouse.rs index 4abd29e6..8f3851e9 100644 --- a/control_plane/src/clickhouse.rs +++ b/control_plane/src/clickhouse.rs @@ -259,6 +259,7 @@ pub async fn compile_automatic_clickhouse_workload( let config = materialize_selected_sql(node, family, query) .map_err(crate::query_plan::QueryPlanError::Invalid)?; let binding = MaterializationBinding { + full_window_slide_ms: None, materialization: config.policy_fingerprint().into(), output_grouping: PhysicalGrouping::Reduce(config.grouping_labels.names()), window_ms: config.slide_interval * 1000, @@ -599,6 +600,7 @@ fn bind_selected_node( )); } Ok(MaterializationBinding { + full_window_slide_ms: None, materialization: selected.policy_fingerprint().into(), output_grouping: PhysicalGrouping::Reduce(selected.grouping_labels.names()), window_ms: selected.slide_interval.saturating_mul(1000), diff --git a/control_plane/src/physical/compiler.rs b/control_plane/src/physical/compiler.rs index 2b7bd073..bc0c42de 100644 --- a/control_plane/src/physical/compiler.rs +++ b/control_plane/src/physical/compiler.rs @@ -1622,6 +1622,8 @@ impl PhysicalCompiler { ))); } Ok(MaterializationBinding { + full_window_slide_ms: matches!(materialization.window_layout, asap_types::WindowMaterializationLayout::FullWindow) + .then_some(materialization.slide_interval.saturating_mul(1_000)), readout_lookback_ms: source_window.map(|seconds| seconds.saturating_mul(1_000)), materialization: fingerprint.into(), output_grouping: PhysicalGrouping::Reduce( @@ -1685,6 +1687,34 @@ impl PhysicalCompiler { instant, fallback: FallbackPolicy::ExactBackend, }) + } else if !request.hybrid_execution + && executable_dags[query_index].is_none() + && !collect_selected_materializations(&query.post_asap, request.hybrid_execution) + .map_err(|reason| CompileError::Query { + query_id: query.query_id.clone(), + reason, + })? + .is_empty() + { + // The selected state has no supported physical implementation. + // Keep native semantics instead of lowering an unbound summary. + let root = crate::query_plan::QueryNodeId(0); + Ok(crate::query_plan::QueryPlanEntry { + language: crate::query_plan::QueryLanguage::PromQl, + query_id: query.query_id.clone(), + canonical_query: canonical.clone(), + fixed_evaluation: None, + root, + nodes: BTreeMap::from([( + root, + crate::query_plan::QueryPlanNode::ExactFallback { + reason: "selected summary has no supported physical implementation" + .into(), + }, + )]), + instant, + fallback: FallbackPolicy::ExactBackend, + }) } else if request.hybrid_execution && !native_root { crate::query_plan::compile_bound_composable_mapped( query.query_id.clone(), @@ -3698,6 +3728,26 @@ pub(crate) mod tests { ); } + // Unsupported extrema state must retain exact routing instead of failing plan compilation (#701). + #[test] + fn unsupported_minimum_retains_native_execution() { + let mut env = environment(10_000); + env.target = PhysicalDeploymentTarget::BackendLocalRemoteWrite; + env.collector_ids.clear(); + for hybrid in [false, true] { + let mut input = request("minimum", "min_over_time(data[5m])"); + input.hybrid_execution = hybrid; + let plan = PhysicalCompiler.compile(input, env.clone()).unwrap(); + assert!(plan.precompute_plan.materializations.is_empty()); + assert!(plan.query_plan.entries.values().all(|entry| { + matches!(entry.nodes.get(&entry.root), Some(crate::query_plan::QueryPlanNode::ExactFallback { .. })) + || matches!(entry.nodes.get(&entry.root), Some(crate::query_plan::QueryPlanNode::Logical { + operator: asap_types::query_plan::logical::LogicalOperator::ExactSubquery { query }, .. + }) if query == "min_over_time(data[5m])") + })); + } + } + // Complete deployment quotes must preserve one producer with two window readouts. #[test] fn complete_cost_selection_preserves_shared_sum_panes() { @@ -6644,17 +6694,18 @@ pub(crate) mod tests { #[test] fn multiple_readouts_share_one_precompute_materialization() { let mut planning_request = request("q-p90", "quantile_over_time(0.90, m[1m])"); - let second = request("q-p99", "quantile_over_time(0.99, m[1m])") - .queries - .into_iter() - .next() - .unwrap(); - planning_request.queries.push(second); + for (id, q) in [("q-p50", 0.5), ("q-p95", 0.95), ("q-p99", 0.99)] { + planning_request.queries.push( + request(id, &format!("quantile_over_time({q}, m[1m])")) + .queries + .remove(0), + ); + } let bundle = PhysicalCompiler .compile(planning_request, environment(10_000)) .unwrap(); - assert_eq!(bundle.query_plan.entries.len(), 2); + assert_eq!(bundle.query_plan.entries.len(), 4); assert_eq!(bundle.summary_catalog.materializations.len(), 1); assert_eq!(bundle.precompute_plan.materializations.len(), 1); assert_eq!(bundle.precompute_plan.schemas.len(), 1); diff --git a/control_plane/src/query_plan.rs b/control_plane/src/query_plan.rs index bcce290d..fdb39f18 100644 --- a/control_plane/src/query_plan.rs +++ b/control_plane/src/query_plan.rs @@ -1266,6 +1266,7 @@ mod catalog_binding_tests { QueryNodeId(1), QueryPlanNode::ReadMaterialization { binding: MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: config.policy_fingerprint().into(), output_grouping: PhysicalGrouping::PerEntity, diff --git a/control_plane/src/query_plan/logical.rs b/control_plane/src/query_plan/logical.rs index e802ea4b..4baadd8e 100644 --- a/control_plane/src/query_plan/logical.rs +++ b/control_plane/src/query_plan/logical.rs @@ -647,6 +647,7 @@ mod hybrid_tests { crate::physical::compiler::materialization_leaf_contract(node) .map_err(QueryPlanError::Invalid)?; Ok(MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: asap_types::PolicyFingerprint( if spatial_filter.is_empty() { 7 } else { 8 }, diff --git a/crates/asap_types/src/plan_publication.rs b/crates/asap_types/src/plan_publication.rs index 35ce812f..d6496030 100644 --- a/crates/asap_types/src/plan_publication.rs +++ b/crates/asap_types/src/plan_publication.rs @@ -61,6 +61,14 @@ impl PhysicalPlanPublication { .get(&binding.materialization.fingerprint()) .copied() .ok_or("query binding has no precompute materialization")?; + let full_window_slide_ms = matches!( + config.window_layout, + crate::WindowMaterializationLayout::FullWindow + ) + .then_some(config.slide_interval.saturating_mul(1_000)); + if binding.full_window_slide_ms != full_window_slide_ms { + return Err("query window layout differs from precompute definition".into()); + } if config.stored_window_ms() != binding.window_ms { return Err("query pane differs from precompute stored window".into()); } diff --git a/crates/asap_types/src/query_plan.rs b/crates/asap_types/src/query_plan.rs index 8c08dabe..49d9adb1 100644 --- a/crates/asap_types/src/query_plan.rs +++ b/crates/asap_types/src/query_plan.rs @@ -429,6 +429,11 @@ impl QueryPlanEntry { } } if let QueryPlanNode::ReadMaterialization { binding } = node { + if binding.full_window_slide_ms.is_some_and(|slide| { + slide == 0 || slide > binding.window_ms || slide > i64::MAX as u64 + }) { + return Err(QueryPlanError::Invalid("invalid full-window slide".into())); + } if binding.readout_lookback_ms == Some(0) { return Err(QueryPlanError::Invalid( "zero semantic readout lookback".into(), @@ -465,6 +470,10 @@ pub enum FallbackPolicy { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(deny_unknown_fields)] pub struct MaterializationBinding { + /// Full-window snapshots must be read individually on this slide grid; + /// absence denotes non-overlapping pane composition. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub full_window_slide_ms: Option, pub materialization: SummaryDefinitionId, /// Query operator grouping applied while folding those SIDs. pub output_grouping: PhysicalGrouping, diff --git a/data_plane/src/drivers/ingest/prometheus_remote_write.rs b/data_plane/src/drivers/ingest/prometheus_remote_write.rs index 33531a27..14c9a70a 100644 --- a/data_plane/src/drivers/ingest/prometheus_remote_write.rs +++ b/data_plane/src/drivers/ingest/prometheus_remote_write.rs @@ -1620,6 +1620,7 @@ mod tests { .next() .unwrap(); let binding = asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, materialization: asap_types::PolicyFingerprint(policy).into(), output_grouping: asap_types::query_plan::PhysicalGrouping::Reduce(vec!["job".into()]), item_labels: vec![], diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/accelerator.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/accelerator.rs index c47a9c3f..88f2c629 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/accelerator.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/accelerator.rs @@ -584,6 +584,7 @@ mod tests { read, QueryPlanNode::ReadMaterialization { binding: MaterializationBinding { + full_window_slide_ms: None, materialization, output_grouping: PhysicalGrouping::Reduce(Vec::new()), item_labels: Vec::new(), diff --git a/data_plane/src/query_engines/asap_query_engine/exact_subqueries.rs b/data_plane/src/query_engines/asap_query_engine/exact_subqueries.rs index aa1537b3..c0ae6e71 100644 --- a/data_plane/src/query_engines/asap_query_engine/exact_subqueries.rs +++ b/data_plane/src/query_engines/asap_query_engine/exact_subqueries.rs @@ -936,6 +936,7 @@ mod tests { QueryNodeId(3), QueryPlanNode::ReadMaterialization { binding: MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: MATERIALIZATION.into(), output_grouping: PhysicalGrouping::Reduce(vec!["job".into()]), 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 ccfb7333..2d246efd 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 @@ -465,6 +465,7 @@ mod tests { asap_types::query_plan::QueryNodeId(1), asap_types::query_plan::QueryPlanNode::ReadMaterialization { binding: asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: policy.into(), output_grouping: asap_types::query_plan::PhysicalGrouping::PerEntity, diff --git a/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs b/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs index b91738c3..a0fba320 100644 --- a/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs +++ b/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs @@ -1000,6 +1000,7 @@ mod tests { QueryNodeId(1), QueryPlanNode::ReadMaterialization { binding: MaterializationBinding { + full_window_slide_ms: None, materialization: config.policy_fingerprint().into(), output_grouping: PhysicalGrouping::PerEntity, item_labels: vec![], @@ -1056,6 +1057,7 @@ mod tests { asap_types::query_plan::FallbackPolicy::ExactBackend, |_node, _family| { Ok(asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: asap_types::PolicyFingerprint(123).into(), output_grouping: asap_types::query_plan::PhysicalGrouping::PerEntity, @@ -1301,6 +1303,7 @@ mod tests { asap_types::query_plan::QueryNodeId(1), QueryPlanNode::ReadMaterialization { binding: asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: policy.into(), output_grouping: asap_types::query_plan::PhysicalGrouping::PerEntity, @@ -1397,6 +1400,7 @@ mod tests { asap_types::query_plan::QueryNodeId(1), QueryPlanNode::ReadMaterialization { binding: asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: policy.into(), output_grouping: asap_types::query_plan::PhysicalGrouping::PerEntity, 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 0b3583e4..6b0f22d6 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 @@ -416,7 +416,7 @@ fn validate_binding_phase( } planner_types::post_asap::validate_pane_coverage( &planner_types::post_asap::PanePhaseBinding { - pane_width_ms: binding.window_ms, + pane_width_ms: binding.full_window_slide_ms.unwrap_or(binding.window_ms), pane_origin_ms: binding.pane_origin_ms, }, i64::try_from(evaluation_ms).ok(), @@ -457,6 +457,12 @@ impl QueryExecutionContext<'_> { )); } validate_binding_phase(binding, self.t1_ms)?; + let full_window = binding.full_window_slide_ms.is_some(); + if full_window && self.t1_ms.saturating_sub(self.t0_ms) != binding.window_ms { + return Err(SummaryExecutorError::Unsupported( + "full-window snapshot requires its exact semantic range", + )); + } enum Candidate { Sketch(DeltaSketchKind), @@ -529,7 +535,7 @@ impl QueryExecutionContext<'_> { matched_metadata += 1; match candidate { Candidate::Sketch(kind) => { - let Some(series) = self + let Some(mut series) = self .index .query_range(sid, self.t0_ms, self.t1_ms) .into_iter() @@ -538,6 +544,14 @@ impl QueryExecutionContext<'_> { check_panes(Vec::new())?; continue; }; + if full_window { + // Overlap retrieval is useful for legacy queries, but merging + // overlapping snapshots counts observations repeatedly. + series.samples.retain(|end, _| *end == self.t1_ms as i64); + if series.samples.is_empty() { + return Err(SummaryExecutorError::NoCandidates); + } + } check_panes(series.samples.keys().copied().collect())?; let key = match &binding.output_grouping { PhysicalGrouping::PerEntity => series.series_label_values.clone(), @@ -1422,6 +1436,7 @@ mod tests { #[test] fn pane_only_reads_require_the_planned_evaluation_phase() { let binding = asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), materialization: asap_types::PolicyFingerprint(7).into(), output_grouping: asap_types::query_plan::PhysicalGrouping::PerEntity, @@ -1432,7 +1447,16 @@ mod tests { validate_binding_phase(&binding, 67_000).unwrap(); assert!(validate_binding_phase(&binding, 68_000).is_err()); + // A full snapshot can slide more frequently than its stored width. + let full = asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: Some(1_000), + ..binding.clone() + }; + validate_binding_phase(&full, 68_000).unwrap(); + assert!(validate_binding_phase(&full, 68_500).is_err()); + let legacy = asap_types::query_plan::MaterializationBinding { + full_window_slide_ms: None, item_labels: Vec::new(), pane_origin_ms: None, ..binding @@ -1831,83 +1855,101 @@ mod tests { use crate::storage_engines::sketch_db::index::SketchEncoding; use crate::storage_engines::types::SerializableToSink; use asap_types::query_plan::{MaterializationBinding, PhysicalGrouping}; - let index = SketchStore::new(); - let fp = asap_types::PolicyFingerprint(701); - let mut meta = kll_meta(1, "m", &["job"]); - meta.policy_fp = fp; - meta.agg_kind = AggKind::Sketch { - algorithm: SketchAlgorithm::UnivMon, - config: SketchConfig::UnivMon { - heap_size: 32, - sketch_rows: 5, - sketch_cols: 1024, - layers: 4, - }, - spatial_filter_canonical: String::new(), - }; - meta.accuracy = None; - meta.capability = Some(Capability::CardinalityApprox); - index.register(meta); - for (start, values) in [(0, [1.0, 2.0]), (1000, [2.0, 3.0])] { - let mut state = UnivMonAccumulator::new(32, 5, 1024, 4).unwrap(); - for value in values { - state.insert_sample(value).unwrap(); - } - index.append_sample( - 1, - BTreeMap::from([("job".into(), "a".into())]), - (start, start + 1000), - SketchSampleState { - bytes: state.serialize_to_bytes(), - encoding: SketchEncoding::MsgpackFull, - }, - ); - } - let context = QueryExecutionContext { - index: &index, - t0_ms: 0, - t1_ms: 2000, - is_cumulative: true, - allowed_materializations: Some(BTreeSet::from([fp])), - }; - let binding = MaterializationBinding { - materialization: fp.into(), - output_grouping: PhysicalGrouping::PerEntity, - item_labels: vec![], - window_ms: 1000, - pane_origin_ms: Some(0), - readout_lookback_ms: Some(2000), - }; - let states = context.read_bound_materialization(&binding).unwrap(); - assert_eq!(states.len(), 1); - assert_eq!(states[0].0.get("job").unwrap(), "a"); - for (query, expected) in [ - ( - SketchQuery::PointCount { - key: ColumnRef::SampleValue, - value: None, + for full_window in [false, true] { + let index = SketchStore::new(); + let fp = asap_types::PolicyFingerprint(701); + let mut meta = kll_meta(1, "m", &["job"]); + meta.policy_fp = fp; + meta.agg_kind = AggKind::Sketch { + algorithm: SketchAlgorithm::UnivMon, + config: SketchConfig::UnivMon { + heap_size: 32, + sketch_rows: 5, + sketch_cols: 1024, + layers: 4, }, - 4.0, - ), - (SketchQuery::Cardinality, 3.0), - (SketchQuery::FrequencyL2, 6.0f64.sqrt()), - (SketchQuery::FrequencyEntropy, 1.5), - ] { - let SummaryValue::Points(points, _) = - context.readout_bound(&states[0].1, &query).unwrap() - else { - panic!("expected scalar points") + spatial_filter_canonical: String::new(), }; - assert_eq!(points.len(), 1); - assert!( - (points[0].1 - expected).abs() < 0.05, - "{query:?}: {:?}", - points - ); + meta.accuracy = None; + meta.capability = Some(Capability::CardinalityApprox); + index.register(meta); + let windows = if full_window { + vec![(0, vec![99.0, 99.0]), (1000, vec![1.0, 2.0, 2.0, 3.0])] + } else { + vec![(0, vec![1.0, 2.0]), (1000, vec![2.0, 3.0])] + }; + for (start, values) in windows { + let mut state = UnivMonAccumulator::new(32, 5, 1024, 4).unwrap(); + for value in values { + state.insert_sample(value).unwrap(); + } + index.append_sample( + 1, + BTreeMap::from([("job".into(), "a".into())]), + (start, start + if full_window { 2000 } else { 1000 }), + SketchSampleState { + bytes: state.serialize_to_bytes(), + encoding: SketchEncoding::MsgpackFull, + }, + ); + } + let context = QueryExecutionContext { + index: &index, + t0_ms: if full_window { 1000 } else { 0 }, + t1_ms: if full_window { 3000 } else { 2000 }, + is_cumulative: true, + allowed_materializations: Some(BTreeSet::from([fp])), + }; + let binding = MaterializationBinding { + full_window_slide_ms: full_window.then_some(1000), + materialization: fp.into(), + output_grouping: PhysicalGrouping::PerEntity, + item_labels: vec![], + window_ms: if full_window { 2000 } else { 1000 }, + pane_origin_ms: Some(0), + readout_lookback_ms: Some(2000), + }; + let states = context.read_bound_materialization(&binding).unwrap(); + assert_eq!(states.len(), 1); + assert_eq!(states[0].0.get("job").unwrap(), "a"); + for (query, expected) in [ + ( + SketchQuery::PointCount { + key: ColumnRef::SampleValue, + value: None, + }, + 4.0, + ), + (SketchQuery::Cardinality, 3.0), + (SketchQuery::FrequencyL2, 6.0f64.sqrt()), + (SketchQuery::FrequencyEntropy, 1.5), + ] { + let SummaryValue::Points(points, _) = + context.readout_bound(&states[0].1, &query).unwrap() + else { + panic!("expected scalar points") + }; + assert_eq!(points.len(), 1); + assert!( + (points[0].1 - expected).abs() < 0.05, + "{query:?}: {:?}", + points + ); + } + if full_window { + let missing = QueryExecutionContext { + t0_ms: 2000, + t1_ms: 4000, + index: context.index, + is_cumulative: true, + allowed_materializations: context.allowed_materializations.clone(), + }; + assert!(missing.read_bound_materialization(&binding).is_err()); + } + let mut unknown = binding; + unknown.materialization = asap_types::PolicyFingerprint(702).into(); + assert!(context.read_bound_materialization(&unknown).is_err()); } - let mut unknown = binding; - unknown.materialization = asap_types::PolicyFingerprint(702).into(); - assert!(context.read_bound_materialization(&unknown).is_err()); } fn ctx(index: &SketchStore) -> QueryExecutionContext<'_> { diff --git a/data_plane/tests/asapquery_compatibility_process_e2e.rs b/data_plane/tests/asapquery_compatibility_process_e2e.rs index f9023709..505ebcf9 100644 --- a/data_plane/tests/asapquery_compatibility_process_e2e.rs +++ b/data_plane/tests/asapquery_compatibility_process_e2e.rs @@ -797,6 +797,7 @@ async fn run_shared_dashboard(multi_pane: bool) { let mut candidates = query.window_implementations; let mut small = candidates[0].clone(); small.implementation_id = "five-second-pane".into(); + small.slide_secs = 5; small.layout = asap_types::WindowMaterializationLayout::Pane { pane_secs: 5 }; small.cost.weighted_cost = 0.0; candidates[0].cost.weighted_cost = 10.0; diff --git a/data_plane/tests/support/physical_fixture.rs b/data_plane/tests/support/physical_fixture.rs index e896e9ee..b06e8bd2 100644 --- a/data_plane/tests/support/physical_fixture.rs +++ b/data_plane/tests/support/physical_fixture.rs @@ -118,6 +118,7 @@ pub fn artifact(config: &StreamingConfig) -> PhysicalPlanInstallRequest { QueryNodeId(0), QueryPlanNode::ReadMaterialization { binding: MaterializationBinding { + full_window_slide_ms: None, materialization: config.policy_fingerprint().into(), output_grouping, item_labels: config.aggregated_labels.labels.clone(), diff --git a/docs/developer_docs/query-engine/issue-701-code-review.md b/docs/developer_docs/query-engine/issue-701-code-review.md new file mode 100644 index 00000000..60f5ac9f --- /dev/null +++ b/docs/developer_docs/query-engine/issue-701-code-review.md @@ -0,0 +1,68 @@ +# Issue 701: windows, extrema, and composed quantile guarantees + +Reviewed against backend main `674b9573` plus PR 700 and Planner PR 404 +(`ea721e889b79e9ca22741a4d0370e9929bcf5b89`). The issue's original output was +produced by the removed v1 inspection path. This review uses executable code. + +## Window identity and runtime reads + +The current compiler derives pane and full-window alternatives from the query +lookback and evaluation cadence when the cadence divides the lookback. Thus a +300-second lookback evaluated every 30 seconds need not use tumbling 300-second +state. Complete workload cost evidence still determines the selected alternative. + +A separate runtime defect remained: full-window snapshots of width 5 seconds, +sliding every second, were retrieved through an overlap scan and cumulatively +merged. A process TopK regression returned 1500 instead of 200 for one item. +The query binding now carries the full-window slide explicitly, validates it +against the installed producer, checks evaluation phase on the slide grid, and +reads only the snapshot ending at the requested evaluation time. Missing snapshots +and incompatible ranges fail closed. Pane reads retain non-overlapping composition. +Old full-window artifacts without this binding must be recompiled before install. + +## min_over_time + +Planner currently represents Min and Max using the same ExactKind::MinMax family. +The backend's materialized readout supports Max; the physical compiler filters +out unsupported Min state. The legacy non-composable lowering then attempted to +bind the removed state and failed with `materialized query has no compiled +executable DAG`. + +The fix retains an explicit native fallback when all selected state lacks a +physical implementation. It does not reinterpret Min as Max or claim accelerated +Min support. Composable lowering retains its existing native dependency behavior. +Full accelerated Min needs an unambiguous Planner readout contract and matching +backend lowering, maintenance, and serving support. + +## Quantiles and division + +Multiple quantiles of the same population can share one maintained sketch. +The compiler regression covers q=0.5, 0.9, 0.95, and 0.99 with one materialization. +This does not imply the ratio inherits a component's relative-error bound. + +If both nonzero quantile values have relative errors at most alpha, the ratio +of their estimates differs from the true ratio by at most +`2 * alpha / (1 - alpha)` in relative terms. No independence assumption is used. +At alpha=1%, this sufficient bound is about 2.0202%. A sufficient component bound +for a 1% ratio target is alpha <= `0.01 / 2.01`, about 0.4975%, together with a +valid nonzero-denominator/domain contract. Sharing a sketch alone supplies no +proof of cancellation. Probabilistic guarantees also need a joint success bound; +rank error, such as a KLL guarantee, is not relative value error. + +Planner currently lacks this domain-aware division proof and conservatively +retains native execution. Even adding the formula would not make 1%-component +sketches satisfy a requested 1% ratio guarantee in general. + +## Remaining integration failures + +After the snapshot-read fix, the compatibility process suite reports 10 passing, +3 failing, and 1 previously ignored Collector-schema test. The failures are: + +- Counter range execution rejects missing full-pane coverage. +- Finite persisted-summary drain reports unpublished summary windows. +- UnivMon producer observations do not select UnivMon on replanning. + +These remain open; this change does not claim the complete compatibility matrix +passes. The three previously failing window/TopK-related executions include two +TopK algorithms and a multi-pane fixture whose explicit slide required updating. +No latency or end-to-end speedup measurement is claimed here. From 35b74bdf39984dec2da48cf11f512b6c1538a8e1 Mon Sep 17 00:00:00 2001 From: zz_y Date: Sat, 12 Sep 2026 16:20:57 -0600 Subject: [PATCH 6/8] Lower Planner aggregate rules and preserve query window populations --- Cargo.lock | 10 +- Cargo.toml | 8 +- control_plane/src/asap_tier_implement.rs | 16 +- control_plane/src/emit/stage_config.rs | 4 +- control_plane/src/physical/compiler.rs | 102 ++++-- control_plane/src/physical/current_series.rs | 3 + .../src/physical/post_asap/matcher.rs | 1 + control_plane/src/physical/workload_cost.rs | 47 ++- control_plane/src/query_plan.rs | 44 ++- control_plane/src/query_plan/logical.rs | 16 + control_plane/src/query_planning.rs | 3 +- control_plane/src/replan.rs | 1 + control_plane/tests/offline_evidence.rs | 1 + crates/asap_types/src/accumulator_spec.rs | 32 +- crates/asap_types/src/precompute_plan.rs | 2 + crates/asap_types/src/query_plan.rs | 1 + .../src/query_plan/current_series.rs | 3 + crates/asap_types/src/query_plan/logical.rs | 2 + .../precompute_engine/accumulator_factory.rs | 10 +- .../precompute_engine/maintenance_runtime.rs | 1 + .../src/precompute_engine/subdag_scheduler.rs | 1 + .../src/precompute_engine/window_manager.rs | 15 + .../asap_query_engine/catalog_resolver.rs | 4 + .../query_engines/asap_query_engine/engine.rs | 82 ++--- .../asap_query_engine/live_serve.rs | 25 +- .../asap_query_engine/logical_dag.rs | 56 +++ .../asap_query_engine/post_asap_readout.rs | 48 +-- .../asap_query_engine/summary_exec.rs | 1 + .../asap_query_engine/summary_executor.rs | 78 +++- .../sketch_db/current_series.rs | 72 +++- .../sketch_db/index/admission.rs | 120 +++++- .../storage_engines/sketch_db/index/mod.rs | 21 +- .../asapquery_compatibility_process_e2e.rs | 14 +- .../tests/support/current_series_process.rs | 44 ++- .../tests/support/issue_701_702_process.rs | 343 ++++++++++++++++++ .../tests/support/univmon_erp_process.rs | 5 +- .../query-engine/issue-701-code-review.md | 165 +++++---- 37 files changed, 1094 insertions(+), 307 deletions(-) create mode 100644 data_plane/tests/support/issue_701_702_process.rs diff --git a/Cargo.lock b/Cargo.lock index 2163e7c3..fc79427f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -373,7 +373,7 @@ dependencies = [ [[package]] name = "asap-aware-mapping" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" dependencies = [ "asap-types", "serde", @@ -384,7 +384,7 @@ dependencies = [ [[package]] name = "asap-frontend-promql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" dependencies = [ "asap-types", "promql-parser 0.10.0 (git+https://github.com/ProjectASAP/promql-parser?rev=9fede7eecca923c9882fe256484d00d37f8706cb)", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -416,12 +416,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=ea721e889b79e9ca22741a4d0370e9929bcf5b89#ea721e889b79e9ca22741a4d0370e9929bcf5b89" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 4a71a3ae..d3b18abc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,10 +20,10 @@ asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", branch [workspace.dependencies] # Keep Planner frontends, selection, and IR on the same immutable revision (current-series Planner PR). # Alias upstream asap-types because this workspace also defines asap_types. -planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } -asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } -asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } -asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "ea721e889b79e9ca22741a4d0370e9929bcf5b89" } +planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } +asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } +asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } +asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } # Shared external deps (used by 2+ crates) serde = { version = "1.0", features = ["derive"] } diff --git a/control_plane/src/asap_tier_implement.rs b/control_plane/src/asap_tier_implement.rs index 02809866..00d80d92 100644 --- a/control_plane/src/asap_tier_implement.rs +++ b/control_plane/src/asap_tier_implement.rs @@ -258,21 +258,13 @@ mod tests { } #[test] - fn avg_over_time_is_not_yet_realizable_matching_capability_for_today() { - // Avg = Sum / Count needs a cross-policy join implement_tree_in_with - // doesn't build (matches capability_for(&AggIntent::Avg) => None - // on the flat path -- see asap_tier_analysis.rs and lower.rs's - // AggFunc::Avg comment). Use avg_over_time (a range-vector - // function), not bare instant avg(...) -- only the former is - // guaranteed to lower through AggFunc::Avg in this frontend. + fn avg_over_time_realizes_exact_sum_divided_by_count() { let roots = implement_promql_for_asap_tier("avg_over_time(http_requests_total[5m])") .expect("parses and implements"); assert_eq!(roots.len(), 1); - assert!( - matches!(roots[0].expr, SummaryExpr::KeepPreAsap(_)), - "Avg has no ASAP-tier realization yet on either path: {:?}", - roots[0].expr, - ); + assert!(matches!(roots[0].expr, SummaryExpr::BinaryOp { .. })); + assert!(roots[0].guarantee.as_ref().unwrap().is_exact()); + planner_types::post_asap::compile_executable_dag(&roots[0]).unwrap(); } #[test] diff --git a/control_plane/src/emit/stage_config.rs b/control_plane/src/emit/stage_config.rs index 6d94e164..2cd88fd4 100644 --- a/control_plane/src/emit/stage_config.rs +++ b/control_plane/src/emit/stage_config.rs @@ -2969,7 +2969,7 @@ pub(crate) fn build_backend_aggregation_json(agg: &BackendAggregation) -> JsonVa match kind { ExactKind::Sum => "Sum", ExactKind::Count => "Count", - ExactKind::MinMax => "MinMax", + ExactKind::MinMax | ExactKind::Min => "MinMax", ExactKind::Increase => "Increase", ExactKind::Rate => "Rate", ExactKind::IRate => "IRate", @@ -3029,7 +3029,7 @@ pub(crate) fn build_backend_aggregation_json(agg: &BackendAggregation) -> JsonVa planner_types::post_asap::ExactKind::MinMax, _ ) - ) { "max" } else { "" }, + ) { "max" } else if matches!(&agg.family, planner_types::post_asap::SummaryFamilyType::ExactAggregate(planner_types::post_asap::ExactKind::Min, _)) { "min" } else { "" }, "metric": agg.metric_name, "labels": { "grouping": agg.grouping, diff --git a/control_plane/src/physical/compiler.rs b/control_plane/src/physical/compiler.rs index bc0c42de..48e91c7e 100644 --- a/control_plane/src/physical/compiler.rs +++ b/control_plane/src/physical/compiler.rs @@ -780,7 +780,7 @@ fn has_unsafe_raw_entity_leaf( let preserves_series_state = matches!( family, SummaryFamilyType::ExactAggregate( - ExactKind::Increase | ExactKind::Rate | ExactKind::MinMax, + ExactKind::Increase | ExactKind::Rate | ExactKind::MinMax | ExactKind::Min, _ ) ); @@ -1096,19 +1096,6 @@ impl PhysicalCompiler { .iter() .any(|candidate| candidate.window_secs == window) })) - && (!matches!( - state.family, - SummaryFamilyType::ExactAggregate( - planner_types::post_asap::ExactKind::MinMax, - _ - ) - ) || crate::query_plan::logical::selected_range_max_materialization( - &query.query_string, - &state.node, - ) - .ok() - .flatten() - .is_some()) && request.materialization_policy.as_ref().is_none_or(|policy| { let key = crate::query_plan::logical::selected_counter_materialization( &query.query_string, @@ -1749,7 +1736,11 @@ impl PhysicalCompiler { } }, ) - }?; + } + .map_err(|error| CompileError::Query { + query_id: query.query_string.clone(), + reason: error.to_string(), + })?; if request.hybrid_execution { // Any Planner-selected leaf without a physical summary binding // is an exact subtree boundary. Deployed plans never retain a @@ -3728,24 +3719,31 @@ pub(crate) mod tests { ); } - // Unsupported extrema state must retain exact routing instead of failing plan compilation (#701). + // The Planner's minimum state lowers without reconstructing direction from text. #[test] - fn unsupported_minimum_retains_native_execution() { + fn minimum_retains_its_typed_direction() { let mut env = environment(10_000); env.target = PhysicalDeploymentTarget::BackendLocalRemoteWrite; env.collector_ids.clear(); - for hybrid in [false, true] { - let mut input = request("minimum", "min_over_time(data[5m])"); - input.hybrid_execution = hybrid; - let plan = PhysicalCompiler.compile(input, env.clone()).unwrap(); - assert!(plan.precompute_plan.materializations.is_empty()); - assert!(plan.query_plan.entries.values().all(|entry| { - matches!(entry.nodes.get(&entry.root), Some(crate::query_plan::QueryPlanNode::ExactFallback { .. })) - || matches!(entry.nodes.get(&entry.root), Some(crate::query_plan::QueryPlanNode::Logical { - operator: asap_types::query_plan::logical::LogicalOperator::ExactSubquery { query }, .. - }) if query == "min_over_time(data[5m])") - })); - } + let mut input = request("minimum", "min_over_time(data[1m])"); + input.hybrid_execution = true; + let plan = PhysicalCompiler.compile(input, env).unwrap(); + assert_eq!(plan.precompute_plan.materializations.len(), 1); + assert_eq!( + plan.precompute_plan.materializations[0].aggregation_sub_type, + "min" + ); + assert!(plan + .query_plan + .entries + .values() + .all(|entry| entry.nodes.values().any(|node| matches!( + node, + crate::query_plan::QueryPlanNode::ExactReadout { + readout: crate::query_plan::ExactReadout::Min, + .. + } + )))); } // Complete deployment quotes must preserve one producer with two window readouts. @@ -3819,6 +3817,47 @@ pub(crate) mod tests { snapshot } + // Issue workloads must expose executable maintained candidates under the schema-2 API. + #[test] + fn issue_701_702_temporal_workloads_have_warm_candidates() { + for text in [ + "avg_over_time(data[5m])", + "min_over_time(data[5m])", + "quantile_over_time(0.9,data[5m])/quantile_over_time(0.5,data[5m])", + "avg_over_time(data[5m])/quantile_over_time(0.5,data[5m])", + ] { + let mut snapshot = planning_snapshot(); + let entry = &mut snapshot.query_workload.repeating_queries.as_mut().unwrap()[0]; + entry.query = Query(text.into()); + entry.time_selection.lookback = Some(DurationMs(300_000)); + if !text.contains("quantile") { + entry.requirements.accuracy = AccuracyRequirement::Explicit(AccuracyTarget::Exact); + } + let (request, environment) = snapshot.planning_request().unwrap(); + let candidates = super::super::workload_cost::with_exact_alternative(request).unwrap(); + let mut reasons = vec![]; + assert!( + candidates.into_iter().any(|candidate| { + match PhysicalCompiler.compile(candidate, environment.clone()) { + Ok(plan) => { + !plan.precompute_plan.materializations.is_empty() + && plan + .query_plan + .entries + .values() + .all(|entry| !entry.materialization_bindings().is_empty()) + } + Err(error) => { + reasons.push(error.to_string()); + false + } + } + }), + "no warm candidate for {text}: {reasons:?}" + ); + } + } + /// Optional counter masks must retain the workload's mandatory sketch bindings. #[test] fn costed_mixed_workload_retains_sketches_and_counter_readouts() { @@ -4563,7 +4602,9 @@ pub(crate) mod tests { let mut workload = request("confidence", "distinct_over_time(m[1m])"); workload.hybrid_execution = true; let result = PhysicalCompiler.compile_metricsql(workload, deployment); - assert!(matches!(result, Err(CompileError::QueryPlan(_)))); + assert!( + matches!(result, Err(CompileError::Query { reason, .. }) if reason.contains("native residual substitution requires an exact selected value")) + ); } #[test] @@ -5368,6 +5409,7 @@ pub(crate) mod tests { lhs: selected.clone(), rhs: selected.clone(), operator: planner_types::post_asap::BinaryOperator { + checked_relative_division: false, kind: planner_types::pre_asap::BinaryOpKind::Arithmetic( planner_types::pre_asap::ArithmeticOpKind::Add, ), diff --git a/control_plane/src/physical/current_series.rs b/control_plane/src/physical/current_series.rs index 3dbcb574..2756d68d 100644 --- a/control_plane/src/physical/current_series.rs +++ b/control_plane/src/physical/current_series.rs @@ -87,6 +87,9 @@ pub(super) fn operator( let readout = match readout { CurrentSeriesReadout::Quantile { q } => SeriesReadout::Quantile { q: *q }, CurrentSeriesReadout::TopK { k } => SeriesReadout::TopK { k: *k as u64 }, + CurrentSeriesReadout::Sum => SeriesReadout::Sum, + CurrentSeriesReadout::Count => SeriesReadout::Count, + CurrentSeriesReadout::Average => SeriesReadout::Average, }; Ok(Some(LogicalOperator::CurrentSeries { population, diff --git a/control_plane/src/physical/post_asap/matcher.rs b/control_plane/src/physical/post_asap/matcher.rs index f5a671e8..01c5c8a1 100644 --- a/control_plane/src/physical/post_asap/matcher.rs +++ b/control_plane/src/physical/post_asap/matcher.rs @@ -193,6 +193,7 @@ mod tests { ExactKind::Sum => ExactParams::Sum, ExactKind::Count => ExactParams::Count, ExactKind::MinMax => ExactParams::MinMax, + ExactKind::Min => ExactParams::Min, ExactKind::Increase => ExactParams::Increase, ExactKind::Rate => ExactParams::Rate, ExactKind::IRate => ExactParams::IRate, diff --git a/control_plane/src/physical/workload_cost.rs b/control_plane/src/physical/workload_cost.rs index 36e58ba4..9b852f9d 100644 --- a/control_plane/src/physical/workload_cost.rs +++ b/control_plane/src/physical/workload_cost.rs @@ -685,8 +685,9 @@ pub fn with_exact_alternative( if already_selected { return Ok(alternatives); } - let mut maintained = alternatives.last().expect("exact alternative").clone(); - let roots: Vec<_> = maintained + let roots: Vec<_> = alternatives + .last() + .expect("exact alternative") .queries .iter() .map(|q| match &q.post_asap.expr { @@ -695,18 +696,40 @@ pub fn with_exact_alternative( }) .collect(); let strategy = asap_aware_mapping::current_series::CurrentSeriesStrategy::new(&roots); - let mut changed = false; - for (query, root) in maintained.queries.iter_mut().zip(&roots) { - if let Some(candidate) = strategy.candidate(root) { - query.post_asap = candidate; - changed = true; + let candidates: Vec<_> = roots.iter().map(|root| strategy.candidate(root)).collect(); + if candidates.iter().any(Option::is_some) { + // Current-series rules are compatible with window summaries in other + // workload roots. Preserve each priced temporal alternative and mask. + let maintained: Vec<_> = alternatives + .iter() + .map(|alternative| { + let mut candidate = alternative.clone(); + for (query, selected) in candidate.queries.iter_mut().zip(&candidates) { + if let Some(selected) = selected { + query.post_asap = std::rc::Rc::clone(selected); + } + } + if candidates.iter().all(Option::is_some) { + candidate.hybrid_execution = false; + candidate.materialization_policy = None; + } + candidate + }) + .collect(); + for candidate in maintained { + if !alternatives.iter().any(|existing| { + existing.hybrid_execution == candidate.hybrid_execution + && existing.materialization_policy == candidate.materialization_policy + && existing + .queries + .iter() + .zip(&candidate.queries) + .all(|(a, b)| a.post_asap == b.post_asap) + }) { + alternatives.push(candidate); + } } } - if changed { - maintained.hybrid_execution = false; - maintained.materialization_policy = None; - alternatives.push(maintained); - } Ok(alternatives) } diff --git a/control_plane/src/query_plan.rs b/control_plane/src/query_plan.rs index fdb39f18..182049c8 100644 --- a/control_plane/src/query_plan.rs +++ b/control_plane/src/query_plan.rs @@ -390,11 +390,12 @@ where } if measures.len() == 1 => { use planner_types::pre_asap::AggIntent; let operation = match &measures[0] { - AggIntent::Sum { .. } => logical::Aggregation::Sum, - AggIntent::Count { .. } => logical::Aggregation::Count, - AggIntent::Min { .. } => logical::Aggregation::Min, - AggIntent::Max { .. } => logical::Aggregation::Max, - AggIntent::Avg { .. } => logical::Aggregation::Avg, + AggIntent::Sum { .. } => Some(logical::Aggregation::Sum), + AggIntent::Count { .. } => Some(logical::Aggregation::Count), + AggIntent::Min { .. } => Some(logical::Aggregation::Min), + AggIntent::Max { .. } => Some(logical::Aggregation::Max), + AggIntent::Avg { .. } => Some(logical::Aggregation::Avg), + AggIntent::TopK { .. } => None, _ => { return Err(QueryPlanError::Invalid( "unsupported exact value aggregation".into(), @@ -422,14 +423,23 @@ where }) }) .collect::, _>>()?; + let grouping = logical::Grouping { + labels, + without: keys.is_without(), + }; + let operator = if let AggIntent::TopK { k, .. } = &measures[0] { + logical::LogicalOperator::TopKSelection { + k: *k as u64, + grouping, + } + } else { + logical::LogicalOperator::Aggregate { + operation: operation.expect("aggregate operation"), + grouping, + } + }; QueryPlanNode::Logical { - operator: logical::LogicalOperator::Aggregate { - operation, - grouping: logical::Grouping { - labels, - without: keys.is_without(), - }, - }, + operator, inputs: vec![self.lower(child)?], } } @@ -611,7 +621,7 @@ where rhs, operator, timing: planner_types::post_asap::ExecutionTiming::ReadTime, - } if self.logical_source.is_some() => { + } if self.logical_source.is_some() || operator.checked_relative_division => { let operator = logical::binary_operator(operator)?; QueryPlanNode::Logical { operator, @@ -887,6 +897,7 @@ fn exact_readout(family: &SummaryFamilyType) -> Option { SummaryFamilyType::ExactAggregate(ExactKind::Increase, _) => Some(ExactReadout::Increase), SummaryFamilyType::ExactAggregate(ExactKind::Rate, _) => Some(ExactReadout::Rate), SummaryFamilyType::ExactAggregate(ExactKind::MinMax, _) => Some(ExactReadout::Max), + SummaryFamilyType::ExactAggregate(ExactKind::Min, _) => Some(ExactReadout::Min), _ => None, } } @@ -970,7 +981,12 @@ pub(crate) fn exact_value_executable(node: &SummaryNode) -> bool { && matches!(reduction, Reduction::PerEntity) && matches!( kind, - ExactKind::Sum | ExactKind::Count | ExactKind::Increase | ExactKind::Rate + ExactKind::Sum + | ExactKind::Count + | ExactKind::Increase + | ExactKind::Rate + | ExactKind::Min + | ExactKind::MinMax ) } else { // Raw producer grouping may move through additive reductions, diff --git a/control_plane/src/query_plan/logical.rs b/control_plane/src/query_plan/logical.rs index 4baadd8e..73dfc330 100644 --- a/control_plane/src/query_plan/logical.rs +++ b/control_plane/src/query_plan/logical.rs @@ -342,6 +342,22 @@ pub(super) fn residual_nodes( pub(super) fn binary_operator( operator: &planner_types::post_asap::BinaryOperator, ) -> Result { + if operator.checked_relative_division { + if operator.vector_match.is_some() + || !matches!( + operator.kind, + planner_types::pre_asap::BinaryOpKind::Arithmetic( + planner_types::pre_asap::ArithmeticOpKind::Div + ) + ) + { + return Err(invalid("invalid Planner checked division contract")); + } + return Ok(LogicalOperator::Binary { + operation: BinaryOperation::CheckedDiv, + return_bool: false, + }); + } if operator.vector_match.is_some() { return Err(invalid("explicit residual vector matching unsupported")); } diff --git a/control_plane/src/query_planning.rs b/control_plane/src/query_planning.rs index 87bc1704..db925755 100644 --- a/control_plane/src/query_planning.rs +++ b/control_plane/src/query_planning.rs @@ -215,7 +215,8 @@ fn planned_capability( | planner_types::post_asap::ExactKind::IRate => { asap_types::AggregationType::Increase } - planner_types::post_asap::ExactKind::MinMax => asap_types::AggregationType::MinMax, + planner_types::post_asap::ExactKind::MinMax + | planner_types::post_asap::ExactKind::Min => asap_types::AggregationType::MinMax, }; Capability::ExactAgg(agg) } diff --git a/control_plane/src/replan.rs b/control_plane/src/replan.rs index 4ee23c58..fb7311de 100644 --- a/control_plane/src/replan.rs +++ b/control_plane/src/replan.rs @@ -662,6 +662,7 @@ impl Replanner { planner_types::post_asap::ExactKind::Count => { planner_types::post_asap::ExactParams::Count } + planner_types::post_asap::ExactKind::Min => planner_types::post_asap::ExactParams::Min, planner_types::post_asap::ExactKind::MinMax => { planner_types::post_asap::ExactParams::MinMax } diff --git a/control_plane/tests/offline_evidence.rs b/control_plane/tests/offline_evidence.rs index 0c4a936b..ce2338c6 100644 --- a/control_plane/tests/offline_evidence.rs +++ b/control_plane/tests/offline_evidence.rs @@ -355,6 +355,7 @@ fn binary_summary_has_explicit_warm_tier_fallback() { lhs: child.clone(), rhs: child.clone(), operator: BinaryOperator { + checked_relative_division: false, kind: BinaryOpKind::Arithmetic(planner_types::pre_asap::ArithmeticOpKind::Div), vector_match: None, }, diff --git a/crates/asap_types/src/accumulator_spec.rs b/crates/asap_types/src/accumulator_spec.rs index 5e6202b6..ab3fc669 100644 --- a/crates/asap_types/src/accumulator_spec.rs +++ b/crates/asap_types/src/accumulator_spec.rs @@ -41,11 +41,9 @@ //! //! Backend-specific execution details remain deliberately separate: //! -//! - **Min/max direction.** Planner's `ExactParams::MinMax` carries no fields — -//! upstream doesn't model a direction axis. `accumulator_factory.rs` -//! keeps reading `AggregationConfig::aggregation_sub_type` directly -//! for this one bit (`eq_ignore_ascii_case("max")`), exactly as it did -//! before this refactor. +//! - **Min/max direction.** Planner's `ExactKind::Min` and legacy maximum +//! `ExactKind::MinMax` map to the shared wire accumulator with an explicit +//! aggregation subtype. The physical compiler preserves this typed direction. //! - **HydraKLL's `(row, col)` tiling.** `SketchParams::Kll` carries //! only `k` — upstream has no concept of the CMS-like grid-of-KLL-cells //! layout `HydraKllSketchAccumulator` uses to parallelize a keyed KLL @@ -234,7 +232,11 @@ impl AggregationConfig { false, ), MinMax => ( - SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax), + if sub_type.eq_ignore_ascii_case("min") { + SummaryFamilyType::ExactAggregate(ExactKind::Min, ExactParams::Min) + } else { + SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax) + }, false, ), DatasketchesKLL => ( @@ -255,7 +257,11 @@ impl AggregationConfig { true, ), MultipleMinMax => ( - SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax), + if sub_type.eq_ignore_ascii_case("min") { + SummaryFamilyType::ExactAggregate(ExactKind::Min, ExactParams::Min) + } else { + SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax) + }, true, ), HydraKLL => { @@ -391,7 +397,11 @@ impl AggregationConfig { false, ), "Min" | "min" | "Max" | "max" => ( - SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax), + if sub_type.eq_ignore_ascii_case("min") { + SummaryFamilyType::ExactAggregate(ExactKind::Min, ExactParams::Min) + } else { + SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax) + }, false, ), "Increase" | "increase" => ( @@ -419,7 +429,11 @@ impl AggregationConfig { true, ), "Min" | "min" | "Max" | "max" => ( - SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax), + if sub_type.eq_ignore_ascii_case("min") { + SummaryFamilyType::ExactAggregate(ExactKind::Min, ExactParams::Min) + } else { + SummaryFamilyType::ExactAggregate(ExactKind::MinMax, ExactParams::MinMax) + }, true, ), "Increase" | "increase" => ( diff --git a/crates/asap_types/src/precompute_plan.rs b/crates/asap_types/src/precompute_plan.rs index 4a6ce9d7..554fe269 100644 --- a/crates/asap_types/src/precompute_plan.rs +++ b/crates/asap_types/src/precompute_plan.rs @@ -178,6 +178,7 @@ pub enum ExactStateKind { Sum, Count, MinMax, + Min, Increase, Rate, IRate, @@ -194,6 +195,7 @@ impl TryFrom<&SummaryFamilyType> for StateFamilyContract { ExactKind::Sum => ExactStateKind::Sum, ExactKind::Count => ExactStateKind::Count, ExactKind::MinMax => ExactStateKind::MinMax, + ExactKind::Min => ExactStateKind::Min, ExactKind::Increase => ExactStateKind::Increase, ExactKind::Rate => ExactStateKind::Rate, ExactKind::IRate => ExactStateKind::IRate, diff --git a/crates/asap_types/src/query_plan.rs b/crates/asap_types/src/query_plan.rs index 49d9adb1..c67454d5 100644 --- a/crates/asap_types/src/query_plan.rs +++ b/crates/asap_types/src/query_plan.rs @@ -638,6 +638,7 @@ pub enum ExactReadout { Increase, Rate, Max, + Min, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] diff --git a/crates/asap_types/src/query_plan/current_series.rs b/crates/asap_types/src/query_plan/current_series.rs index d8acee5f..8dc53abf 100644 --- a/crates/asap_types/src/query_plan/current_series.rs +++ b/crates/asap_types/src/query_plan/current_series.rs @@ -46,4 +46,7 @@ impl SeriesPopulation { pub enum SeriesReadout { Quantile { q: f64 }, TopK { k: u64 }, + Sum, + Count, + Average, } diff --git a/crates/asap_types/src/query_plan/logical.rs b/crates/asap_types/src/query_plan/logical.rs index 0615b7e8..b0064bb8 100644 --- a/crates/asap_types/src/query_plan/logical.rs +++ b/crates/asap_types/src/query_plan/logical.rs @@ -97,6 +97,8 @@ pub enum BinaryOperation { Sub, Mul, Div, + /// Division with the Planner relative-value certificate domain checks. + CheckedDiv, Mod, Pow, Equal, diff --git a/data_plane/src/precompute_engine/accumulator_factory.rs b/data_plane/src/precompute_engine/accumulator_factory.rs index 81ae8fb9..8056ff1d 100644 --- a/data_plane/src/precompute_engine/accumulator_factory.rs +++ b/data_plane/src/precompute_engine/accumulator_factory.rs @@ -1067,10 +1067,12 @@ pub fn create_accumulator_updater(config: &AggregationConfig) -> Box Box::new( - MinMaxAccumulatorUpdater::new(config.aggregation_sub_type.eq_ignore_ascii_case("max")), - ), - (SummaryFamilyType::ExactAggregate(ExactKind::MinMax, _), true) => { + (SummaryFamilyType::ExactAggregate(ExactKind::MinMax | ExactKind::Min, _), false) => { + Box::new(MinMaxAccumulatorUpdater::new( + config.aggregation_sub_type.eq_ignore_ascii_case("max"), + )) + } + (SummaryFamilyType::ExactAggregate(ExactKind::MinMax | ExactKind::Min, _), true) => { Box::new(MultipleMinMaxAccumulatorUpdater::new( config.aggregation_sub_type.eq_ignore_ascii_case("max"), )) diff --git a/data_plane/src/precompute_engine/maintenance_runtime.rs b/data_plane/src/precompute_engine/maintenance_runtime.rs index ab1f35d4..e570579b 100644 --- a/data_plane/src/precompute_engine/maintenance_runtime.rs +++ b/data_plane/src/precompute_engine/maintenance_runtime.rs @@ -3255,6 +3255,7 @@ mod tests { ]; operation.output_schema.time_index = Some(0); let mut operator = BinaryOperator { + checked_relative_division: false, kind: BinaryOpKind::Arithmetic(ArithmeticOpKind::Sub), vector_match: None, }; diff --git a/data_plane/src/precompute_engine/subdag_scheduler.rs b/data_plane/src/precompute_engine/subdag_scheduler.rs index 69d67c40..e7ac372c 100644 --- a/data_plane/src/precompute_engine/subdag_scheduler.rs +++ b/data_plane/src/precompute_engine/subdag_scheduler.rs @@ -328,6 +328,7 @@ mod tests { binary.payload = ExecutableOperatorPayload::Binary { timing: planner_types::post_asap::ExecutionTiming::MaintenanceTime, operator: BinaryOperator { + checked_relative_division: false, kind: BinaryOpKind::Arithmetic(ArithmeticOpKind::Sub), vector_match: None, }, diff --git a/data_plane/src/precompute_engine/window_manager.rs b/data_plane/src/precompute_engine/window_manager.rs index ed0b84b9..7656a872 100644 --- a/data_plane/src/precompute_engine/window_manager.rs +++ b/data_plane/src/precompute_engine/window_manager.rs @@ -64,6 +64,9 @@ impl WindowManager { pub fn stored_bucket_starts(&self, timestamp_ms: i64) -> Vec { if self.stores_full_windows { self.window_starts_containing(timestamp_ms) + .into_iter() + .filter(|start| *start >= 0) + .collect() } else { vec![self.pane_start_for(timestamp_ms)] } @@ -193,6 +196,18 @@ impl WindowManager { mod tests { use super::*; + // Storage timestamps are unsigned: never admit windows the sink cannot publish. + #[test] + fn full_window_storage_starts_stay_in_the_storage_time_domain() { + let manager = WindowManager::with_layout( + 5, + 1, + Some(0), + &asap_types::WindowMaterializationLayout::FullWindow, + ); + assert_eq!(manager.stored_bucket_starts(999), vec![0]); + } + #[test] fn stored_bucket_assignment_distinguishes_full_windows_from_base_panes() { // Admission and execution need the stored extent, not only slide cadence. diff --git a/data_plane/src/query_engines/asap_query_engine/catalog_resolver.rs b/data_plane/src/query_engines/asap_query_engine/catalog_resolver.rs index 3009c68c..bc7d72df 100644 --- a/data_plane/src/query_engines/asap_query_engine/catalog_resolver.rs +++ b/data_plane/src/query_engines/asap_query_engine/catalog_resolver.rs @@ -80,6 +80,10 @@ impl ResolvedMaterialization<'_> { ExactReadout::Increase | ExactReadout::Rate => { matches!(aggregation_type, Increase | MultipleIncrease) } + ExactReadout::Min => { + matches!(aggregation_type, MinMax | MultipleMinMax) + && aggregation_sub_type.eq_ignore_ascii_case("min") + } ExactReadout::Max => { matches!(aggregation_type, MinMax | MultipleMinMax) && aggregation_sub_type.eq_ignore_ascii_case("max") 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 7d9b6b1f..ed6c24dd 100644 --- a/data_plane/src/query_engines/asap_query_engine/engine.rs +++ b/data_plane/src/query_engines/asap_query_engine/engine.rs @@ -2073,15 +2073,9 @@ mod asap_tier_classify_tests { env.encode_to_vec() } - /// REGRESSION of the HLL `count(metric)` "No result" e2e failure - /// (`controller_plan_to_query_full_roundtrip_hll`) isolated to the - /// engine layer. `count(unique_users_per_min)` is the distinct-count - /// idiom. The Planner DAG represents this as a cardinality readout, - /// so the executor returns the HLL distinct-count directly. A single - /// FULL HLL frame (~500 users) is used so - /// the instant projection reads the real estimate. + /// Temporal distinct reads HLL cardinality; PromQL count counts vector rows. #[tokio::test] - async fn execute_count_hll_returns_cardinality_not_rowcount() { + async fn execute_temporal_distinct_hll_returns_cardinality() { let idx = Arc::new(SketchStore::new()); let sid = 7500u64; idx.register(hll_meta(sid, "unique_users_per_min")); @@ -2101,14 +2095,17 @@ mod asap_tier_classify_tests { ); let engine = build_engine_with_index(idx); - let result = engine.execute("count(unique_users_per_min)").await.expect( - "count(hll_metric) must dispatch to the Cardinality family \ + let result = engine + .execute("distinct_over_time(unique_users_per_min[1m])") + .await + .expect( + "distinct_over_time must dispatch to the Cardinality family \ via the candidate capability (empty trace function) and \ return the HLL distinct-count, NOT capability-miss", - ); + ); assert!( result_nonempty(&result), - "count(unique_users_per_min) over an HLL sid must return a \ + "distinct_over_time(unique_users_per_min[1m]) over an HLL sid must return a \ non-empty cardinality estimate (regression: empty `asap_query` \ No-result)" ); @@ -2156,15 +2153,9 @@ mod asap_tier_classify_tests { .encode_to_vec() } - /// FIX 2 — GLOBAL HLL distinct rollup. `count(hll_metric)` with no `by` - /// must MERGE the per-series HLL registers (register-wise max) across ALL - /// matched series and estimate ONCE — the distinct UNION cardinality. Two - /// series share an overlapping prefix of items and each carry disjoint - /// items, so summing per-series estimates would over-count the overlap. - /// The merged global estimate must land within HLL error of the true - /// union, and be strictly below the naive per-series sum. + /// Per-series temporal distinct must not turn into global distinct-series count. #[tokio::test] - async fn execute_count_hll_global_merges_registers_across_series() { + async fn execute_temporal_distinct_preserves_series_populations() { let idx = Arc::new(SketchStore::new()); let now_ms = std::time::SystemTime::now() .duration_since(std::time::SystemTime::UNIX_EPOCH) @@ -2178,7 +2169,6 @@ mod asap_tier_classify_tests { let precision = 14u32; // ~0.8% standard error; legal for epsilon=0.01 let a_items: Vec = (0..600).map(|i| format!("u-{i}")).collect(); let b_items: Vec = (400..1000).map(|i| format!("u-{i}")).collect(); - let true_union = 1000.0_f64; for (sid, items) in [(8200u64, &a_items), (8201u64, &b_items)] { let mut meta = hll_meta(sid, "unique_users_global"); @@ -2187,10 +2177,11 @@ mod asap_tier_classify_tests { config: SketchConfig::Hll { precision }, spatial_filter_canonical: String::new(), }; + meta.group_by_keys.insert("instance".into()); idx.register(meta); idx.append_sample( sid, - BTreeMap::new(), + BTreeMap::from([("instance".into(), sid.to_string())]), (w_start, w_end), SketchSampleState { bytes: encode_hll_from_items(precision, items), @@ -2201,44 +2192,23 @@ mod asap_tier_classify_tests { let engine = build_engine_with_index(idx); let result = engine - .execute("count(unique_users_global)") + .execute("distinct_over_time(unique_users_global[1m])") .await - .expect("global count(hll_metric) must answer, not capability-miss"); - - // GLOBAL distinct is a single scalar — exactly one element. - let est = match &result { + .unwrap(); + let estimates: Vec<_> = match result { crate::query_engines::query_result::QueryResult::Vector(v) => { - assert_eq!( - v.values.len(), - 1, - "global count() must collapse to ONE merged estimate, got {} \ - (per-series leak): {v:?}", - v.values.len() - ); - v.values[0].value - } - crate::query_engines::query_result::QueryResult::Matrix(m) => { - assert_eq!(m.values.len(), 1, "one merged series"); - m.values[0].samples.last().map(|s| s.value).unwrap_or(0.0) + v.values.iter().map(|v| v.value).collect() } + crate::query_engines::query_result::QueryResult::Matrix(m) => m + .values + .iter() + .map(|v| v.samples.last().unwrap().value) + .collect(), }; - - // Within HLL error of the true union (p=12 → ~1.04/sqrt(2^12) ≈ 1.6%; - // allow a generous 8% band for the estimator's finite-sample noise). - let rel_err = (est - true_union).abs() / true_union; - assert!( - rel_err < 0.08, - "global merged estimate {est} must be within HLL error of the \ - true union {true_union} (rel_err {rel_err:.4})" - ); - - // And strictly below the naive per-series sum (600 + 600 = 1200), - // proving registers were MERGED (max), not the estimates SUMMED. - assert!( - est < 1150.0, - "merged global estimate {est} must be well below the per-series \ - sum (~1200) — proves register-merge, not estimate-sum" - ); + assert_eq!(estimates.len(), 2); + for estimate in estimates { + assert!((estimate - 600.0).abs() / 600.0 < 0.08); + } } /// REPRODUCTION (root-cause hunt): `quantile_over_time(0.99, 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 2d246efd..00f6c759 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 @@ -494,32 +494,13 @@ mod tests { } #[test] - fn flag_on_global_merge_shape_is_served_merged_not_declined() { - // Previously `flag_on_ambiguous_shape_falls_back`, asserting - // `result.is_none()`: the grouping-ambiguity gate declined this - // shape because an empty `by` couldn't be told apart from "reduce - // everything" (ASAPController#163). With `Reduction` (#165) the - // executor resolves it -- `count(...)` lowers to `Reduce([])`, both - // sids share one group key, and the new path serves the correctly - // merged answer instead of falling back. + fn count_does_not_use_hll_distinct_cardinality() { let idx = SketchStore::new(); register_hll(&idx, 1, "svc-a", &["a", "b", "c"]); register_hll(&idx, 2, "svc-b", &["d", "e", "f"]); - let result = - try_serve_from_summary_executor(&idx, "count(unique_users)", 1_000, 2_000, true); - let result = result - .expect("global-merge shape is no longer ambiguous -- it must be served, not declined"); - assert_eq!( - result.series.len(), - 1, - "a by-less count() must merge both sids into ONE series, got {:?}", - result.series - ); - // Disjoint item sets {a,b,c} + {d,e,f} -> merged cardinality ~6. - let card = result.series[0].1[0].1; assert!( - (4.0..=8.0).contains(&card), - "merged cardinality {card} should be ~6 (both sids), not ~3 (one sid)" + try_serve_from_summary_executor(&idx, "count(unique_users)", 1_000, 2_000, true) + .is_none() ); } diff --git a/data_plane/src/query_engines/asap_query_engine/logical_dag.rs b/data_plane/src/query_engines/asap_query_engine/logical_dag.rs index b4e2436f..c2c9aaab 100644 --- a/data_plane/src/query_engines/asap_query_engine/logical_dag.rs +++ b/data_plane/src/query_engines/asap_query_engine/logical_dag.rs @@ -552,6 +552,31 @@ fn binary( left: Value, right: Value, ) -> Result { + if operation == BinaryOperation::CheckedDiv { + let valid = |value: &Value, denominator: bool| match value { + Value::Scalar(v) => v.is_finite() && (!denominator || *v != 0.0), + Value::Vector(rows) => rows + .iter() + .all(|(_, v)| v.is_finite() && (!denominator || *v != 0.0)), + Value::Matrix(..) => false, + }; + if boolean || !valid(&left, false) || !valid(&right, true) { + return Err(miss( + "relative division domain requires finite operands and a nonzero divisor", + )); + } + let result = binary(BinaryOperation::Div, false, left, right)?; + let normal = match &result { + Value::Scalar(v) => v.is_normal(), + Value::Vector(rows) => rows.iter().all(|(_, v)| v.is_normal()), + Value::Matrix(..) => false, + }; + return if normal { + Ok(result) + } else { + Err(miss("relative division result requires exact evaluation outside normal floating-point range")) + }; + } let arithmetic = matches!( operation, BinaryOperation::Add @@ -750,6 +775,37 @@ mod topk_tests { .collect() } + // A conditional accuracy certificate must fall back rather than return an unbounded ratio. + #[test] + fn checked_relative_division_enforces_its_execution_domain() { + for (a, b) in [ + (1., 0.), + (0., 0.), + (1., f64::INFINITY), + (f64::NAN, 2.), + (f64::MAX, f64::MIN_POSITIVE), + (f64::MIN_POSITIVE, f64::MAX), + ] { + assert!(binary( + BinaryOperation::CheckedDiv, + false, + Value::Scalar(a), + Value::Scalar(b) + ) + .is_err()); + } + let Value::Scalar(value) = binary( + BinaryOperation::CheckedDiv, + false, + Value::Scalar(5.), + Value::Scalar(10.), + ) + .unwrap() else { + panic!("scalar"); + }; + assert_eq!(value, 0.5); + } + #[test] fn topk_selects_by_sample_value_and_preserves_series_labels() { let values = vec![ diff --git a/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs b/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs index a0fba320..d08109f0 100644 --- a/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs +++ b/data_plane/src/query_engines/asap_query_engine/post_asap_readout.rs @@ -288,8 +288,11 @@ impl QueryNodeRuntime for PhysicalQueryRuntime<'_> { let mut labels = key.clone(); if self.language == control_plane::query_plan::QueryLanguage::MetricsQl - && *readout - != control_plane::query_plan::ExactReadout::Max + && !matches!( + readout, + control_plane::query_plan::ExactReadout::Max + | control_plane::query_plan::ExactReadout::Min + ) { labels.remove("__name__"); } @@ -1089,40 +1092,19 @@ mod tests { } #[test] - fn global_merge_shape_now_merges_instead_of_being_declined() { - // The exact ASAPController#163 shape: two HLL sids, no explicit - // by(), an aggregation-operator query. This test previously - // asserted `ambiguous_merge_risk == true` and TWO unmerged series - // -- i.e. it pinned the old workaround, where an empty `by` left - // `find_candidates` unable to tell "reduce everything" apart from - // "no grouping concept," so `live_serve` declined to serve the - // shape at all. - // - // With `Reduction` (ASAPController#165) that ambiguity is gone: - // `count(...)` is a genuine aggregation operator, so it lowers to - // `Reduce([])` and `resolve_group_key` gives every candidate the - // SAME group key -- the two sids MERGE into one answer, which is - // what the query actually asked for. No gate, no fallback. + fn count_cannot_be_answered_by_merging_hll_registers() { 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_post_asap_readout(&idx, "count(unique_users)", 1_000, 2_000, true, accuracy()) - .expect("should execute"); - assert_eq!( - outcome.series.len(), - 1, - "a by-less count() is a full reduction -- both HLL sids must merge into ONE \ - series, not stay split (and not be declined), got {:?}", - outcome.series - ); - // Disjoint item sets {a,b,c} + {d,e,f} -> merged cardinality ~6. - let (_group, points) = &outcome.series[0]; - let card = points[0].1; - assert!( - (4.0..=8.0).contains(&card), - "merged cardinality {card} should be ~6 (both sids' disjoint items), not ~3" - ); + assert!(execute_post_asap_readout( + &idx, + "count(unique_users)", + 1_000, + 2_000, + true, + accuracy() + ) + .is_err()); } #[test] diff --git a/data_plane/src/query_engines/asap_query_engine/summary_exec.rs b/data_plane/src/query_engines/asap_query_engine/summary_exec.rs index 672363d1..0dec5060 100644 --- a/data_plane/src/query_engines/asap_query_engine/summary_exec.rs +++ b/data_plane/src/query_engines/asap_query_engine/summary_exec.rs @@ -548,6 +548,7 @@ mod tests { lhs: child.clone(), rhs: child.clone(), operator: planner_types::post_asap::BinaryOperator { + checked_relative_division: false, kind: planner_types::pre_asap::BinaryOpKind::Arithmetic( planner_types::pre_asap::ArithmeticOpKind::Div, ), 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 6b0f22d6..087c4550 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 @@ -256,6 +256,10 @@ impl GroupState { asap_types::query_plan::ExactReadout::Max, AggregationType::MinMax | AggregationType::MultipleMinMax, ) => asap_types::Statistic::Max, + ( + asap_types::query_plan::ExactReadout::Min, + AggregationType::MinMax | AggregationType::MultipleMinMax, + ) => asap_types::Statistic::Min, _ => return None, }; @@ -284,8 +288,10 @@ impl GroupState { if matches!( agg_type, AggregationType::MinMax | AggregationType::MultipleMinMax - ) && readout == asap_types::query_plan::ExactReadout::Max - { + ) && matches!( + readout, + asap_types::query_plan::ExactReadout::Max | asap_types::query_plan::ExactReadout::Min + ) { return entries .iter() .flat_map(|windows| windows.values()) @@ -296,7 +302,11 @@ impl GroupState { }) .collect::>>()? .into_iter() - .reduce(f64::max); + .reduce(if readout == asap_types::query_plan::ExactReadout::Min { + f64::min + } else { + f64::max + }); } let mut merged: Option> = None; for windows in entries { @@ -448,10 +458,11 @@ impl QueryExecutionContext<'_> { SummaryExecutorError::Unsupported("query end exceeds signed event time") })?, }; - if self - .index - .has_pending_summary_updates(binding.materialization, query_range) - { + if self.index.has_pending_summary_updates( + binding.materialization, + query_range, + binding.full_window_slide_ms.is_some(), + ) { return Err(SummaryExecutorError::Unsupported( "materialization population has unpublished input", )); @@ -551,6 +562,13 @@ impl QueryExecutionContext<'_> { if series.samples.is_empty() { return Err(SummaryExecutorError::NoCandidates); } + } else { + // The legacy index includes a preceding frame for delta + // decoding. Certified panes reset their base per window; + // that preceding population is not part of this query. + series + .samples + .retain(|end, _| *end > self.t0_ms as i64 && *end <= self.t1_ms as i64); } check_panes(series.samples.keys().copied().collect())?; let key = match &binding.output_grouping { @@ -577,10 +595,40 @@ impl QueryExecutionContext<'_> { let coverage = self .index .exact_agg_coverage_bounds(sid, self.t0_ms, self.t1_ms); + if coverage.is_none() + && full_window + && self.index.full_summary_window_known_empty( + binding.materialization, + sid, + query_range, + ) + { + continue; + } if coverage != Some((self.t0_ms, self.t1_ms)) { - return Err(SummaryExecutorError::Unsupported( - "counter SDS requires full-pane query coverage", - )); + validate_binding_phase(binding, self.t0_ms)?; + let empty = |start: u64, end: u64| { + start == end + || self.index.summary_window_known_empty( + binding.materialization, + sid, + asap_types::sds::HalfOpenTimeRange { + start_ms: start as i64, + end_ms: end as i64, + }, + ) + }; + let edges_are_empty = coverage.is_some_and(|(start, end)| { + start >= self.t0_ms + && end <= self.t1_ms + && empty(self.t0_ms, start) + && empty(end, self.t1_ms) + }); + if !edges_are_empty { + return Err(SummaryExecutorError::Unsupported( + "counter SDS requires full-pane query coverage", + )); + } } } if matches!( @@ -1876,7 +1924,11 @@ mod tests { let windows = if full_window { vec![(0, vec![99.0, 99.0]), (1000, vec![1.0, 2.0, 2.0, 3.0])] } else { - vec![(0, vec![1.0, 2.0]), (1000, vec![2.0, 3.0])] + vec![ + (0, vec![99.0, 99.0]), + (1000, vec![1.0, 2.0]), + (2000, vec![2.0, 3.0]), + ] }; for (start, values) in windows { let mut state = UnivMonAccumulator::new(32, 5, 1024, 4).unwrap(); @@ -1895,8 +1947,8 @@ mod tests { } let context = QueryExecutionContext { index: &index, - t0_ms: if full_window { 1000 } else { 0 }, - t1_ms: if full_window { 3000 } else { 2000 }, + t0_ms: 1000, + t1_ms: 3000, is_cumulative: true, allowed_materializations: Some(BTreeSet::from([fp])), }; diff --git a/data_plane/src/storage_engines/sketch_db/current_series.rs b/data_plane/src/storage_engines/sketch_db/current_series.rs index 545cc29b..79c1a37f 100644 --- a/data_plane/src/storage_engines/sketch_db/current_series.rs +++ b/data_plane/src/storage_engines/sketch_db/current_series.rs @@ -41,7 +41,7 @@ struct Member { #[derive(Default)] struct Group { ordered: BTreeSet, - cached: Option<(Vec, Vector)>, + cached: Option<(Vec, Vector, f64, f64)>, } struct Population { definition: SeriesPopulation, @@ -196,10 +196,17 @@ impl Population { .take(self.definition.max_k as usize) .map(|r| (r.labels.clone(), r.value)) .collect(); - group.cached = Some((values, top)); + let sum = compensated_sum(group.ordered.iter().map(|r| r.value)); + let count = group.ordered.len() as f64; + let average = if sum.is_finite() { + sum / count + } else { + compensated_sum(group.ordered.iter().map(|r| r.value / count)) + }; + group.cached = Some((values, top, sum, average)); self.cache_builds += 1; } - let (values, top) = group.cached.as_ref().unwrap(); + let (values, top, sum, average) = group.cached.as_ref().unwrap(); match readout { SeriesReadout::Quantile { q } => { let value = if *q < 0. { @@ -216,12 +223,34 @@ impl Population { result.push((labels.clone(), value)); } SeriesReadout::TopK { k } => result.extend(top.iter().take(*k as usize).cloned()), + SeriesReadout::Sum => result.push((labels.clone(), *sum)), + SeriesReadout::Count => result.push((labels.clone(), group.ordered.len() as f64)), + SeriesReadout::Average => result.push((labels.clone(), *average)), } } result } } +// Rebuild shared statistics after replacement/expiry, avoiding subtraction drift. +fn compensated_sum(values: impl Iterator) -> f64 { + let (mut sum, mut correction) = (0.0_f64, 0.0); + for value in values { + let next = sum + value; + if next.is_finite() { + correction += if sum.abs() >= value.abs() { + (sum - next) + value + } else { + (value - next) + sum + }; + } else { + correction = 0.0; + } + sum = next; + } + sum + correction +} + #[derive(Default)] pub struct CurrentSeriesStore { generation: Option<(u64, u64)>, @@ -422,6 +451,43 @@ mod tests { ); } } + // Equal sample values still represent two series; replacements and stale markers retract them. + #[test] + fn sum_count_average_follow_current_series_membership() { + let p = definition(); + let plan = plan(&p); + let mut store = CurrentSeriesStore::default(); + warm(&mut store, &plan); + for (at, samples, expected) in [ + ( + 301_000, + vec![sample("y", "api", 301_000, Some(1.))], + [7., 3., 7. / 3.], + ), + ( + 302_000, + vec![sample("z", "api", 302_000, None)], + [2., 2., 1.], + ), + ] { + store.ingest(&plan, &samples); + for (readout, truth) in [ + SeriesReadout::Sum, + SeriesReadout::Count, + SeriesReadout::Average, + ] + .into_iter() + .zip(expected) + { + let values = store.read((7, 1), &p, &readout, at).unwrap(); + assert!( + (values[0].1 - truth).abs() < 1e-12, + "{readout:?}: {values:?}" + ); + } + } + } + /// Four quantiles reuse one distribution, and smaller k reads the shared maximum-k prefix. #[test] fn quantiles_and_topk_share_state_and_promote_after_updates_and_staleness() { diff --git a/data_plane/src/storage_engines/sketch_db/index/admission.rs b/data_plane/src/storage_engines/sketch_db/index/admission.rs index 344fd95c..dc30248e 100644 --- a/data_plane/src/storage_engines/sketch_db/index/admission.rs +++ b/data_plane/src/storage_engines/sketch_db/index/admission.rs @@ -255,6 +255,16 @@ impl AdmissionInventory { definition: SummaryDefinitionId, series_id: u64, range: HalfOpenTimeRange, + ) -> bool { + self.known_empty_with_layout(definition, series_id, range, false) + } + + pub(super) fn known_empty_with_layout( + &self, + definition: SummaryDefinitionId, + series_id: u64, + range: HalfOpenTimeRange, + full_window: bool, ) -> bool { self.finite_input == FiniteInputState::Complete && self.published_series.contains_key(&series_id) @@ -267,8 +277,12 @@ impl AdmissionInventory { .is_none_or(|floor| range.start_ms >= *floor) && !self.windows.iter().any(|(coordinate, state)| { coordinate.summary_definition_id == definition - && coordinate.time_range.start_ms < range.end_ms - && coordinate.time_range.end_ms > range.start_ms + && (if full_window { + coordinate.time_range == range + } else { + coordinate.time_range.start_ms < range.end_ms + && coordinate.time_range.end_ms > range.start_ms + }) && state.series_id == Some(series_id) }) } @@ -285,11 +299,16 @@ impl AdmissionInventory { &self, definition: SummaryDefinitionId, range: HalfOpenTimeRange, + full_window: bool, ) -> bool { self.windows.iter().any(|(coordinate, state)| { coordinate.summary_definition_id == definition - && coordinate.time_range.start_ms < range.end_ms - && coordinate.time_range.end_ms > range.start_ms + && (if full_window { + coordinate.time_range == range + } else { + coordinate.time_range.start_ms < range.end_ms + && coordinate.time_range.end_ms > range.start_ms + }) && state.published < state.admitted }) } @@ -366,9 +385,9 @@ mod tests { .admit(&generation, BTreeSet::from([a.clone(), b.clone()])) .unwrap(); inventory.acknowledge(&generation, &a, revision).unwrap(); - assert!(inventory.has_pending(a.summary_definition_id, a.time_range)); + assert!(inventory.has_pending(a.summary_definition_id, a.time_range, false)); inventory.acknowledge(&generation, &b, revision).unwrap(); - assert!(!inventory.has_pending(a.summary_definition_id, a.time_range)); + assert!(!inventory.has_pending(a.summary_definition_id, a.time_range, false)); } #[test] @@ -388,7 +407,11 @@ mod tests { .acknowledge(&generation, &coordinate, first) .unwrap(); assert_ne!(before, inventory.revision()); - assert!(inventory.has_pending(coordinate.summary_definition_id, coordinate.time_range)); + assert!(inventory.has_pending( + coordinate.summary_definition_id, + coordinate.time_range, + false + )); inventory.retire_completed_before(coordinate.summary_definition_id, 1000); assert_eq!(inventory.windows.len(), 1); inventory @@ -447,6 +470,89 @@ mod tests { assert!(inventory.admit(&old, BTreeSet::from([coordinate])).is_err()); } + // Unpublished future snapshots cannot block an already-published full window. + #[test] + fn pending_full_window_checks_only_the_requested_snapshot() { + let generation = generation(1); + let mut inventory = AdmissionInventory::default(); + inventory.install(generation.clone()); + let current = window("a"); + let mut future = current.clone(); + future.time_range = HalfOpenTimeRange { + start_ms: 500, + end_ms: 1500, + }; + let revision = inventory + .admit(&generation, BTreeSet::from([current.clone(), future])) + .unwrap(); + assert!(inventory.has_pending(current.summary_definition_id, current.time_range, true)); + inventory + .acknowledge(&generation, ¤t, revision) + .unwrap(); + assert!(inventory.has_pending(current.summary_definition_id, current.time_range, false)); + assert!(!inventory.has_pending(current.summary_definition_id, current.time_range, true)); + } + + // A neighboring full snapshot may overlap an empty query population. + #[test] + fn full_window_empty_proof_uses_exact_window_identity() { + let generation = generation(1); + let mut inventory = AdmissionInventory::default(); + inventory.install(generation.clone()); + let first = window("a"); + let mut neighbor = first.clone(); + neighbor.time_range = HalfOpenTimeRange { + start_ms: 500, + end_ms: 1500, + }; + let mut other = window("b"); + other.time_range = HalfOpenTimeRange { + start_ms: 1000, + end_ms: 2000, + }; + let revision = inventory + .admit( + &generation, + BTreeSet::from([first.clone(), neighbor.clone(), other.clone()]), + ) + .unwrap(); + for (coordinate, sid) in [(&first, 1), (&neighbor, 1), (&other, 2)] { + inventory + .record_series(&generation, coordinate, sid) + .unwrap(); + inventory + .acknowledge(&generation, coordinate, revision) + .unwrap(); + } + assert!(!inventory.known_empty_with_layout( + first.summary_definition_id, + 1, + other.time_range, + true + )); + inventory.seal_finite(&generation).unwrap(); + assert!(!inventory.known_empty(first.summary_definition_id, 1, other.time_range)); + assert!(inventory.known_empty_with_layout( + first.summary_definition_id, + 1, + other.time_range, + true + )); + assert!(!inventory.known_empty_with_layout( + first.summary_definition_id, + 2, + other.time_range, + true + )); + inventory.retire_completed_before(first.summary_definition_id, 2000); + assert!(!inventory.known_empty_with_layout( + first.summary_definition_id, + 1, + other.time_range, + true + )); + } + #[test] fn only_finite_completion_proves_an_inactive_series_window_empty() { let generation = generation(1); diff --git a/data_plane/src/storage_engines/sketch_db/index/mod.rs b/data_plane/src/storage_engines/sketch_db/index/mod.rs index 228eaf09..9e13463f 100644 --- a/data_plane/src/storage_engines/sketch_db/index/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/index/mod.rs @@ -1072,6 +1072,24 @@ impl SketchStore { .known_empty(definition, series_id, range) } + /// A closed full-window producer publishes every nonempty exact window. + /// Overlapping neighboring snapshots do not establish population in this one. + pub(crate) fn full_summary_window_known_empty( + &self, + definition: SummaryDefinitionId, + series_id: u64, + range: HalfOpenTimeRange, + ) -> bool { + use std::sync::atomic::Ordering::SeqCst; + self.active_mutations.load(SeqCst) == 0 + && self.finite_mutation_revision.load(SeqCst) == self.mutation_revision.load(SeqCst) + && self + .admission + .read() + .unwrap() + .known_empty_with_layout(definition, series_id, range, true) + } + pub(crate) fn summary_update_revision(&self) -> SummaryReadRevision { SummaryReadRevision::capture( self.admission.read().unwrap().revision(), @@ -1091,11 +1109,12 @@ impl SketchStore { &self, definition: SummaryDefinitionId, range: HalfOpenTimeRange, + full_window: bool, ) -> bool { self.admission .read() .unwrap() - .has_pending(definition, range) + .has_pending(definition, range, full_window) } /// Share the installed metadata snapshot without copying descriptors or state. diff --git a/data_plane/tests/asapquery_compatibility_process_e2e.rs b/data_plane/tests/asapquery_compatibility_process_e2e.rs index 505ebcf9..a11edb3b 100644 --- a/data_plane/tests/asapquery_compatibility_process_e2e.rs +++ b/data_plane/tests/asapquery_compatibility_process_e2e.rs @@ -31,6 +31,9 @@ mod immutable_maintenance_process; #[path = "support/current_series_process.rs"] mod current_series_process; +#[path = "support/issue_701_702_process.rs"] +mod issue_701_702_process; + // Test-only quotes preserve the fixture's local candidate without a production bypass. fn quote_snapshot_for_test( snapshot: control_plane::physical::compiler::BackendLocalPlanningSnapshot, @@ -1201,10 +1204,13 @@ async fn collector_free_profile_serves_complete_matrix_and_falls_back_exactly() let backend_port = unused_port(); let output_dir = tempfile::tempdir().expect("backend output directory"); - let fixture = serde_json::from_str(include_str!( - "../../docs/examples/asapquery-compatibility-demo-snapshot.json" - )) - .unwrap(); + let mut fixture: control_plane::physical::compiler::BackendLocalPlanningSnapshot = + serde_json::from_str(include_str!( + "../../docs/examples/asapquery-compatibility-demo-snapshot.json" + )) + .unwrap(); + // These range checks read older evaluations after finite drain. + fixture.implementation.query_staleness_margin_ms = 60_000; let priced = quote_snapshot_for_test(fixture); let snapshot = output_dir.path().join("snapshot.json"); std::fs::write(&snapshot, serde_json::to_vec(&priced).unwrap()).unwrap(); diff --git a/data_plane/tests/support/current_series_process.rs b/data_plane/tests/support/current_series_process.rs index 56f1377f..627be60e 100644 --- a/data_plane/tests/support/current_series_process.rs +++ b/data_plane/tests/support/current_series_process.rs @@ -39,6 +39,10 @@ async fn current_series_quantiles_topk_share_and_replace_values() { queries.push(format!("topk({k}, a)")); queries.push(format!("topk by (job) ({k}, a)")); } + for operation in ["sum", "count", "avg"] { + queries.push(format!("{operation}(a)")); + queries.push(format!("{operation} by (job) (a)")); + } snapshot.query_workload.repeating_queries = Some( queries .iter() @@ -90,11 +94,21 @@ async fn current_series_quantiles_topk_share_and_replace_values() { quotes, }); let planned = snapshot.clone().compile().unwrap(); - assert!(planned - .query_plan - .entries - .values() - .all(|e| serde_json::to_string(e).unwrap().contains("current_series"))); + assert!( + planned + .query_plan + .entries + .values() + .all(|e| serde_json::to_string(e).unwrap().contains("current_series")), + "non-current entries: {:?}", + planned + .query_plan + .entries + .values() + .filter(|e| !serde_json::to_string(e).unwrap().contains("current_series")) + .map(|e| (&e.canonical_query, &e.nodes)) + .collect::>() + ); let output = tempfile::tempdir().unwrap(); let path = output.path().join("snapshot.json"); std::fs::write(&path, serde_json::to_vec(&snapshot).unwrap()).unwrap(); @@ -239,6 +253,16 @@ async fn current_series_quantiles_topk_share_and_replace_values() { "{body}" ); } + for operation in ["sum", "count", "avg"] { + for text in [ + format!("{operation}(a)"), + format!("{operation} by (job) (a)"), + ] { + let body = query(&client, &base, &text, end).await; + assert!(is_warm(&body), "{text}: {body}"); + compare_native(&client, &native, &text, end, &body).await; + } + } let metrics = client .get(format!("{base}/metrics")) .send() @@ -291,6 +315,16 @@ async fn current_series_quantiles_topk_share_and_replace_values() { &body, ) .await; + for operation in ["sum", "count", "avg"] { + for text in [ + format!("{operation}(a)"), + format!("{operation} by (job) (a)"), + ] { + let body = query(&client, &base, &text, end + offset).await; + assert!(is_warm(&body), "{text}: {body}"); + compare_native(&client, &native, &text, end + offset, &body).await; + } + } let body = query(&client, &base, "quantile by (job) (0.5, a)", end + offset).await; assert!(is_warm(&body), "{body}"); compare_native( diff --git a/data_plane/tests/support/issue_701_702_process.rs b/data_plane/tests/support/issue_701_702_process.rs new file mode 100644 index 00000000..fd66ff89 --- /dev/null +++ b/data_plane/tests/support/issue_701_702_process.rs @@ -0,0 +1,343 @@ +//! Issue workloads execute their selected Planner DAG on the production HTTP path. +use super::*; +use control_plane::physical::{ + compiler::{ + BackendLocalPlanningSnapshot, PhysicalCompiler, BACKEND_REVISION, PLANNER_REVISION, + }, + workload_cost::{self, WorkloadCostEvidence, WorkloadQuote}, +}; + +// This fixture backfills fifteen minutes across many maintained populations. +// Its readiness budget covers ingestion, not a query latency benchmark. +async fn wait_for_issue_warm_instant( + client: &reqwest::Client, + base: &str, + query: &str, + at: f64, + log_path: &std::path::Path, +) -> Value { + let deadline = tokio::time::Instant::now() + Duration::from_secs(90); + loop { + let result: Value = client + .get(format!("{base}/api/v1/query")) + .query(&[("query", query.to_string()), ("time", at.to_string())]) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + if is_warm(&result) && first_value(&result, "value").is_some() { + return result; + } + assert!( + tokio::time::Instant::now() < deadline, + "{query} did not become warm: {result}; log: {}", + std::fs::read_to_string(log_path).unwrap_or_default() + ); + tokio::time::sleep(Duration::from_millis(100)).await; + } +} + +fn queries() -> Vec<(String, u64, u64)> { + let mut queries = vec![]; + for q in [0.5, 0.55, 0.6, 0.65, 0.7, 0.75, 0.9, 0.95, 0.99, 0.999] { + queries.push(( + format!("quantile_over_time({q}, issue701_data[15m])"), + 900, + 60, + )); + queries.push(( + format!("quantile_over_time({q}, issue701_data[5m])"), + 300, + if q == 0.9 { 30 } else { 10 }, + )); + queries.push((format!("quantile by(job)({q}, issue701_data)"), 1, 1)); + } + for operation in ["sum", "count", "avg", "min", "max"] { + queries.push((format!("{operation}_over_time(issue701_data[5m])"), 300, 30)); + } + for operation in ["sum", "count", "avg"] { + queries.push((format!("{operation}(issue701_data)"), 1, 1)); + queries.push((format!("{operation} by(job)(issue701_data)"), 1, 1)); + } + for operation in ["sum", "count"] { + queries.push(( + format!("topk(5, {operation}_over_time(issue701_data[5m]))"), + 300, + 30, + )); + } + queries.push(( + "sum by(job)(sum_over_time(issue701_data[5m]))".into(), + 300, + 30, + )); + queries.push(( + "quantile_over_time(0.9, issue701_data[5m]) / quantile_over_time(0.5, issue701_data[5m])" + .into(), + 300, + 60, + )); + queries.push(( + "avg_over_time(issue701_data[5m]) / quantile_over_time(0.5, issue701_data[5m])".into(), + 300, + 30, + )); + queries +} + +// A single mixed workload covers moving windows, current series, minimum/average, +// and ratio accuracy. Optional native URL adds a real Prometheus differential oracle. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn issue_workloads_execute_warm_at_successive_evaluations() { + let native = std::env::var("ASAP_CURRENT_SERIES_PROMETHEUS_URL").ok(); + if let Some(url) = &native { + let info: Value = reqwest::get(format!("{url}/api/v1/status/buildinfo")) + .await + .unwrap() + .json() + .await + .unwrap(); + let version = info["data"]["version"] + .as_str() + .expect("Prometheus version"); + assert!( + version.starts_with("3."), + "boundary-aligned oracle requires Prometheus 3.x left-open ranges, found {version}" + ); + } + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let mock_url = format!("http://{}", listener.local_addr().unwrap()); + let _mock = tokio::spawn(async move { + axum::serve( + listener, + Router::new().route("/-/healthy", get(|| async { "healthy" })), + ) + .await + .unwrap(); + }); + let queries = queries(); + let mut fixture: Value = serde_json::from_str(include_str!( + "../../../docs/examples/asapquery-planning-snapshot.json" + )) + .unwrap(); + fixture["implementation"]["source_sample_interval_ms"] = 1000.into(); + fixture["implementation"]["horizon_seconds"] = 3600.into(); + fixture["implementation"]["implementation_cost"]["horizon_seconds"] = 3600.into(); + let template = fixture["query_workload"]["repeating_queries"][0].clone(); + fixture["query_workload"]["repeating_queries"] = queries + .iter() + .map(|(query, lookback, cadence)| { + let mut entry = template.clone(); + entry["query"] = query.clone().into(); + entry["time_selection"]["lookback"] = (lookback * 1000).into(); + entry["demand"]["fixed_interval_at"]["interval"] = (cadence * 1000).into(); + if !query.contains("quantile") { + entry["requirements"]["accuracy"] = serde_json::json!({"explicit":"Exact"}); + } + entry + }) + .collect(); + let mut snapshot: BackendLocalPlanningSnapshot = serde_json::from_value(fixture).unwrap(); + let (request, environment) = snapshot.clone().planning_request().unwrap(); + let candidates = workload_cost::with_exact_alternative(request).unwrap(); + let fully_warm = |plan: &control_plane::physical::compiler::PhysicalPlan| { + plan.query_plan.entries.values().all(|entry| entry.nodes.values().all(|node| !matches!(node, + control_plane::query_plan::QueryPlanNode::ExactFallback { .. } + | control_plane::query_plan::QueryPlanNode::ExternalExact { .. } + | control_plane::query_plan::QueryPlanNode::Logical { + operator: control_plane::query_plan::logical::LogicalOperator::ExactSubquery { .. }, .. + } + ))) + }; + let mut errors = vec![]; + let mut found = false; + let quotes = candidates + .into_iter() + .filter_map(|candidate| { + let plan = match PhysicalCompiler.compile(candidate.clone(), environment.clone()) { + Ok(plan) => plan, + Err(error) => { + errors.push(error.to_string()); + return None; + } + }; + let warm = fully_warm(&plan); + + found |= warm; + let manifest = workload_cost::manifest(&plan, &candidate.queries).unwrap(); + Some(WorkloadQuote { + unit_costs: manifest + .components + .keys() + .map(|key| (key.clone(), if warm { 1.0 } else { 1e12 })) + .collect(), + manifest, + executable: true, + }) + }) + .collect(); + assert!(found, "no complete warm candidate: {errors:?}"); + snapshot.workload_cost_evidence = Some(WorkloadCostEvidence { + backend_revision: BACKEND_REVISION.into(), + planner_revision: PLANNER_REVISION.into(), + data_snapshot_id: "issue-701-702-process".into(), + model_version: "synthetic-correctness-quotes".into(), + observed_at_unix_ms: environment.observed_at_unix_ms, + valid_for_ms: environment.max_evidence_age_ms, + quotes, + }); + let plan = snapshot.clone().compile().unwrap(); + assert!(fully_warm(&plan)); + let output = tempfile::tempdir().unwrap(); + let path = output.path().join("snapshot.json"); + std::fs::write(&path, serde_json::to_vec(&snapshot).unwrap()).unwrap(); + let port = unused_port(); + let mut command = Command::new(env!("CARGO_BIN_EXE_data_plane")); + command + .args(["--profile", "asapquery", "--planning-snapshot"]) + .arg(&path) + .args(["--http-port", &port.to_string(), "--output-dir"]) + .arg(output.path()) + .args([ + "--precompute-allowed-lateness-ms", + "0", + "--precompute-flush-interval-ms", + "25", + ]) + .stdout(Stdio::null()) + .stderr(Stdio::inherit()); + command.args([ + "--prometheus-server", + native.as_deref().unwrap_or(&mock_url), + "--forward-unsupported-queries", + ]); + let mut child = ChildGuard(command.spawn().unwrap()); + let client = reqwest::Client::new(); + let backend = format!("http://127.0.0.1:{port}"); + wait_until_ready(&client, &format!("{backend}/api/v1/health"), &mut child.0).await; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as i64; + let origin = now - now.rem_euclid(900_000) - 1_800_000; + for (start, end) in [(1, 901), (902, 961)] { + let wire = WriteRequest { + timeseries: [("a", "api", 1.0), ("b", "api", 2.0), ("c", "db", 3.0)] + .into_iter() + .map(|(pod, job, factor)| { + let samples: Vec<_> = (start..=end) + .map(|i| (origin + i * 1000, factor * (1 + i % 31) as f64)) + .collect(); + series_with_labels("issue701_data", &[("pod", pod), ("job", job)], &samples) + }) + .collect(), + }; + if let Some(url) = &native { + assert_eq!(remote_write(&client, url, &wire).await, 204); + } + assert_eq!(remote_write(&client, &backend, &wire).await, 204); + for (query, _, _) in &queries { + // Temporal reads trail the source watermark by one sample; current + // populations are queried at the latest input, without historical replay. + let evaluation = if query.contains('[') { end - 1 } else { end }; + let at = (origin + evaluation * 1000) as f64 / 1000.0; + let actual = wait_for_issue_warm_instant( + &client, + &backend, + query, + at, + &output.path().join("query_engine.log"), + ) + .await; + assert!(is_warm(&actual), "{query}: {actual}"); + if let Some(url) = &native { + let expected: Value = client + .get(format!("{url}/api/v1/query")) + .query(&[("query", query.clone()), ("time", at.to_string())]) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let rows = |body: &Value| -> std::collections::BTreeMap { + body["data"]["result"] + .as_array() + .unwrap() + .iter() + .map(|row| { + ( + serde_json::to_string(&row["metric"]).unwrap(), + row["value"][1].as_str().unwrap().parse().unwrap(), + ) + }) + .collect() + }; + let (actual, expected) = (rows(&actual), rows(&expected)); + assert_eq!( + actual.keys().collect::>(), + expected.keys().collect::>(), + "{query}" + ); + for (labels, truth) in expected { + let tolerance = if query.contains("quantile") { + 0.01 * truth.abs() + } else { + 1e-9 * truth.abs().max(1.0) + }; + assert!( + (actual[&labels] - truth).abs() <= tolerance, + "{query}: {} vs {truth}", + actual[&labels] + ); + } + } + } + } + if let Some(url) = &native { + let wire = WriteRequest { + timeseries: [("a", "api"), ("b", "api"), ("c", "db")] + .into_iter() + .map(|(pod, job)| { + let samples: Vec<_> = (962..=1261).map(|i| (origin + i * 1000, 0.0)).collect(); + series_with_labels("issue701_data", &[("pod", pod), ("job", job)], &samples) + }) + .collect(), + }; + assert_eq!(remote_write(&client, url, &wire).await, 204); + assert_eq!(remote_write(&client, &backend, &wire).await, 204); + let at = (origin + 1260 * 1000) as f64 / 1000.0; + for (query, _, _) in queries.iter().filter(|(q, _, _)| q.contains(" / ")) { + let params = [("query", query.clone()), ("time", at.to_string())]; + let expected: Value = client + .get(format!("{url}/api/v1/query")) + .query(¶ms) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let actual: Value = client + .get(format!("{backend}/api/v1/query")) + .query(¶ms) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert!( + !is_warm(&actual), + "undefined relative error must fall back: {query}" + ); + assert_eq!( + actual["data"], expected["data"], + "zero denominator: {query}" + ); + } + } +} diff --git a/data_plane/tests/support/univmon_erp_process.rs b/data_plane/tests/support/univmon_erp_process.rs index 7c496d4a..c800fc20 100644 --- a/data_plane/tests/support/univmon_erp_process.rs +++ b/data_plane/tests/support/univmon_erp_process.rs @@ -108,6 +108,8 @@ async fn measured_readout_evidence_selects_and_executes_univmon() { .map(|query| { let mut entry = template.clone(); entry["query"] = (*query).into(); + // Each observed population must match the five-second calibration workload. + entry["demand"]["fixed_interval_at"]["interval"] = 5000.into(); entry["requirements"]["accuracy"] = serde_json::json!({"explicit": {"Epsilon": 0.2}}); entry }) @@ -252,8 +254,7 @@ async fn measured_readout_evidence_selects_and_executes_univmon() { .enumerate() .map(|(i, v)| (base + 1 + i as i64, *v)) .collect(); - // Declared finite source includes the preceding boundary; this sample is - // outside the query's left-open range and does not alter its truth. + // Bracket the calibrated population; these boundary samples are outside it. samples.insert(0, (base, 0.0)); assert_eq!( remote_write( diff --git a/docs/developer_docs/query-engine/issue-701-code-review.md b/docs/developer_docs/query-engine/issue-701-code-review.md index 60f5ac9f..50fe4aae 100644 --- a/docs/developer_docs/query-engine/issue-701-code-review.md +++ b/docs/developer_docs/query-engine/issue-701-code-review.md @@ -1,68 +1,97 @@ -# Issue 701: windows, extrema, and composed quantile guarantees - -Reviewed against backend main `674b9573` plus PR 700 and Planner PR 404 -(`ea721e889b79e9ca22741a4d0370e9929bcf5b89`). The issue's original output was -produced by the removed v1 inspection path. This review uses executable code. - -## Window identity and runtime reads - -The current compiler derives pane and full-window alternatives from the query -lookback and evaluation cadence when the cadence divides the lookback. Thus a -300-second lookback evaluated every 30 seconds need not use tumbling 300-second -state. Complete workload cost evidence still determines the selected alternative. - -A separate runtime defect remained: full-window snapshots of width 5 seconds, -sliding every second, were retrieved through an overlap scan and cumulatively -merged. A process TopK regression returned 1500 instead of 200 for one item. -The query binding now carries the full-window slide explicitly, validates it -against the installed producer, checks evaluation phase on the slide grid, and -reads only the snapshot ending at the requested evaluation time. Missing snapshots -and incompatible ranges fail closed. Pane reads retain non-overlapping composition. -Old full-window artifacts without this binding must be recompiled before install. - -## min_over_time - -Planner currently represents Min and Max using the same ExactKind::MinMax family. -The backend's materialized readout supports Max; the physical compiler filters -out unsupported Min state. The legacy non-composable lowering then attempted to -bind the removed state and failed with `materialized query has no compiled -executable DAG`. - -The fix retains an explicit native fallback when all selected state lacks a -physical implementation. It does not reinterpret Min as Max or claim accelerated -Min support. Composable lowering retains its existing native dependency behavior. -Full accelerated Min needs an unambiguous Planner readout contract and matching -backend lowering, maintenance, and serving support. - -## Quantiles and division - -Multiple quantiles of the same population can share one maintained sketch. -The compiler regression covers q=0.5, 0.9, 0.95, and 0.99 with one materialization. -This does not imply the ratio inherits a component's relative-error bound. - -If both nonzero quantile values have relative errors at most alpha, the ratio -of their estimates differs from the true ratio by at most -`2 * alpha / (1 - alpha)` in relative terms. No independence assumption is used. -At alpha=1%, this sufficient bound is about 2.0202%. A sufficient component bound -for a 1% ratio target is alpha <= `0.01 / 2.01`, about 0.4975%, together with a -valid nonzero-denominator/domain contract. Sharing a sketch alone supplies no -proof of cancellation. Probabilistic guarantees also need a joint success bound; -rank error, such as a KLL guarantee, is not relative value error. - -Planner currently lacks this domain-aware division proof and conservatively -retains native execution. Even adding the formula would not make 1%-component -sketches satisfy a requested 1% ratio guarantee in general. - -## Remaining integration failures - -After the snapshot-read fix, the compatibility process suite reports 10 passing, -3 failing, and 1 previously ignored Collector-schema test. The failures are: - -- Counter range execution rejects missing full-pane coverage. -- Finite persisted-summary drain reports unpublished summary windows. -- UnivMon producer observations do not select UnivMon on replanning. - -These remain open; this change does not claim the complete compatibility matrix -passes. The three previously failing window/TopK-related executions include two -TopK algorithms and a multi-pane fixture whose explicit slide required updating. -No latency or end-to-end speedup measurement is claimed here. +# Issues 701 and 702: Planner candidates and backend execution + +Audience: developers reviewing Planner PR 404 and backend PR 700. The issues used +removed version-1 snapshots. Deployment now uses schema 2 with complete physical +workload cost evidence; these fixes do not restore the unpriced deployment path. + +## Rule ownership + +ASAPPlanner constructs the candidate DAG and its accuracy contract. The backend +lowers its typed operators, shares compatible physical state, prices complete +alternatives, and enforces runtime population and coverage constraints. + +- `ExactKind::Min` distinguishes minimum from the legacy maximum family. The + compiler emits the matching accumulator subtype and minimum readout. +- The temporal-average rule emits independently maintained sum and observation + count, exact finalization, and division. The backend can share their sum/count + producer without inventing an average rewrite. +- Current-series rules emit `MaintainCurrentSeries` and `ReadCurrentSeries` for + quantile, TopK, sum, count, and average, globally or grouped. The backend keeps + each series' latest live value and handles replacement, stale markers, expiry, + and bounded resources. This is exact current-population state, not an + append-only temporal DDSketch. Counts include equal-valued distinct series. +- Explicitly exact TopK over temporal aggregates can consume maintained exact + values. Existing approximate heap-sketch candidates retain their selection path. +- Current-series alternatives can coexist with temporal materializations in the + same workload. Equivalent current-only alternatives are deduplicated so they do + not produce ambiguous cost quotes. + +## Sliding-window execution + +The semantic lookback and evaluation cadence generate distinct pane and +full-window alternatives. Complete cost evidence chooses the physical layout. +Full-window bindings retain the slide and validate the requested phase/range. +Only the snapshot ending at the requested evaluation is read; overlapping full +snapshots must not be merged or treated as required pending work for that read. +Pane layouts retain their non-overlapping coverage checks and exclude the legacy +index's preceding carry-in frame from the query population. The bound-read +regression includes an out-of-range preceding population and verifies that its +counts and values do not enter the result. + +Unsigned storage excludes pre-epoch window starts from admission and publication. +Finite empty-window proofs use exact window identity for full-window layouts, +retain the replay/retention floor, and never equate missing unclosed input with +an empty population. Historical process checks explicitly declare their required +staleness/retention margin. + +## Quantile expression guarantees + +Point quantiles with compatible population and accuracy requirements share state. +Sharing alone does not make division preserve a component error bound. +For numerator error `a` and denominator error `b < 1`, a sufficient relative +ratio bound is `(a + b) / (1 - b)`. Independence is unnecessary for this +algebraic bound; probabilistic failures still require joint accounting. + +Planner's checked relative-division candidate sizes DDSketch operands against the +whole expression budget. A 1% ratio needs component accuracy slightly below +`0.01 / 2.01`, approximately 0.4975%, including floating-point slack. Average / +quantile uses the same composition rule with an exact numerator. Rank-only KLL +certificates do not establish this relative-value guarantee. + +The compiler lowers the typed checked division. Runtime requires finite operands, +a nonzero denominator, and a finite normal result. Unmet domain or coverage +conditions route to exact execution. An ordinary division is not silently given +a stronger guarantee. + +## Acceptance coverage + +Compiler regressions cover typed minimum, shared quantiles, average, and both +ratio forms. Runtime regressions cover current-series membership, checked-division +domain failures, storage-domain windows, pending full-window isolation, and finite +empty-window evidence. The process workload combines the issue query families, +ten quantiles, 15-minute and 5-minute windows, and consecutive evaluations while +input remains open. `ASAP_CURRENT_SERIES_PROMETHEUS_URL` enables real Prometheus +result comparison against Prometheus 3.x; the boundary-aligned fixture rejects a +2.x oracle because that version includes the left boundary, unlike the installed +PromQL window contract. See the [Prometheus migration guide](https://prometheus.io/docs/prometheus/3.5/migration/). +Test quotes are synthetic correctness preferences, not measured performance or +speedup evidence. + +The UnivMon integration fixture brackets one complete calibration population and +uses the same window cadence for the producer observation. A partial sliding tail +must not be asserted to match the complete calibration distribution. + +Validation on 2026-09-12: + +- Planner `cargo +1.98.0 test --workspace`: 1,097 passed. +- Backend `cargo +1.98.0 test --workspace --lib`: 2,098 passed; the subsequent + focused pane-population regression also passed. +- Compatibility process suite with a fresh Prometheus 3.5.0 remote-write oracle: + 14 passed, 1 previously ignored Collector-schema integration. The mixed test + checks 46 queries at two successive evaluations and two zero-denominator + fallbacks; the current-series test also compares replacement/staleness updates. +- Workspace/all-targets clippy passed with warnings denied; affected data-plane + checks were repeated after the final pane-read correction. + +These are correctness results. No measured performance win is claimed. The +separately ignored Collector schema integration remains outside these fixes. From 8a97796ed05deca2cc1d7fc67e9e43465e443940 Mon Sep 17 00:00:00 2001 From: zz_y Date: Sun, 13 Sep 2026 07:42:01 -0600 Subject: [PATCH 7/8] Lower generic population DAGs by executor capability and verify UnivMon sharing --- Cargo.lock | 10 +-- Cargo.toml | 8 +- control_plane/src/physical/compiler.rs | 82 ++++++++++++++----- ...ent_series.rs => maintained_population.rs} | 46 ++++++----- control_plane/src/physical/mod.rs | 2 +- control_plane/src/physical/workload_cost.rs | 14 +++- .../tests/support/current_series_process.rs | 20 ++--- .../tests/support/univmon_erp_process.rs | 41 ++++++++++ .../compiler-rule-boundary-review.md | 2 +- .../current-series-aggregations.md | 21 ++++- .../query-engine/issue-701-code-review.md | 2 +- 11 files changed, 178 insertions(+), 70 deletions(-) rename control_plane/src/physical/{current_series.rs => maintained_population.rs} (62%) diff --git a/Cargo.lock b/Cargo.lock index fc79427f..260369ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -373,7 +373,7 @@ dependencies = [ [[package]] name = "asap-aware-mapping" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" dependencies = [ "asap-types", "serde", @@ -384,7 +384,7 @@ dependencies = [ [[package]] name = "asap-frontend-promql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" dependencies = [ "asap-types", "promql-parser 0.10.0 (git+https://github.com/ProjectASAP/promql-parser?rev=9fede7eecca923c9882fe256484d00d37f8706cb)", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -416,12 +416,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=8a943abcefc009e4265fcea4ac3235245c342d52#8a943abcefc009e4265fcea4ac3235245c342d52" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index d3b18abc..53a9fb1f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,10 +20,10 @@ asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", branch [workspace.dependencies] # Keep Planner frontends, selection, and IR on the same immutable revision (current-series Planner PR). # Alias upstream asap-types because this workspace also defines asap_types. -planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } -asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } -asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } -asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "8a943abcefc009e4265fcea4ac3235245c342d52" } +planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } +asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } +asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } +asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } # Shared external deps (used by 2+ crates) serde = { version = "1.0", features = ["derive"] } diff --git a/control_plane/src/physical/compiler.rs b/control_plane/src/physical/compiler.rs index 48e91c7e..fa167a4f 100644 --- a/control_plane/src/physical/compiler.rs +++ b/control_plane/src/physical/compiler.rs @@ -973,7 +973,7 @@ impl PhysicalCompiler { environment: DeploymentEnvironment, metricsql: bool, ) -> Result { - if super::current_series::supported(&request) + if super::maintained_population::supported(&request) && (metricsql || environment.target != PhysicalDeploymentTarget::BackendLocalRemoteWrite) { @@ -1119,7 +1119,9 @@ impl PhysicalCompiler { .collect::>(); // An exact native fallback has no maintained state and must not // depend on evidence for unused window/state implementations. - if selected.is_empty() && super::current_series::operator(&request, query)?.is_none() { + if selected.is_empty() + && super::maintained_population::operator(&request, query)?.is_none() + { continue; } let executable = @@ -1491,20 +1493,21 @@ impl PhysicalCompiler { ); } - let plan_id = if request.hybrid_execution || super::current_series::supported(&request) { - use std::hash::{Hash, Hasher}; - let mut hash = std::collections::hash_map::DefaultHasher::new(); - stable_workload_plan_id(&plan_materializations, &request.queries).hash(&mut hash); - "typed-local-residual-v3-counter-index".hash(&mut hash); - super::current_series::supported(&request).hash(&mut hash); - request.materialization_policy.hash(&mut hash); - for query in &request.queries { - format!("{:?}", query.post_asap).hash(&mut hash); - } - hash.finish() - } else { - stable_workload_plan_id(&plan_materializations, &request.queries) - }; + let plan_id = + if request.hybrid_execution || super::maintained_population::supported(&request) { + use std::hash::{Hash, Hasher}; + let mut hash = std::collections::hash_map::DefaultHasher::new(); + stable_workload_plan_id(&plan_materializations, &request.queries).hash(&mut hash); + "typed-local-residual-v3-counter-index".hash(&mut hash); + super::maintained_population::supported(&request).hash(&mut hash); + request.materialization_policy.hash(&mut hash); + for query in &request.queries { + format!("{:?}", query.post_asap).hash(&mut hash); + } + hash.finish() + } else { + stable_workload_plan_id(&plan_materializations, &request.queries) + }; let envelope = PlanEnvelope { plan_id, plan_version: environment.plan_version, @@ -1651,7 +1654,7 @@ impl PhysicalCompiler { false }; let mut entry = if let Some(operator) = - super::current_series::operator(&request, query)? + super::maintained_population::operator(&request, query)? { let root = crate::query_plan::QueryNodeId(0); let compiled = executable_dags[query_index] @@ -3683,7 +3686,7 @@ pub(crate) mod tests { assert_eq!(populations.len(), 1); let installed = serde_json::to_string(&plan.precompute_plan.executable_dags).unwrap(); assert!( - installed.contains("MaintainCurrentSeries"), + installed.contains("MaintainPopulation"), "shared state must originate in the installed Planner DAG" ); } @@ -3699,26 +3702,61 @@ pub(crate) mod tests { ) .unwrap(), ); - let strategy = asap_aware_mapping::current_series::CurrentSeriesStrategy::new( + let strategy = asap_aware_mapping::maintained_population::MaintainedPopulationStrategy::new( std::slice::from_ref(&root), ); request.queries[0].post_asap = strategy.candidate(&root).unwrap(); - let before = super::super::current_series::operator(&request, &request.queries[0]) + let before = super::super::maintained_population::operator(&request, &request.queries[0]) .unwrap() .unwrap(); request.queries[0].query_string = "quantile(0.99, b)".into(); - let after = super::super::current_series::operator(&request, &request.queries[0]) + let after = super::super::maintained_population::operator(&request, &request.queries[0]) .unwrap() .unwrap(); assert_eq!(before, after); request.queries[0].post_asap = crate::planner_selection::keep_pre_asap(&root).unwrap(); assert!( - super::super::current_series::operator(&request, &request.queries[0]) + super::super::maintained_population::operator(&request, &request.queries[0]) .unwrap() .is_none() ); } + // A valid table-row DAG cannot be served by remote-write latest-series state. + #[test] + fn maintained_table_population_requires_a_compatible_executor() { + let mut request = request("sql", "quantile(0.5, a)"); + let mut root = crate::query_parser::parse_query_expr_canonical( + "quantile(0.5, a)", + AccuracyTarget::Exact, + ) + .unwrap(); + let planner_types::pre_asap::QueryExpr::Aggregate { child, .. } = &mut root else { + unreachable!() + }; + let planner_types::pre_asap::QueryExpr::Scan { source, schema, .. } = Rc::make_mut(child) + else { + unreachable!() + }; + *source = planner_types::pre_asap::Source::Table { + table_ref: "samples".into(), + }; + schema.closed = true; + let root = Rc::new(root); + let rule = asap_aware_mapping::maintained_population::MaintainedPopulationStrategy::new( + std::slice::from_ref(&root), + ); + let candidate = rule.candidate(&root).unwrap(); + planner_types::post_asap::compile_executable_dag(&candidate).unwrap(); + assert!(!super::super::maintained_population::supported_node( + &candidate + )); + request.queries[0].post_asap = candidate; + let error = super::super::maintained_population::operator(&request, &request.queries[0]) + .unwrap_err(); + assert!(error.to_string().contains("row-update executor"), "{error}"); + } + // The Planner's minimum state lowers without reconstructing direction from text. #[test] fn minimum_retains_its_typed_direction() { diff --git a/control_plane/src/physical/current_series.rs b/control_plane/src/physical/maintained_population.rs similarity index 62% rename from control_plane/src/physical/current_series.rs rename to control_plane/src/physical/maintained_population.rs index 2756d68d..8efffaf7 100644 --- a/control_plane/src/physical/current_series.rs +++ b/control_plane/src/physical/maintained_population.rs @@ -1,22 +1,24 @@ -//! Lower Planner-selected current-series operators; never discover query rewrites here. +//! Lower typed population operators according to executor membership capabilities. use super::compiler::{CompileError, PlanningQuery, PlanningRequest}; use asap_types::query_plan::{ current_series::{SeriesPopulation, SeriesReadout}, logical::{Grouping, LabelMatch, LabelMatcher, LogicalOperator}, }; -use planner_types::post_asap::{current_series::*, SummaryExpr, SummaryNode, ValueOperation}; +use planner_types::post_asap::{ + maintained_population::*, SummaryExpr, SummaryNode, ValueOperation, +}; -fn selected(node: &SummaryNode) -> Option<(&CurrentSeriesPopulation, &CurrentSeriesReadout)> { +fn selected(node: &SummaryNode) -> Option<(&MaintainedPopulation, &PopulationReadout)> { let SummaryExpr::ValueOperation { child, - operation: ValueOperation::ReadCurrentSeries { readout }, + operation: ValueOperation::ReadPopulation { readout }, .. } = &node.expr else { return None; }; let SummaryExpr::ValueOperation { - operation: ValueOperation::MaintainCurrentSeries { population }, + operation: ValueOperation::MaintainPopulation { population }, .. } = &child.expr else { @@ -25,11 +27,14 @@ fn selected(node: &SummaryNode) -> Option<(&CurrentSeriesPopulation, &CurrentSer Some((population, readout)) } +pub(super) fn supported_node(node: &SummaryNode) -> bool { + selected(node).is_some_and(|(population, _)| { + matches!(population.input, PopulationInput::CurrentSeries(_)) + }) +} + pub(super) fn supported(request: &PlanningRequest) -> bool { - request - .queries - .iter() - .any(|q| selected(&q.post_asap).is_some()) + request.queries.iter().any(|q| supported_node(&q.post_asap)) } pub(super) fn operator( @@ -39,6 +44,9 @@ pub(super) fn operator( let Some((spec, readout)) = selected(&query.post_asap) else { return Ok(None); }; + let PopulationInput::CurrentSeries(input) = &spec.input else { + return Err(CompileError::Query { query_id: query.query_id.clone(), reason: "maintained table-row populations require a row-update executor; remote-write current-series state is incompatible".into() }); + }; let populations: std::collections::BTreeSet<_> = request .queries .iter() @@ -53,8 +61,8 @@ pub(super) fn operator( .min(1_073_741_824) / populations.len().max(1) as u64; let population = SeriesPopulation { - metric: spec.metric.clone(), - matchers: spec + metric: input.metric.clone(), + matchers: input .matchers .iter() .map(|m| LabelMatcher { @@ -69,10 +77,10 @@ pub(super) fn operator( }) .collect(), grouping: Grouping { - labels: spec.grouping.clone(), - without: spec.without, + labels: input.grouping.clone(), + without: input.without, }, - lookback_ms: spec.lookback_ms, + lookback_ms: input.lookback_ms, max_k: spec.max_k as u64, quantiles: spec.quantiles, max_bytes, @@ -85,11 +93,11 @@ pub(super) fn operator( }; population.validate()?; let readout = match readout { - CurrentSeriesReadout::Quantile { q } => SeriesReadout::Quantile { q: *q }, - CurrentSeriesReadout::TopK { k } => SeriesReadout::TopK { k: *k as u64 }, - CurrentSeriesReadout::Sum => SeriesReadout::Sum, - CurrentSeriesReadout::Count => SeriesReadout::Count, - CurrentSeriesReadout::Average => SeriesReadout::Average, + PopulationReadout::Quantile { q } => SeriesReadout::Quantile { q: *q }, + PopulationReadout::TopK { k } => SeriesReadout::TopK { k: *k as u64 }, + PopulationReadout::Sum => SeriesReadout::Sum, + PopulationReadout::Count => SeriesReadout::Count, + PopulationReadout::Average => SeriesReadout::Average, }; Ok(Some(LogicalOperator::CurrentSeries { population, diff --git a/control_plane/src/physical/mod.rs b/control_plane/src/physical/mod.rs index eb955892..72b5abb2 100644 --- a/control_plane/src/physical/mod.rs +++ b/control_plane/src/physical/mod.rs @@ -50,4 +50,4 @@ pub use plan::{CostEstimate, ExecutionMode, PipelineStage, PlanNode, PlanSummary pub mod publication; -pub(crate) mod current_series; +pub(crate) mod maintained_population; diff --git a/control_plane/src/physical/workload_cost.rs b/control_plane/src/physical/workload_cost.rs index 9b852f9d..6c58902d 100644 --- a/control_plane/src/physical/workload_cost.rs +++ b/control_plane/src/physical/workload_cost.rs @@ -680,7 +680,7 @@ fn select_with_frontend( pub fn with_exact_alternative( request: PlanningRequest, ) -> Result, CompileError> { - let already_selected = super::current_series::supported(&request); + let already_selected = super::maintained_population::supported(&request); let mut alternatives = materialization_alternatives(request)?; if already_selected { return Ok(alternatives); @@ -695,8 +695,16 @@ pub fn with_exact_alternative( _ => unreachable!("native alternative retains canonical roots"), }) .collect(); - let strategy = asap_aware_mapping::current_series::CurrentSeriesStrategy::new(&roots); - let candidates: Vec<_> = roots.iter().map(|root| strategy.candidate(root)).collect(); + let strategy = + asap_aware_mapping::maintained_population::MaintainedPopulationStrategy::new(&roots); + let candidates: Vec<_> = roots + .iter() + .map(|root| { + strategy + .candidate(root) + .filter(|node| super::maintained_population::supported_node(node)) + }) + .collect(); if candidates.iter().any(Option::is_some) { // Current-series rules are compatible with window summaries in other // workload roots. Preserve each priced temporal alternative and mask. diff --git a/data_plane/tests/support/current_series_process.rs b/data_plane/tests/support/current_series_process.rs index 627be60e..cbec7c3c 100644 --- a/data_plane/tests/support/current_series_process.rs +++ b/data_plane/tests/support/current_series_process.rs @@ -61,17 +61,15 @@ async fn current_series_quantiles_topk_share_and_replace_values() { let plan = PhysicalCompiler .compile(candidate.clone(), env.clone()) .ok()?; - let warm = - candidate.queries.iter().all(|query| { - matches!( - &query.post_asap.expr, - planner_types::post_asap::SummaryExpr::ValueOperation { - operation: - planner_types::post_asap::ValueOperation::ReadCurrentSeries { .. }, - .. - } - ) - }); + let warm = candidate.queries.iter().all(|query| { + matches!( + &query.post_asap.expr, + planner_types::post_asap::SummaryExpr::ValueOperation { + operation: planner_types::post_asap::ValueOperation::ReadPopulation { .. }, + .. + } + ) + }); let manifest = workload_cost::manifest(&plan, &candidate.queries).unwrap(); Some(WorkloadQuote { unit_costs: manifest diff --git a/data_plane/tests/support/univmon_erp_process.rs b/data_plane/tests/support/univmon_erp_process.rs index c800fc20..8d6a2b04 100644 --- a/data_plane/tests/support/univmon_erp_process.rs +++ b/data_plane/tests/support/univmon_erp_process.rs @@ -139,6 +139,45 @@ async fn measured_readout_evidence_selects_and_executes_univmon() { .any(|m| m.aggregation_type == asap_types::AggregationType::UnivMon), "{plan:#?}" ); + // All three readouts can use one state when the selected parameters and + // population agree. Each still needs its own calibration evidence. + let mut shared_fixture = fixture.clone(); + shared_fixture["implementation"]["erp"]["runtime"]["allowed_algorithms"] = + serde_json::json!(["UnivMon"]); + let records = shared_fixture["implementation"]["erp"]["artifact"]["records"] + .as_array_mut() + .unwrap(); + records.remove(0); + let shared = quote_snapshot_for_test( + serde_json::from_value::(shared_fixture.clone()).unwrap(), + ) + .compile() + .unwrap(); + assert_eq!( + shared.precompute_plan.materializations.len(), + 1, + "distinct, L2 and entropy share one frequency population: {shared:#?}" + ); + assert_eq!( + shared.precompute_plan.materializations[0].aggregation_type, + asap_types::AggregationType::UnivMon + ); + for query in queries { + let entry = shared + .query_plan + .entries + .values() + .find(|e| e.canonical_query == query) + .unwrap(); + assert!( + !entry.nodes.values().any(|n| matches!( + n, + control_plane::query_plan::QueryPlanNode::ExactFallback { .. } + | control_plane::query_plan::QueryPlanNode::ExternalExact { .. } + )), + "{entry:#?}" + ); + } // Removing only entropy evidence must leave the L2 path executable. let mut missing_entropy = fixture.clone(); for row in missing_entropy["implementation"]["erp"]["artifact"]["records"] @@ -188,6 +227,8 @@ async fn measured_readout_evidence_selects_and_executes_univmon() { .. } ))); + let plan = shared; + let fixture = shared_fixture; let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let fallback_url = format!("http://{}", listener.local_addr().unwrap()); let fallback = tokio::spawn(async move { diff --git a/docs/developer_docs/query-engine/compiler-rule-boundary-review.md b/docs/developer_docs/query-engine/compiler-rule-boundary-review.md index 9ea4abf6..7c432093 100644 --- a/docs/developer_docs/query-engine/compiler-rule-boundary-review.md +++ b/docs/developer_docs/query-engine/compiler-rule-boundary-review.md @@ -18,7 +18,7 @@ Rechecked after merging main `674b9573` (#699): the four implementation files co ## PR #700 correction -The original backend `current_series.rs` parsed PromQL and independently assembled a maintained-state alternative. The correction moves recognition and workload sharing into Planner's `CurrentSeriesStrategy`, adds typed maintenance/readout operators, removes `PlanningRequest.current_series`, and lowers the selected operators directly. Regression tests check that the installed Planner DAG contains the producer and that changing catalog text does not change physical operator binding. +The original backend `current_series.rs` parsed PromQL and independently assembled a maintained-state alternative. The correction moves recognition and workload sharing into Planner's `MaintainedPopulationStrategy`, adds typed maintenance/readout operators, removes `PlanningRequest.current_series`, and lowers the selected operators directly. Regression tests check that the installed Planner DAG contains the producer and that changing catalog text does not change physical operator binding. ## Within the backend's responsibility diff --git a/docs/developer_docs/query-engine/current-series-aggregations.md b/docs/developer_docs/query-engine/current-series-aggregations.md index f78793ba..444c1f4a 100644 --- a/docs/developer_docs/query-engine/current-series-aggregations.md +++ b/docs/developer_docs/query-engine/current-series-aggregations.md @@ -6,9 +6,9 @@ alternative for `quantile(q, metric)` and `topk(k, metric)`, including `by` and literals. Selector offsets, `@`, nested input expressions and MetricsQL use the existing alternatives; they are not admitted by this implementation. -ASAPPlanner owns this transformation through the opt-in `CurrentSeriesStrategy` -over canonical IR. It emits `MaintainCurrentSeries` at maintenance time and -`ReadCurrentSeries` at read time, with source/filter/group identity, quantile +ASAPPlanner owns this transformation through the opt-in `MaintainedPopulationStrategy` +over canonical IR. It emits `MaintainPopulation` at maintenance time and +`ReadPopulation` at read time, with source/filter/group identity, quantile consumers and the maximum requested k in its typed contract. Compatible producers are shared by Planner CSE. The backend consumes these nodes, binds resource and input-lag limits, and retains the Planner DAG in the installed plan. It does not @@ -70,3 +70,18 @@ Prometheus instance with `--web.enable-remote-write-receiver`, then run the proc test. It writes the same samples to both services and compares values and labels for quantiles and TopK, including value replacement and staleness. Test quotes are synthetic and must not be used as performance evidence. + +The Planner population IR also represents table-row multisets. They share the +aggregate rule and readout vocabulary with current-series populations, but not +the membership contract. The backend remote-write executor admits only the +current-series variant; explicitly selected table-row state returns a capability +error until a row-update/deletion executor is available. Existing SQL window +summary compilation remains independent of this capability. + +The generalization is covered by six SQL frontend tests in Planner (quantiles, +scalar readouts, TopK limits, grouping, filters and invalid value columns), backend +capability rejection, and the current-series process test against Prometheus 3.5. +The UnivMon process test also installs one shared materialization for distinct, +frequency L2 and entropy with identical input/window/parameters, checks all three +readouts against held-out raw values, and checks readout-specific missing-evidence +fallback. This establishes sharing and correctness, not a measured speedup. diff --git a/docs/developer_docs/query-engine/issue-701-code-review.md b/docs/developer_docs/query-engine/issue-701-code-review.md index 50fe4aae..17e938a4 100644 --- a/docs/developer_docs/query-engine/issue-701-code-review.md +++ b/docs/developer_docs/query-engine/issue-701-code-review.md @@ -15,7 +15,7 @@ alternatives, and enforces runtime population and coverage constraints. - The temporal-average rule emits independently maintained sum and observation count, exact finalization, and division. The backend can share their sum/count producer without inventing an average rewrite. -- Current-series rules emit `MaintainCurrentSeries` and `ReadCurrentSeries` for +- Current-series rules emit `MaintainPopulation` and `ReadPopulation` for quantile, TopK, sum, count, and average, globally or grouped. The backend keeps each series' latest live value and handles replacement, stale markers, expiry, and bounded resources. This is exact current-population state, not an From ea31d1b54da5815a6abed418a4e7a52c5ae96255 Mon Sep 17 00:00:00 2001 From: zz_y Date: Sun, 13 Sep 2026 08:34:52 -0600 Subject: [PATCH 8/8] fix: expire lookback boundary members and guard temporal average overflow --- Cargo.lock | 10 +- Cargo.toml | 8 +- control_plane/src/physical/compiler.rs | 28 ++++ control_plane/src/query_plan.rs | 5 +- control_plane/src/query_plan/logical.rs | 11 +- control_plane/tests/offline_evidence.rs | 1 + crates/asap_types/src/query_plan/logical.rs | 2 + .../precompute_engine/maintenance_runtime.rs | 1 + .../src/precompute_engine/subdag_scheduler.rs | 1 + .../asap_query_engine/logical_dag.rs | 60 +++++++- .../asap_query_engine/summary_exec.rs | 1 + .../sketch_db/current_series.rs | 37 ++++- .../tests/support/current_series_process.rs | 27 ++++ .../tests/support/issue_701_702_process.rs | 140 ++++++++++++++++++ .../current-series-aggregations.md | 5 + .../query-engine/issue-701-code-review.md | 20 +++ 16 files changed, 335 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 260369ba..991bfe8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -373,7 +373,7 @@ dependencies = [ [[package]] name = "asap-aware-mapping" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=b8b5d705215200361b6a2c3b389d513d554077ca#b8b5d705215200361b6a2c3b389d513d554077ca" dependencies = [ "asap-types", "serde", @@ -384,7 +384,7 @@ dependencies = [ [[package]] name = "asap-frontend-promql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=b8b5d705215200361b6a2c3b389d513d554077ca#b8b5d705215200361b6a2c3b389d513d554077ca" dependencies = [ "asap-types", "promql-parser 0.10.0 (git+https://github.com/ProjectASAP/promql-parser?rev=9fede7eecca923c9882fe256484d00d37f8706cb)", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=b8b5d705215200361b6a2c3b389d513d554077ca#b8b5d705215200361b6a2c3b389d513d554077ca" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -416,12 +416,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=b8b5d705215200361b6a2c3b389d513d554077ca#b8b5d705215200361b6a2c3b389d513d554077ca" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=7f84cbbd8773d8dedc69890d62b18a3d4e479779#7f84cbbd8773d8dedc69890d62b18a3d4e479779" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=b8b5d705215200361b6a2c3b389d513d554077ca#b8b5d705215200361b6a2c3b389d513d554077ca" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 53a9fb1f..fecbce11 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,10 +20,10 @@ asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", branch [workspace.dependencies] # Keep Planner frontends, selection, and IR on the same immutable revision (current-series Planner PR). # Alias upstream asap-types because this workspace also defines asap_types. -planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } -asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } -asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } -asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "7f84cbbd8773d8dedc69890d62b18a3d4e479779" } +planner-types = { package = "asap-types", git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "b8b5d705215200361b6a2c3b389d513d554077ca" } +asap-aware-mapping = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "b8b5d705215200361b6a2c3b389d513d554077ca" } +asap-frontend-promql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "b8b5d705215200361b6a2c3b389d513d554077ca" } +asap-frontend-sql = { git = "https://github.com/ProjectASAP/ASAPPlanner", rev = "b8b5d705215200361b6a2c3b389d513d554077ca" } # Shared external deps (used by 2+ crates) serde = { version = "1.0", features = ["derive"] } diff --git a/control_plane/src/physical/compiler.rs b/control_plane/src/physical/compiler.rs index fa167a4f..b37e8413 100644 --- a/control_plane/src/physical/compiler.rs +++ b/control_plane/src/physical/compiler.rs @@ -3757,6 +3757,33 @@ pub(crate) mod tests { assert!(error.to_string().contains("row-update executor"), "{error}"); } + // Compiler preserves the Planner's conditional-average execution guard. + #[test] + fn temporal_average_lowers_with_finite_division_guard() { + let mut environment = environment(10_000); + environment.target = PhysicalDeploymentTarget::BackendLocalRemoteWrite; + environment.collector_ids.clear(); + let request = request("average", "avg_over_time(a[1m])"); + let plan = PhysicalCompiler.compile(request, environment).unwrap(); + assert!( + plan.query_plan + .entries + .values() + .flat_map(|e| e.nodes.values()) + .any(|node| matches!( + node, + crate::query_plan::QueryPlanNode::Logical { + operator: asap_types::query_plan::logical::LogicalOperator::Binary { + operation: asap_types::query_plan::logical::BinaryOperation::FiniteDiv, + .. + }, + .. + } + )), + "{plan:#?}" + ); + } + // The Planner's minimum state lowers without reconstructing direction from text. #[test] fn minimum_retains_its_typed_direction() { @@ -5448,6 +5475,7 @@ pub(crate) mod tests { rhs: selected.clone(), operator: planner_types::post_asap::BinaryOperator { checked_relative_division: false, + checked_finite_division: false, kind: planner_types::pre_asap::BinaryOpKind::Arithmetic( planner_types::pre_asap::ArithmeticOpKind::Add, ), diff --git a/control_plane/src/query_plan.rs b/control_plane/src/query_plan.rs index 182049c8..deb1dc88 100644 --- a/control_plane/src/query_plan.rs +++ b/control_plane/src/query_plan.rs @@ -621,7 +621,10 @@ where rhs, operator, timing: planner_types::post_asap::ExecutionTiming::ReadTime, - } if self.logical_source.is_some() || operator.checked_relative_division => { + } if self.logical_source.is_some() + || operator.checked_relative_division + || operator.checked_finite_division => + { let operator = logical::binary_operator(operator)?; QueryPlanNode::Logical { operator, diff --git a/control_plane/src/query_plan/logical.rs b/control_plane/src/query_plan/logical.rs index 73dfc330..6aa6d448 100644 --- a/control_plane/src/query_plan/logical.rs +++ b/control_plane/src/query_plan/logical.rs @@ -342,8 +342,9 @@ pub(super) fn residual_nodes( pub(super) fn binary_operator( operator: &planner_types::post_asap::BinaryOperator, ) -> Result { - if operator.checked_relative_division { - if operator.vector_match.is_some() + if operator.checked_relative_division || operator.checked_finite_division { + if (operator.checked_relative_division && operator.checked_finite_division) + || operator.vector_match.is_some() || !matches!( operator.kind, planner_types::pre_asap::BinaryOpKind::Arithmetic( @@ -354,7 +355,11 @@ pub(super) fn binary_operator( return Err(invalid("invalid Planner checked division contract")); } return Ok(LogicalOperator::Binary { - operation: BinaryOperation::CheckedDiv, + operation: if operator.checked_finite_division { + BinaryOperation::FiniteDiv + } else { + BinaryOperation::CheckedDiv + }, return_bool: false, }); } diff --git a/control_plane/tests/offline_evidence.rs b/control_plane/tests/offline_evidence.rs index ce2338c6..abcd47ef 100644 --- a/control_plane/tests/offline_evidence.rs +++ b/control_plane/tests/offline_evidence.rs @@ -356,6 +356,7 @@ fn binary_summary_has_explicit_warm_tier_fallback() { rhs: child.clone(), operator: BinaryOperator { checked_relative_division: false, + checked_finite_division: false, kind: BinaryOpKind::Arithmetic(planner_types::pre_asap::ArithmeticOpKind::Div), vector_match: None, }, diff --git a/crates/asap_types/src/query_plan/logical.rs b/crates/asap_types/src/query_plan/logical.rs index b0064bb8..9ac0ba04 100644 --- a/crates/asap_types/src/query_plan/logical.rs +++ b/crates/asap_types/src/query_plan/logical.rs @@ -99,6 +99,8 @@ pub enum BinaryOperation { Div, /// Division with the Planner relative-value certificate domain checks. CheckedDiv, + /// Division for conditional exact rewrites: finite inputs and finite output. + FiniteDiv, Mod, Pow, Equal, diff --git a/data_plane/src/precompute_engine/maintenance_runtime.rs b/data_plane/src/precompute_engine/maintenance_runtime.rs index e570579b..7afdc396 100644 --- a/data_plane/src/precompute_engine/maintenance_runtime.rs +++ b/data_plane/src/precompute_engine/maintenance_runtime.rs @@ -3256,6 +3256,7 @@ mod tests { operation.output_schema.time_index = Some(0); let mut operator = BinaryOperator { checked_relative_division: false, + checked_finite_division: false, kind: BinaryOpKind::Arithmetic(ArithmeticOpKind::Sub), vector_match: None, }; diff --git a/data_plane/src/precompute_engine/subdag_scheduler.rs b/data_plane/src/precompute_engine/subdag_scheduler.rs index e7ac372c..918f8477 100644 --- a/data_plane/src/precompute_engine/subdag_scheduler.rs +++ b/data_plane/src/precompute_engine/subdag_scheduler.rs @@ -329,6 +329,7 @@ mod tests { timing: planner_types::post_asap::ExecutionTiming::MaintenanceTime, operator: BinaryOperator { checked_relative_division: false, + checked_finite_division: false, kind: BinaryOpKind::Arithmetic(ArithmeticOpKind::Sub), vector_match: None, }, diff --git a/data_plane/src/query_engines/asap_query_engine/logical_dag.rs b/data_plane/src/query_engines/asap_query_engine/logical_dag.rs index c2c9aaab..4642b058 100644 --- a/data_plane/src/query_engines/asap_query_engine/logical_dag.rs +++ b/data_plane/src/query_engines/asap_query_engine/logical_dag.rs @@ -552,7 +552,10 @@ fn binary( left: Value, right: Value, ) -> Result { - if operation == BinaryOperation::CheckedDiv { + if matches!( + operation, + BinaryOperation::CheckedDiv | BinaryOperation::FiniteDiv + ) { let valid = |value: &Value, denominator: bool| match value { Value::Scalar(v) => v.is_finite() && (!denominator || *v != 0.0), Value::Vector(rows) => rows @@ -562,19 +565,28 @@ fn binary( }; if boolean || !valid(&left, false) || !valid(&right, true) { return Err(miss( - "relative division domain requires finite operands and a nonzero divisor", + "checked division requires finite operands and a nonzero divisor", )); } let result = binary(BinaryOperation::Div, false, left, right)?; + let valid_result = |v: &f64| { + if operation == BinaryOperation::FiniteDiv { + v.is_finite() + } else { + v.is_normal() + } + }; let normal = match &result { - Value::Scalar(v) => v.is_normal(), - Value::Vector(rows) => rows.iter().all(|(_, v)| v.is_normal()), + Value::Scalar(v) => valid_result(v), + Value::Vector(rows) => rows.iter().all(|(_, v)| valid_result(v)), Value::Matrix(..) => false, }; return if normal { Ok(result) } else { - Err(miss("relative division result requires exact evaluation outside normal floating-point range")) + Err(miss( + "checked division result is outside the declared floating-point domain", + )) }; } let arithmetic = matches!( @@ -775,6 +787,44 @@ mod topk_tests { .collect() } + // An overflowing sum cannot implement average, but zero/subnormal averages remain valid. + #[test] + fn finite_division_guards_temporal_average_without_rejecting_zero() { + let mut sum = crate::precompute_engine::operators::sum_accumulator::SumAccumulator::new(); + sum.update(1e308); + sum.update(1e308); + assert!(binary( + BinaryOperation::FiniteDiv, + false, + Value::Scalar(sum.sum), + Value::Scalar(2.0) + ) + .is_err()); + for (a, b, expected) in [ + (0.0, 2.0, 0.0), + (10.0, 2.0, 5.0), + (f64::MIN_POSITIVE, 2.0, f64::MIN_POSITIVE / 2.0), + ] { + let Value::Scalar(value) = binary( + BinaryOperation::FiniteDiv, + false, + Value::Scalar(a), + Value::Scalar(b), + ) + .unwrap() else { + panic!("scalar") + }; + assert_eq!(value, expected); + } + assert!(binary( + BinaryOperation::FiniteDiv, + false, + Value::Scalar(1.0), + Value::Scalar(0.0) + ) + .is_err()); + } + // A conditional accuracy certificate must fall back rather than return an unbounded ratio. #[test] fn checked_relative_division_enforces_its_execution_domain() { diff --git a/data_plane/src/query_engines/asap_query_engine/summary_exec.rs b/data_plane/src/query_engines/asap_query_engine/summary_exec.rs index 0dec5060..ff0ef957 100644 --- a/data_plane/src/query_engines/asap_query_engine/summary_exec.rs +++ b/data_plane/src/query_engines/asap_query_engine/summary_exec.rs @@ -549,6 +549,7 @@ mod tests { rhs: child.clone(), operator: planner_types::post_asap::BinaryOperator { checked_relative_division: false, + checked_finite_division: false, kind: planner_types::pre_asap::BinaryOpKind::Arithmetic( planner_types::pre_asap::ArithmeticOpKind::Div, ), diff --git a/data_plane/src/storage_engines/sketch_db/current_series.rs b/data_plane/src/storage_engines/sketch_db/current_series.rs index 79c1a37f..fd809dc4 100644 --- a/data_plane/src/storage_engines/sketch_db/current_series.rs +++ b/data_plane/src/storage_engines/sketch_db/current_series.rs @@ -104,7 +104,7 @@ impl Population { } fn expire(&mut self, cutoff: i64) { while let Some((timestamp, labels)) = self.expiry.first().cloned() { - if timestamp >= cutoff { + if timestamp > cutoff { break; } self.remove(&labels); @@ -113,7 +113,7 @@ impl Population { fn update(&mut self, sample: &CanonicalSample, cutoff: i64) { if self.unavailable || sample.metric.as_ref() != self.definition.metric - || sample.timestamp_ms < cutoff + || sample.timestamp_ms <= cutoff { return; } @@ -569,8 +569,8 @@ mod tests { .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 600_000) .unwrap() .len(), - 2 - ); // inclusive lookback boundary + 1 + ); // Prometheus 3.5 lookback is left-open assert_eq!( store .read((7, 1), &p, &SeriesReadout::Quantile { q: 0.5 }, 600_001) @@ -600,4 +600,33 @@ mod tests { .unwrap_err() .contains("budget")); } + + // Prometheus 3.5 selectors exclude samples exactly at evaluation - lookback. + #[test] + fn lookback_left_boundary_expires_members_for_all_shared_readouts() { + let p = definition(); + let plan = plan(&p); + let mut store = CurrentSeriesStore::default(); + warm(&mut store, &plan); + for t in (360_000..=600_000).step_by(60_000) { + store.ingest(&plan, &[sample("y", "api", t, Some(9.))]); + } + for (readout, expected) in [ + (SeriesReadout::Count, 1.0), + (SeriesReadout::Sum, 9.0), + (SeriesReadout::Average, 9.0), + (SeriesReadout::Quantile { q: 0.5 }, 9.0), + ] { + let rows = store.read((7, 1), &p, &readout, 600_000).unwrap(); + assert_eq!( + rows, + vec![(BTreeMap::from([("job".into(), "api".into())]), expected)] + ); + } + let rows = store + .read((7, 1), &p, &SeriesReadout::TopK { k: 3 }, 600_000) + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].0["pod"], "y"); + } } diff --git a/data_plane/tests/support/current_series_process.rs b/data_plane/tests/support/current_series_process.rs index cbec7c3c..656aaef2 100644 --- a/data_plane/tests/support/current_series_process.rs +++ b/data_plane/tests/support/current_series_process.rs @@ -343,5 +343,32 @@ async fn current_series_quantiles_topk_share_and_replace_values() { "{body}" ); } + // Keep one series fresh while all members last seen at `end` hit the exact + // left lookback boundary. Native Prometheus 3.5 must agree for every readout. + let updates: Vec<_> = (60_000..=300_000) + .step_by(60_000) + .map(|offset| (end + offset, 9.0)) + .collect(); + let wire = WriteRequest { + timeseries: vec![series_with_labels( + "a", + &[("pod", "y"), ("job", "api")], + &updates, + )], + }; + if let Some(url) = &native { + assert_eq!(remote_write(&client, url, &wire).await, 204); + } + assert_eq!(remote_write(&client, &base, &wire).await, 204); + for text in &queries { + let body = query(&client, &base, text, end + 300_000).await; + assert!(is_warm(&body), "{text}: {body}"); + assert_eq!( + body["data"]["result"].as_array().unwrap().len(), + 1, + "{text}: {body}" + ); + compare_native(&client, &native, text, end + 300_000, &body).await; + } task.abort(); } diff --git a/data_plane/tests/support/issue_701_702_process.rs b/data_plane/tests/support/issue_701_702_process.rs index fd66ff89..0af326e0 100644 --- a/data_plane/tests/support/issue_701_702_process.rs +++ b/data_plane/tests/support/issue_701_702_process.rs @@ -341,3 +341,143 @@ async fn issue_workloads_execute_warm_at_successive_evaluations() { } } } + +// Finite input can overflow sum; the installed average must fall back while zero stays warm. +#[tokio::test] +async fn temporal_average_overflow_falls_back_after_state_is_warm() { + let native = std::env::var("ASAP_CURRENT_SERIES_PROMETHEUS_URL").ok(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let mock_url = format!("http://{}", listener.local_addr().unwrap()); + let mock = tokio::spawn(async move { + axum::serve(listener, Router::new() + .route("/-/healthy", get(|| async { "healthy" })) + .route("/api/v1/query", get(|| async { Json(serde_json::json!({"status":"success", "data":{"resultType":"vector", "result":[{"metric":{},"value":[0,"1e308"]}]}})) }))) + .await.unwrap(); + }); + let mut fixture: Value = serde_json::from_str(include_str!( + "../../../docs/examples/asapquery-planning-snapshot.json" + )) + .unwrap(); + fixture["implementation"]["source_sample_interval_ms"] = 1000.into(); + let template = fixture["query_workload"]["repeating_queries"][0].clone(); + fixture["query_workload"]["repeating_queries"] = ["avg", "sum", "count"] + .map(|op| { + let mut entry = template.clone(); + entry["query"] = format!("{op}_over_time(average_overflow[5s])").into(); + entry["time_selection"]["lookback"] = 5000.into(); + entry["demand"]["fixed_interval_at"]["interval"] = 1000.into(); + entry["requirements"]["accuracy"] = serde_json::json!({"explicit":"Exact"}); + entry + }) + .to_vec() + .into(); + let snapshot = quote_snapshot_for_test(serde_json::from_value(fixture).unwrap()); + let plan = snapshot.clone().compile().unwrap(); + assert!(plan + .query_plan + .entries + .values() + .flat_map(|entry| entry.nodes.values()) + .any(|node| matches!( + node, + control_plane::query_plan::QueryPlanNode::Logical { + operator: control_plane::query_plan::logical::LogicalOperator::Binary { + operation: control_plane::query_plan::logical::BinaryOperation::FiniteDiv, + .. + }, + .. + } + ))); + let output = tempfile::tempdir().unwrap(); + let path = output.path().join("snapshot.json"); + std::fs::write(&path, serde_json::to_vec(&snapshot).unwrap()).unwrap(); + let port = unused_port(); + let mut command = Command::new(env!("CARGO_BIN_EXE_data_plane")); + command + .args(["--profile", "asapquery", "--planning-snapshot"]) + .arg(&path) + .args(["--http-port", &port.to_string(), "--output-dir"]) + .arg(output.path()) + .args([ + "--precompute-allowed-lateness-ms", + "0", + "--precompute-flush-interval-ms", + "25", + "--prometheus-server", + native.as_deref().unwrap_or(&mock_url), + "--forward-unsupported-queries", + ]) + .stdout(Stdio::null()) + .stderr(Stdio::inherit()); + let mut child = ChildGuard(command.spawn().unwrap()); + let client = reqwest::Client::new(); + let backend = format!("http://127.0.0.1:{port}"); + wait_until_ready(&client, &format!("{backend}/api/v1/health"), &mut child.0).await; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as i64; + let origin = now - now.rem_euclid(1000) - 60_000; + for (start, end, value) in [(1, 6, 0.0), (7, 12, 1e308)] { + let samples: Vec<_> = (start..=end).map(|i| (origin + i * 1000, value)).collect(); + let wire = WriteRequest { + timeseries: vec![series_with_labels("average_overflow", &[], &samples)], + }; + if let Some(url) = &native { + assert_eq!(remote_write(&client, url, &wire).await, 204); + } + assert_eq!(remote_write(&client, &backend, &wire).await, 204); + let at = (origin + (end - 1) * 1000) as f64 / 1000.0; + for op in ["sum", "count"] { + wait_for_issue_warm_instant( + &client, + &backend, + &format!("{op}_over_time(average_overflow[5s])"), + at, + &output.path().join("query_engine.log"), + ) + .await; + } + let query = "avg_over_time(average_overflow[5s])"; + if value == 0.0 { + let result = wait_for_issue_warm_instant( + &client, + &backend, + query, + at, + &output.path().join("query_engine.log"), + ) + .await; + assert_eq!(first_value(&result, "value"), Some(0.0)); + } else { + let params = [("query", query.to_string()), ("time", at.to_string())]; + let actual: Value = client + .get(format!("{backend}/api/v1/query")) + .query(¶ms) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert!( + !is_warm(&actual), + "overflowed average must fall back: {actual}" + ); + assert_eq!(first_value(&actual, "value"), Some(1e308), "{actual}"); + if let Some(url) = &native { + let expected: Value = client + .get(format!("{url}/api/v1/query")) + .query(¶ms) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert_eq!(actual["data"], expected["data"]); + } + } + } + mock.abort(); +} diff --git a/docs/developer_docs/query-engine/current-series-aggregations.md b/docs/developer_docs/query-engine/current-series-aggregations.md index 444c1f4a..77fda0b7 100644 --- a/docs/developer_docs/query-engine/current-series-aggregations.md +++ b/docs/developer_docs/query-engine/current-series-aggregations.md @@ -85,3 +85,8 @@ The UnivMon process test also installs one shared materialization for distinct, frequency L2 and entropy with identical input/window/parameters, checks all three readouts against held-out raw values, and checks readout-specific missing-evidence fallback. This establishes sharing and correctness, not a measured speedup. + +Current-series lookback follows Prometheus 3.5: `(evaluation - 5m, evaluation]`. +Expiration removes members at the lower boundary, and admission does not reinsert +samples at that boundary. The process regression compares every shared readout +with native Prometheus when only one series remains fresh. diff --git a/docs/developer_docs/query-engine/issue-701-code-review.md b/docs/developer_docs/query-engine/issue-701-code-review.md index 17e938a4..eac11bda 100644 --- a/docs/developer_docs/query-engine/issue-701-code-review.md +++ b/docs/developer_docs/query-engine/issue-701-code-review.md @@ -95,3 +95,23 @@ Validation on 2026-09-12: These are correctness results. No measured performance win is claimed. The separately ignored Collector schema integration remains outside these fixes. + +## Review follow-up: lookback boundary and average overflow + +The exact lower lookback boundary now expires current-series members, matching +Prometheus 3.5. A regression covers count, sum, average, quantile and TopK when +all but one series reach that boundary. + +Temporal average no longer exports an unconditional sum/count logical rewrite. +Planner marks the physical division with `checked_finite_division`; the compiler +retains it as `FiniteDiv`. Nonfinite operands/results or a zero divisor trigger +the original-query fallback. A zero or subnormal finite average remains eligible. +The production HTTP test first waits for both sum and count state to become warm, +then verifies that overflowing `1e308` samples return the native finite average. + +Validation on 2026-09-13: backend workspace library tests passed (2,102); the +Prometheus 3.5 compatibility process suite passed 15 tests with the existing +Collector-schema test ignored; workspace/all-targets clippy passed with warnings +denied. Planner workspace unit/integration tests passed, and the workspace +doctests passed on a separate run after a transient cached-crate lookup failure. +The Planner's GitHub test and format/lint checks also passed.