Skip to content

Commit db5b80c

Browse files
zzylolclaude
andcommitted
fix(revisions): keep Planner errors typed and reuse installed programs
Planner execution errors keep their type, so a memory limit during revision admission or window building stays a resource error rather than a 400 input rejection. Revisions use the installed raw programs instead of re-deriving them per request, count buffered samples once however many windows share them, and fail a non-empty window that yields no population. Heaps keep their install-time update check while they use the kernel interpreter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 59c977a commit db5b80c

5 files changed

Lines changed: 98 additions & 45 deletions

File tree

‎data_plane/src/drivers/ingest/prometheus_remote_write.rs‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1736,7 +1736,7 @@ impl PrometheusRemoteWriteReceiver {
17361736
now_ms: u64,
17371737
) -> Result<u64, crate::precompute_engine::revisions::RevisionError> {
17381738
use crate::precompute_engine::revisions::{encode_state, InputSample, RevisionRecord};
1739-
use crate::precompute_engine::{raw_dag::RawDagProgram, window_manager::WindowManager};
1739+
use crate::precompute_engine::window_manager::WindowManager;
17401740
use asap_types::sds::StoredOutputId;
17411741
let (plan, store) = runtime.installed()?;
17421742
let generation = plan
@@ -1825,15 +1825,21 @@ impl PrometheusRemoteWriteReceiver {
18251825
}
18261826
for config in plan.precompute_plan.materializations.iter().filter(|c| c.derived_input.is_none()) {
18271827
let filter = compile_spatial_filter(&config.spatial_filter_normalized)?;
1828-
let program = RawDagProgram::from_plan(&plan.precompute_plan, config)?;
1828+
let program = plan.installed_precompute_plan.raw_programs.get(&config.policy_fp_u64()).ok_or("revision raw program missing")?;
18291829
let mut matching = samples.iter().filter_map(|sample| {
18301830
let labels = sample.labels.iter().map(|(k,v)|(k.clone(),v.clone())).collect();
18311831
(sample.metric == config.metric && filter.matches(&labels)).then_some(())?;
18321832
Some((sample.series.as_str(), sample.timestamp_ms, sample.value?))
18331833
}).collect::<Vec<_>>();
18341834
matching.sort_by(|a, b| (a.0, a.1).cmp(&(b.0, b.1)));
1835-
// Reject the whole request when the Planner graph cannot admit it.
1836-
program.validate(matching, runtime.policy.max_checkpoint_bytes).map_err(|_| crate::precompute_engine::revisions::AdmissionRejected("sample is outside the bound summary kernel's input domain"))?;
1835+
// Reject the whole request when the Planner graph cannot admit its
1836+
// values; resource and setup failures are not input errors.
1837+
if let Err(error) = program.validate(matching, runtime.policy.max_checkpoint_bytes) {
1838+
return Err(match error.downcast_ref::<asap_physical_operators::Error>() {
1839+
Some(asap_physical_operators::Error::MemoryLimit) | None => error,
1840+
Some(_) => crate::precompute_engine::revisions::AdmissionRejected("sample is outside the bound summary kernel's input domain").into(),
1841+
});
1842+
}
18371843
}
18381844
Ok(())
18391845
}, |inputs, revision, captured_at_ms, admitted, publish| {
@@ -1855,19 +1861,18 @@ impl PrometheusRemoteWriteReceiver {
18551861
let WorkerMessage::GroupSamples { sid, policy_fp, group_key, mut samples, .. } = message else { return Err("revision routing returned non-sample input".into()); };
18561862
let config = plan.precompute_plan.materializations.iter().find(|c| c.policy_fingerprint() == policy_fp).ok_or("revision raw configuration missing")?;
18571863
if committed.contains_key(&policy_fp.0) { continue; }
1858-
let program = RawDagProgram::from_plan(&plan.precompute_plan, config)?;
1864+
let program = plan.installed_precompute_plan.raw_programs.get(&config.policy_fp_u64()).ok_or("revision raw program missing")?;
18591865
let manager = WindowManager::with_layout(config.window_size, config.slide_interval, config.pane_origin_ms, &config.window_layout);
18601866
// Each stored window's admitted samples, built by the Planner graph below.
1861-
let mut windows: BTreeMap<(u64,u64), Vec<(String,i64,f64)>> = BTreeMap::new();
1862-
let mut buffered = 0usize;
1867+
// Overlapping windows share each sample's series key.
1868+
let mut windows: BTreeMap<(u64,u64), Vec<(Arc<str>,i64,f64)>> = BTreeMap::new();
18631869
samples.sort_by(|a,b| (&a.0,a.1).cmp(&(&b.0,b.1)));
18641870
for (series,time,value) in samples {
1871+
let series: Arc<str> = Arc::from(series);
18651872
for start in manager.stored_bucket_starts(time) {
18661873
let (start,end) = manager.stored_bucket_bounds(start);
18671874
if start < 0 || end < 0 || end as u64 > captured_at_ms || (start as u64) < captured_at_ms.saturating_sub(retain.saturating_sub(max_window)) { continue; }
1868-
buffered = buffered.saturating_add(series.len() + std::mem::size_of::<(String,i64,f64)>());
1869-
if buffered > runtime.policy.max_checkpoint_bytes { return Err(Box::new(asap_physical_operators::Error::MemoryLimit)); }
1870-
windows.entry((start as u64,end as u64)).or_default().push((series.clone(), time, value));
1875+
windows.entry((start as u64,end as u64)).or_default().push((Arc::clone(&series), time, value));
18711876
}
18721877
}
18731878
// This captured revision contains every accepted local input.
@@ -1896,14 +1901,13 @@ impl PrometheusRemoteWriteReceiver {
18961901
for (window, samples) in windows {
18971902
let state = if samples.is_empty() {
18981903
// A known-empty counter pane of this captured revision.
1899-
Some(program.empty_state()?)
1904+
program.empty_state()?
19001905
} else {
19011906
let bounds = (i64::try_from(window.0)?, i64::try_from(window.1)?);
1902-
program.build(samples.iter().map(|(s,t,v)|(s.as_str(),*t,*v)), bounds, runtime.policy.max_checkpoint_bytes)?
1907+
program.build(samples.iter().map(|(s,t,v)|(s.as_ref(),*t,*v)), bounds, runtime.policy.max_checkpoint_bytes)?
1908+
.ok_or("revision window admitted no population")?
19031909
};
1904-
if let Some(state) = state {
1905-
states.insert(window, Arc::from(state));
1906-
}
1910+
states.insert(window, Arc::from(state));
19071911
}
19081912
for ((start,end),state) in &states {
19091913
records.get_mut(&policy_fp.into()).unwrap().push(RevisionRecord { reference: reference.clone(), group: group.clone(), start_ms:*start, end_ms:*end, payload:encode_state(Arc::clone(state),program.family.clone())? });

‎data_plane/src/precompute_engine/raw_dag.rs‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ use planner_types::post_asap::{
1818
use planner_types::pre_asap::{ColumnRef, QueryExpr, Source};
1919
use std::collections::HashMap;
2020

21+
/// Planner execution errors keep their type (e.g. `MemoryLimit`); setup and
22+
/// binding failures are messages.
23+
pub type BuildError = Box<dyn std::error::Error + Send + Sync>;
24+
2125
/// A validated executable projection; semantics come from the installed node.
2226
/// The retained node ID makes failures attributable to the selected DAG.
2327
#[derive(Debug, Clone)]
@@ -197,6 +201,9 @@ impl RawDagProgram {
197201
grouping: grouping.clone(),
198202
reduction: reduction.clone(),
199203
};
204+
if program.heap() {
205+
program.validate_heap_update()?;
206+
}
200207
if let Some(old) = &selected {
201208
if old.family != program.family
202209
|| old.input != program.input
@@ -233,7 +240,7 @@ impl RawDagProgram {
233240
samples: impl IntoIterator<Item = (&'a str, i64, f64)>,
234241
pane: (i64, i64),
235242
max_bytes: usize,
236-
) -> Result<Option<Box<dyn AggregateCore>>, String> {
243+
) -> Result<Option<Box<dyn AggregateCore>>, BuildError> {
237244
let samples = pane_batch(samples);
238245
if self.heap() {
239246
let mut updater = self.heap_updater()?;
@@ -247,11 +254,9 @@ impl RawDagProgram {
247254
match self.execute(samples, pane, max_bytes)?.as_slice() {
248255
[] => Ok(None),
249256
[row] => match row.as_slice() {
250-
[_, _, Value::Summary { state, .. }] => {
251-
asap_summary_state::physical::from_physical(state.as_ref())
252-
.map(Some)
253-
.map_err(|e| e.to_string())
254-
}
257+
[_, _, Value::Summary { state, .. }] => Ok(Some(
258+
asap_summary_state::physical::from_physical(state.as_ref())?,
259+
)),
255260
_ => Err("raw precompute output is not a population state".into()),
256261
},
257262
_ => Err("one routed group produced several populations".into()),
@@ -263,7 +268,7 @@ impl RawDagProgram {
263268
&self,
264269
samples: impl IntoIterator<Item = (&'a str, i64, f64)>,
265270
max_bytes: usize,
266-
) -> Result<(), String> {
271+
) -> Result<(), BuildError> {
267272
let samples = pane_batch(samples);
268273
if self.heap() {
269274
let updater = self.heap_updater()?;
@@ -293,7 +298,7 @@ impl RawDagProgram {
293298
samples: impl IntoIterator<Item = (&'a str, i64, f64)>,
294299
pane: (i64, i64),
295300
max_bytes: usize,
296-
) -> Result<Vec<Vec<Value>>, String> {
301+
) -> Result<Vec<Vec<Value>>, BuildError> {
297302
use futures::StreamExt;
298303
let schema = precompute::raw_sample_schema();
299304
let rows = samples
@@ -322,7 +327,7 @@ impl RawDagProgram {
322327
},
323328
)
324329
.map_err(|e| e.to_string())?;
325-
futures::executor::block_on(async {
330+
Ok(futures::executor::block_on(async {
326331
let mut stream = graph.execute(program.roots(), context)?.pop().ok_or(
327332
asap_physical_operators::Error::Invalid("missing output".into()),
328333
)?;
@@ -331,8 +336,31 @@ impl RawDagProgram {
331336
rows.extend(batch?.rows().iter().cloned());
332337
}
333338
Ok::<_, asap_physical_operators::Error>(rows)
334-
})
335-
.map_err(|e| e.to_string())
339+
})?)
340+
}
341+
342+
/// Heaps still use the kernel interpreter, so install only updates it evaluates.
343+
fn validate_heap_update(&self) -> Result<(), String> {
344+
if !matches!(
345+
&self.input.weight,
346+
SummaryInputExpr::Column(ColumnRef::SampleValue) | SummaryInputExpr::Constant(_)
347+
) {
348+
return Err("raw heap weight expression is unsupported".into());
349+
}
350+
fn item(expr: &SummaryInputExpr) -> bool {
351+
match expr {
352+
SummaryInputExpr::Column(ColumnRef::Named(_) | ColumnRef::SampleValue) => true,
353+
SummaryInputExpr::Tuple(items) => items.iter().all(item),
354+
SummaryInputExpr::EntityIdentity(
355+
planner_types::post_asap::EntityIdentity::PromqlLabelSet { excluding },
356+
) => excluding.is_empty(),
357+
_ => false,
358+
}
359+
}
360+
if !self.input.item.as_ref().is_some_and(item) {
361+
return Err("raw heap item expression is unsupported".into());
362+
}
363+
self.heap_updater().map(|_| ())
336364
}
337365

338366
fn heap_updater(&self) -> Result<Box<dyn AccumulatorUpdater>, String> {

‎data_plane/src/precompute_engine/revisions.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -592,8 +592,12 @@ impl RevisionRuntime {
592592
.iter()
593593
.filter(|c| c.derived_input.is_none())
594594
{
595-
let program = super::raw_dag::RawDagProgram::from_plan(&plan.precompute_plan, config)?;
596-
let bytes = encode_state(Arc::from(program.empty_state()?), program.family)?;
595+
let program = plan
596+
.installed_precompute_plan
597+
.raw_programs
598+
.get(&config.policy_fp_u64())
599+
.ok_or("revision raw program missing")?;
600+
let bytes = encode_state(Arc::from(program.empty_state()?), program.family.clone())?;
597601
if bytes.len() > self.policy.max_checkpoint_bytes {
598602
return Err(Box::new(asap_physical_operators::Error::MemoryLimit));
599603
}

‎data_plane/src/precompute_engine/worker.rs‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,13 +1596,15 @@ fn build_pane(
15961596
pane: (i64, i64),
15971597
) -> Result<Option<Box<dyn AggregateCore>>, String> {
15981598
if let Some(program) = program {
1599-
return program.build(
1600-
samples
1601-
.iter()
1602-
.map(|(series, time, value)| (series.as_ref(), *time, *value)),
1603-
pane,
1604-
asap_physical_operators::runtime::Limits::default().max_bytes,
1605-
);
1599+
return program
1600+
.build(
1601+
samples
1602+
.iter()
1603+
.map(|(series, time, value)| (series.as_ref(), *time, *value)),
1604+
pane,
1605+
asap_physical_operators::runtime::Limits::default().max_bytes,
1606+
)
1607+
.map_err(|e| e.to_string());
16061608
}
16071609
#[cfg(test)]
16081610
{
@@ -5032,7 +5034,8 @@ mod dag_execution_tests {
50325034
}
50335035
}
50345036

5035-
// Revision admission checks samples with the installed Planner graph.
5037+
// Revision admission checks samples with the installed Planner graph, and
5038+
// keeps a Planner memory limit distinguishable from invalid input.
50365039
#[test]
50375040
fn admission_validates_with_the_planner_graph() {
50385041
let plan = plan("sum_over_time(asap_demo_gauge[5s])");
@@ -5045,9 +5048,21 @@ mod dag_execution_tests {
50455048
assert!(program
50465049
.validate([(series, 1000, 1.0), (series, 2000, 2.0)], 1 << 20)
50475050
.is_ok());
5048-
assert!(program
5051+
let rejected = program
50495052
.validate([(series, 1000, f64::INFINITY)], 1 << 20)
5050-
.is_err());
5053+
.unwrap_err();
5054+
assert!(!matches!(
5055+
rejected.downcast_ref::<asap_physical_operators::Error>(),
5056+
Some(asap_physical_operators::Error::MemoryLimit)
5057+
));
5058+
// A resource limit stays typed, so admission does not report it as bad input.
5059+
let limited = program
5060+
.validate((0..64).map(|t| (series, t * 10, 1.0)), 1)
5061+
.unwrap_err();
5062+
assert!(matches!(
5063+
limited.downcast_ref::<asap_physical_operators::Error>(),
5064+
Some(asap_physical_operators::Error::MemoryLimit)
5065+
));
50515066
}
50525067

50535068
// A raw output installs only with its Planner-compiled precompute graph.

‎data_plane/src/storage_engines/sketch_db/backfill/window_builder.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,13 @@ pub fn build_dag_accumulator(
173173
i64::try_from(window.0).map_err(|_| "backfill window start overflow")?,
174174
i64::try_from(window.1).map_err(|_| "backfill window end overflow")?,
175175
);
176-
program.build(
177-
samples
178-
.iter()
179-
.map(|sample| (sample.labels.as_str(), sample.timestamp_ms, sample.value)),
180-
window,
181-
asap_physical_operators::runtime::Limits::default().max_bytes,
182-
)
176+
program
177+
.build(
178+
samples
179+
.iter()
180+
.map(|sample| (sample.labels.as_str(), sample.timestamp_ms, sample.value)),
181+
window,
182+
asap_physical_operators::runtime::Limits::default().max_bytes,
183+
)
184+
.map_err(|e| e.to_string())
183185
}

0 commit comments

Comments
 (0)