Skip to content

Commit b4d3c50

Browse files
committed
fix(planner): preserve maintained population selection horizons
1 parent dcd1513 commit b4d3c50

3 files changed

Lines changed: 60 additions & 11 deletions

File tree

‎crates/asap-aware-mapping/src/maintained_population.rs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,17 @@ fn recognize(root: &QueryExpr) -> Option<(MaintainedPopulation, PopulationReadou
119119
return Some((population, readout, Rc::clone(source)));
120120
}
121121
// A bare PromQL selector carries the declared ingestion interval as a
122-
// temporal input scope. It still denotes the same current-series
123-
// population; preserve the wrapper as the maintained input below.
124-
let series_source = match source.as_ref() {
125-
QueryExpr::TimeRange { child, .. } => child.as_ref(),
126-
other => other,
122+
// temporal input scope. Membership must expire at that horizon; retain
123+
// the wrapper as the maintained input so validation can check agreement.
124+
let (series_source, lookback_ms) = match source.as_ref() {
125+
QueryExpr::TimeRange { range, child } => {
126+
let ms = u64::try_from(range.as_millis()).ok()?;
127+
if ms == 0 || std::time::Duration::from_millis(ms) != *range {
128+
return None;
129+
}
130+
(child.as_ref(), ms)
131+
}
132+
other => (other, 300_000),
127133
};
128134
let QueryExpr::Scan {
129135
source: Source::TimeSeries { metric },
@@ -183,7 +189,7 @@ fn recognize(root: &QueryExpr) -> Option<(MaintainedPopulation, PopulationReadou
183189
matchers,
184190
grouping: labels,
185191
without: grouping.is_without(),
186-
lookback_ms: 300_000,
192+
lookback_ms,
187193
}),
188194
max_k: 0,
189195
quantiles: false,
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
mod support;
2+
use asap_aware_mapping::maintained_population::MaintainedPopulationStrategy;
3+
use asap_types::post_asap::maintained_population::PopulationInput;
4+
use asap_types::post_asap::{SummaryExpr, ValueOperation};
5+
use asap_types::types::AccuracyTarget;
6+
use std::rc::Rc;
7+
8+
// A population for a one-second selector must expire members after one second.
9+
#[test]
10+
fn population_preserves_selector_horizon() {
11+
let root = Rc::new(support::lower_promql("sum(a)", AccuracyTarget::Exact).unwrap());
12+
let candidate = MaintainedPopulationStrategy::new(std::slice::from_ref(&root))
13+
.candidate(&root)
14+
.unwrap();
15+
let SummaryExpr::ValueOperation { child, .. } = &candidate.expr else {
16+
panic!()
17+
};
18+
let SummaryExpr::ValueOperation {
19+
operation: ValueOperation::MaintainPopulation { population },
20+
..
21+
} = &child.expr
22+
else {
23+
panic!()
24+
};
25+
let PopulationInput::CurrentSeries(spec) = &population.input else {
26+
panic!()
27+
};
28+
assert_eq!(spec.lookback_ms, 1_000);
29+
asap_types::post_asap::compile_executable_dag(&candidate).unwrap();
30+
let asap_types::pre_asap::QueryExpr::Aggregate { child: source, .. } = root.as_ref() else {
31+
panic!()
32+
};
33+
assert!(spec.matches_input(source));
34+
let mut wrong = spec.clone();
35+
wrong.lookback_ms = 300_000;
36+
assert!(!wrong.matches_input(source));
37+
}

‎crates/types/src/post_asap/maintained_population.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,18 @@ impl CurrentSeriesInput {
4040
pub fn matches_input(&self, input: &crate::pre_asap::QueryExpr) -> bool {
4141
use crate::pre_asap::{CompareOpKind, DataType, QueryExpr, ScalarValue, Source};
4242
// PromQL instant selectors carry an ingestion-interval `TimeRange` as
43-
// their input scope. Population membership remains the wrapped series
44-
// selector; shifted and otherwise transformed inputs still fail below.
43+
// their input scope. The population must use the same expiry horizon;
44+
// shifted and otherwise transformed inputs still fail below.
4545
let input = match input {
46-
QueryExpr::TimeRange { child, .. } => child.as_ref(),
47-
other => other,
46+
QueryExpr::TimeRange { range, child }
47+
if self.lookback_ms > 0
48+
&& *range == std::time::Duration::from_millis(self.lookback_ms) =>
49+
{
50+
child.as_ref()
51+
}
52+
QueryExpr::TimeRange { .. } => return false,
53+
other if self.lookback_ms == 300_000 => other,
54+
_ => return false,
4855
};
4956
let QueryExpr::Scan {
5057
source: Source::TimeSeries { metric },
@@ -58,7 +65,6 @@ impl CurrentSeriesInput {
5865
|| *metric != self.metric
5966
|| schema.closed
6067
|| schema.time_index.is_none()
61-
|| self.lookback_ms != 300_000
6268
{
6369
return false;
6470
}

0 commit comments

Comments
 (0)