From 24e1f6bd63d1b507cc0f235ecc7be2999f0eebaf Mon Sep 17 00:00:00 2001 From: zz_y Date: Thu, 10 Sep 2026 21:02:35 -0600 Subject: [PATCH 01/29] fix(erp): bound retained runtime feedback globally --- control_plane/src/runtime_samples.rs | 235 ++++++++++++++++++++++++--- 1 file changed, 215 insertions(+), 20 deletions(-) diff --git a/control_plane/src/runtime_samples.rs b/control_plane/src/runtime_samples.rs index e61afb776..3812b6ce9 100644 --- a/control_plane/src/runtime_samples.rs +++ b/control_plane/src/runtime_samples.rs @@ -71,6 +71,7 @@ pub struct RuntimeSamplesStats { pub records_stored: AtomicU64, pub records_evicted: AtomicU64, pub decode_errors: AtomicU64, + pub records_rejected: AtomicU64, } impl RuntimeSamplesStats { @@ -80,6 +81,7 @@ impl RuntimeSamplesStats { records_stored: self.records_stored.load(Ordering::Relaxed), records_evicted: self.records_evicted.load(Ordering::Relaxed), decode_errors: self.decode_errors.load(Ordering::Relaxed), + records_rejected: self.records_rejected.load(Ordering::Relaxed), } } } @@ -90,22 +92,80 @@ pub struct RuntimeSamplesStatsSnapshot { pub records_stored: u64, pub records_evicted: u64, pub decode_errors: u64, + pub records_rejected: u64, } /// Bounded FIFO ring buffer of runtime records, keyed by -/// `(source, sketch, impl)`. Each key gets its own buffer so a -/// chatty source can't starve a quiet one. +/// `(source, sketch, impl)`. Each key has its own record cap. Global key and +/// serialized-byte limits evict whole oldest keys; consumers see missing +/// evidence rather than a partially retained observation. pub struct RuntimeSamplesStore { - buffers: RwLock>>, + buffers: RwLock, per_key_capacity: usize, + max_keys: usize, + max_json_bytes: usize, stats: Arc, } +#[derive(Default)] +struct RuntimeBuffers { + by_key: HashMap>, + insertion_order: VecDeque, + json_bytes: usize, +} + +impl RuntimeBuffers { + fn remove(&mut self, key: &SampleKey) -> usize { + self.insertion_order.retain(|stored| stored != key); + let Some(records) = self.by_key.remove(key) else { + return 0; + }; + self.json_bytes -= records.iter().map(|(_, bytes)| bytes).sum::(); + records.len() + } +} + +// Count serialized bytes without allocating another copy of the observation. +fn encoded_size(record: &RuntimeRecord, limit: usize) -> Option { + struct Counter { + bytes: usize, + limit: usize, + } + impl std::io::Write for Counter { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.bytes = self + .bytes + .checked_add(bytes.len()) + .filter(|size| *size <= self.limit) + .ok_or_else(|| std::io::Error::other("runtime record exceeds byte budget"))?; + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + let mut count = Counter { bytes: 0, limit }; + serde_json::to_writer(&mut count, record).ok()?; + Some(count.bytes) +} + impl RuntimeSamplesStore { pub fn new(per_key_capacity: usize) -> Arc { + Self::with_limits(per_key_capacity, 1024, 16 * 1024 * 1024) + } + + /// Limits serialized retained metadata bytes, not allocator RSS. A zero + /// limit disables retention; rejected latest records invalidate older fits. + pub fn with_limits( + per_key_capacity: usize, + max_keys: usize, + max_json_bytes: usize, + ) -> Arc { Arc::new(Self { - buffers: RwLock::new(HashMap::new()), + buffers: RwLock::new(RuntimeBuffers::default()), per_key_capacity, + max_keys, + max_json_bytes, stats: Arc::new(RuntimeSamplesStats::default()), }) } @@ -120,45 +180,86 @@ impl RuntimeSamplesStore { #[cfg(test)] pub(crate) fn append_for_test(&self, rec: RuntimeRecord) { - self.append(rec) + self.append(rec); } - fn append(&self, rec: RuntimeRecord) { + fn invalidate(&self, key: &SampleKey) { + let removed = self.buffers.write().remove(key); + self.stats + .records_evicted + .fetch_add(removed as u64, Ordering::Relaxed); + } + + fn append(&self, rec: RuntimeRecord) -> bool { let key = SampleKey { source: rec.source.clone(), sketch: rec.sketch.clone(), impl_name: rec.impl_name.clone(), }; - let mut map = self.buffers.write(); - let buf = map - .entry(key) - .or_insert_with(|| VecDeque::with_capacity(self.per_key_capacity)); - if buf.len() >= self.per_key_capacity { - buf.pop_front(); - self.stats.records_evicted.fetch_add(1, Ordering::Relaxed); + let size = encoded_size(&rec, self.max_json_bytes); + if self.per_key_capacity == 0 || self.max_keys == 0 || size.is_none() { + self.invalidate(&key); + self.stats.records_rejected.fetch_add(1, Ordering::Relaxed); + return false; + } + let size = size.unwrap(); + let mut buffers = self.buffers.write(); + if let Some(records) = buffers.by_key.get_mut(&key) { + if records.len() >= self.per_key_capacity { + let (_, bytes) = records.pop_front().unwrap(); + buffers.json_bytes -= bytes; + self.stats.records_evicted.fetch_add(1, Ordering::Relaxed); + } + } + while (buffers.by_key.len() >= self.max_keys && !buffers.by_key.contains_key(&key)) + || buffers.json_bytes.saturating_add(size) > self.max_json_bytes + { + let oldest = buffers + .insertion_order + .front() + .cloned() + .expect("retained keys have an order"); + let removed = buffers.remove(&oldest); + self.stats + .records_evicted + .fetch_add(removed as u64, Ordering::Relaxed); + } + if !buffers.by_key.contains_key(&key) { + buffers.insertion_order.push_back(key.clone()); } - buf.push_back(rec); + buffers + .by_key + .entry(key) + .or_default() + .push_back((rec, size)); + buffers.json_bytes += size; self.stats.records_stored.fetch_add(1, Ordering::Relaxed); + true } /// Peek the latest record for a given key, or `None` if the /// key has never been seen. Used by the replanner / /// decision loop to read freshness signals. pub fn latest(&self, key: &SampleKey) -> Option { - self.buffers.read().get(key).and_then(|b| b.back().cloned()) + self.buffers + .read() + .by_key + .get(key) + .and_then(|b| b.back().map(|(record, _)| record.clone())) } /// Snapshot the full ring for a key. O(n) clone; non-hot-path only. pub fn snapshot(&self, key: &SampleKey) -> Vec { self.buffers .read() + .by_key .get(key) - .map(|b| b.iter().cloned().collect()) + .map(|b| b.iter().map(|(record, _)| record.clone()).collect()) .unwrap_or_default() } pub fn keys(&self) -> Vec { - self.buffers.read().keys().cloned().collect() + self.buffers.read().by_key.keys().cloned().collect() } } @@ -208,7 +309,12 @@ impl RuntimeSamples for RuntimeSamplesService { .stats .decode_errors .fetch_add(1, Ordering::Relaxed); - tracing::warn!(error = %e, "runtime-samples: malformed payload_json, skipping"); + self.store.invalidate(&SampleKey { + source: pb.source.clone(), + sketch: pb.sketch.clone(), + impl_name: pb.impl_name.clone(), + }); + tracing::warn!(error = %e, "runtime-samples: malformed payload_json, invalidating source evidence"); continue; } }; @@ -228,8 +334,9 @@ impl RuntimeSamples for RuntimeSamplesService { schema_version: pb.schema_version, payload, }; - self.store.append(rec); - accepted += 1; + if self.store.append(rec) { + accepted += 1; + } } Ok(tonic::Response::new(PushAck { accepted })) } @@ -305,6 +412,94 @@ mod tests { assert_eq!(snap.decode_errors, 1); } + /// Replanning can create new definition keys forever; retain only a bounded + /// set and expose eviction as missing evidence. + #[tokio::test] + async fn global_key_limit_evicts_whole_oldest_source() { + let store = RuntimeSamplesStore::with_limits(4, 2, 16_384); + let svc = RuntimeSamplesService::new(Arc::clone(&store)); + for source in ["old", "current", "new"] { + let ack = svc + .push(tonic::Request::new(PushBatch { + records: vec![make_pb_record(source, "hll", "runtime", 10.0)], + })) + .await + .unwrap(); + assert_eq!(ack.into_inner().accepted, 1); + } + assert_eq!(store.keys().len(), 2); + assert!(store + .latest(&SampleKey { + source: "old".into(), + sketch: "hll".into(), + impl_name: "runtime".into() + }) + .is_none()); + assert_eq!(store.stats.snapshot().records_evicted, 1); + } + + /// Byte pressure never truncates population fits inside an observation. + #[tokio::test] + async fn byte_budget_and_oversized_latest_invalidate_old_evidence() { + let store = RuntimeSamplesStore::with_limits(10, 10, 600); + let svc = RuntimeSamplesService::new(Arc::clone(&store)); + for source in ["a", "b", "c", "d"] { + svc.push(tonic::Request::new(PushBatch { + records: vec![make_pb_record(source, "hll", "runtime", 10.0)], + })) + .await + .unwrap(); + assert!(store.buffers.read().json_bytes <= 600); + } + assert!(store.stats.snapshot().records_evicted > 0); + let key = SampleKey { + source: "d".into(), + sketch: "hll".into(), + impl_name: "runtime".into(), + }; + assert!(store.latest(&key).is_some()); + let mut oversized = make_pb_record("d", "hll", "runtime", 10.0); + oversized.payload_json = serde_json::json!({"large": "x".repeat(1000)}).to_string(); + let ack = svc + .push(tonic::Request::new(PushBatch { + records: vec![oversized], + })) + .await + .unwrap(); + assert_eq!(ack.into_inner().accepted, 0); + assert!(store.latest(&key).is_none()); + assert_eq!(store.stats.snapshot().records_rejected, 1); + } + + #[tokio::test] + async fn malformed_latest_and_disabled_retention_cannot_leave_old_fit() { + let store = RuntimeSamplesStore::new(2); + let svc = RuntimeSamplesService::new(Arc::clone(&store)); + let mut record = make_pb_record("a", "hll", "runtime", 10.0); + svc.push(tonic::Request::new(PushBatch { + records: vec![record.clone()], + })) + .await + .unwrap(); + record.payload_json = "{".into(); + svc.push(tonic::Request::new(PushBatch { + records: vec![record], + })) + .await + .unwrap(); + assert!(store.keys().is_empty()); + let disabled = RuntimeSamplesStore::with_limits(0, 2, 600); + let service = RuntimeSamplesService::new(Arc::clone(&disabled)); + let ack = service + .push(tonic::Request::new(PushBatch { + records: vec![make_pb_record("a", "hll", "runtime", 10.0)], + })) + .await + .unwrap(); + assert_eq!(ack.into_inner().accepted, 0); + assert!(disabled.keys().is_empty()); + } + #[tokio::test] async fn ring_evicts_oldest_past_capacity() { let store = RuntimeSamplesStore::new(3); From 955ffe8ca3d33650630e27cbddb8fd35f0953a60 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 08:27:21 -0600 Subject: [PATCH 02/29] feat(catalog): identify derived summary inputs --- crates/asap_types/src/aggregation_config.rs | 48 ++- crates/asap_types/src/derived_input.rs | 322 ++++++++++++++++++ crates/asap_types/src/lib.rs | 1 + crates/asap_types/src/policy_fingerprint.rs | 9 +- crates/asap_types/src/precompute_plan.rs | 11 + .../asap_types/src/precompute_plan/catalog.rs | 11 +- crates/asap_types/src/sds.rs | 16 +- crates/asap_types/src/summary_catalog.rs | 57 +++- .../drivers/ingest/prometheus_remote_write.rs | 2 + data_plane/src/drivers/query/servers/http.rs | 1 + .../src/precompute_engine/output_sink.rs | 1 + .../sketch_db/lifecycle/eviction.rs | 1 + .../storage_engines/types/streaming_config.rs | 16 + .../tests/test_utilities/engine_factories.rs | 8 + .../summary-catalog-sds-architecture.md | 21 ++ 15 files changed, 503 insertions(+), 22 deletions(-) create mode 100644 crates/asap_types/src/derived_input.rs diff --git a/crates/asap_types/src/aggregation_config.rs b/crates/asap_types/src/aggregation_config.rs index 3e9f5cab3..8254b65b1 100644 --- a/crates/asap_types/src/aggregation_config.rs +++ b/crates/asap_types/src/aggregation_config.rs @@ -103,6 +103,8 @@ pub struct PrecomputeMaterialization { pub grouping_labels: crate::GroupingProjection, #[serde(default, skip_serializing_if = "Option::is_none")] pub partitioning: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub derived_input: Option, pub aggregated_labels: KeyByLabelNames, pub rollup_labels: KeyByLabelNames, pub original_yaml: String, @@ -200,9 +202,39 @@ impl PrecomputeMaterialization { .saturating_mul(1_000) } + pub fn source_identity(&self) -> crate::sds::DataSourceIdentity { + use crate::sds::DataSourceIdentity; + if let Some(input) = &self.derived_input { + DataSourceIdentity::Derived { + input: input.clone(), + } + } else if let Some(table_ref) = &self.table_name { + DataSourceIdentity::Table { + table_ref: table_ref.clone(), + } + } else { + DataSourceIdentity::TimeSeries { + metric: self.metric.clone(), + } + } + } + pub fn population_filter_canonical(&self) -> Result { + if let Some(input) = &self.derived_input { + input.validate()?; + if self.table_name.is_some() + || self.table_population.is_some() + || self.table_timestamp_column.is_some() + || !self.spatial_filter.is_empty() + { + return Err("derived inputs cannot also declare a raw source/filter".into()); + } + } self.effective_value_projection().validate()?; - if self.value_projection.is_some() && self.table_name.is_none() { + if self.value_projection.is_some() + && self.table_name.is_none() + && self.derived_input.is_none() + { return Err("explicit table value projection requires a table source".into()); } if let Some(column) = &self.table_timestamp_column { @@ -255,6 +287,7 @@ impl PrecomputeMaterialization { parameters, grouping_labels: grouping_labels.into(), partitioning: None, + derived_input: None, aggregated_labels, rollup_labels, original_yaml, @@ -400,6 +433,11 @@ impl PrecomputeMaterialization { table_name, value_column, ); + config.derived_input = data + .get("derived_input") + .filter(|v| !v.is_null()) + .map(|v| serde_json::from_value(v.clone())) + .transpose()?; config.partitioning = data .get("partitioning") .filter(|value| !value.is_null()) @@ -595,6 +633,11 @@ impl PrecomputeMaterialization { table_name, value_column, ); + config.derived_input = aggregation_data + .get("derived_input") + .filter(|v| !v.is_null()) + .map(|v| serde_yaml::from_value(v.clone())) + .transpose()?; config.partitioning = aggregation_data .get("partitioning") .filter(|value| !value.is_null()) @@ -640,6 +683,9 @@ impl SerializableToSink for PrecomputeMaterialization { "metric": self.metric, }); + if let Some(input) = &self.derived_input { + json["derived_input"] = serde_json::json!(input); + } // Only include numAggregatesToRetain if it's Some if let Some(num_aggregates) = self.num_aggregates_to_retain { json["numAggregatesToRetain"] = serde_json::json!(num_aggregates); diff --git a/crates/asap_types/src/derived_input.rs b/crates/asap_types/src/derived_input.rs new file mode 100644 index 000000000..5aa86fbb2 --- /dev/null +++ b/crates/asap_types/src/derived_input.rs @@ -0,0 +1,322 @@ +//! Content identity for a maintenance sub-DAG cut at existing summary inputs. +//! The executable program remains in OwnedPostAsapDag; this is its catalog key. +use std::collections::{BTreeMap, BTreeSet}; + +use planner_types::post_asap::PostAsapNodeId; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; + +use crate::{executable_plan::OwnedPostAsapDag, sds::SummaryDefinitionId}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct DerivedInputIdentity { + pub inputs: BTreeSet, + pub program_sha256: String, +} + +impl DerivedInputIdentity { + pub fn validate(&self) -> Result<(), String> { + if self.inputs.is_empty() + || self.inputs.iter().any(|id| id.fingerprint().is_unset()) + || self.program_sha256.len() != 64 + || !self + .program_sha256 + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + { + return Err("derived input requires summary references and a canonical SHA-256".into()); + } + Ok(()) + } + + /// Hash semantic nodes and edge roles, replacing input frontiers with stable + /// catalog IDs. Query IDs, node numbering, and catalog generations are absent. + pub fn from_dag( + document: &OwnedPostAsapDag, + root: PostAsapNodeId, + frontiers: &BTreeMap, + ) -> Result { + if document.schema_version != crate::executable_plan::OWNED_POST_ASAP_DAG_SCHEMA_VERSION { + return Err("unsupported derived program document version".into()); + } + document.decode()?; + let mut incoming: BTreeMap<_, Vec<_>> = BTreeMap::new(); + for edge in &document.edges { + incoming.entry(edge.consumer).or_default().push(edge); + } + let nodes: BTreeMap<_, _> = document.nodes.iter().map(|n| (n.id, n)).collect(); + if frontiers.contains_key(&root) { + return Err("derived program requires a non-frontier root".into()); + } + let mut hashes = BTreeMap::new(); + let mut visiting = BTreeSet::new(); + let mut inputs = BTreeSet::new(); + let mut stack = vec![(root, false)]; + while let Some((id, finish)) = stack.pop() { + if hashes.contains_key(&id) { + continue; + } + let node = nodes + .get(&id) + .ok_or("derived program references missing node")?; + if let Some(summary) = frontiers.get(&id) { + inputs.insert(*summary); + hashes.insert(id, serde_json::json!({"summary": summary})); + continue; + } + if !finish { + if !visiting.insert(id) { + return Err("derived program has a cycle".into()); + } + stack.push((id, true)); + let edges = incoming + .get(&id) + .ok_or("derived program has an unbound input leaf")?; + for edge in edges { + stack.push((edge.producer, false)); + } + continue; + } + let mut edges = Vec::new(); + for edge in incoming.get(&id).into_iter().flatten() { + let input = hashes + .get(&edge.producer) + .ok_or("derived input was not evaluated")?; + edges.push( + serde_json::to_vec(&serde_json::json!({ + "input": input, "role": edge.role, "schema": edge.intermediate_schema, + "state": edge.data_state, "grouping": edge.grouping, "window": edge.window, + })) + .map_err(|e| e.to_string())?, + ); + } + edges.sort(); + let bytes = serde_json::to_vec(&serde_json::json!({ + "version": 1, "operator": node.operator, "payload": node.payload, + "state": node.output_state, "schema": node.output_schema, + "guarantee": node.guarantee, "inputs": edges, + })) + .map_err(|e| e.to_string())?; + hashes.insert( + id, + serde_json::json!(format!("{:x}", Sha256::digest(bytes))), + ); + visiting.remove(&id); + } + let identity = Self { + inputs, + program_sha256: hashes[&root] + .as_str() + .ok_or("derived root has no semantic hash")? + .into(), + }; + identity.validate()?; + Ok(identity) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::summary_catalog::SummaryCatalog; + use crate::{AggregationType, KeyByLabelNames, PrecomputeMaterialization, WindowKind}; + + fn config() -> PrecomputeMaterialization { + PrecomputeMaterialization::new( + AggregationType::Sum, + String::new(), + Default::default(), + KeyByLabelNames::empty(), + KeyByLabelNames::empty(), + KeyByLabelNames::empty(), + String::new(), + 10, + 10, + WindowKind::Tumbling, + String::new(), + "m".into(), + None, + None, + None, + ) + } + + #[test] + fn derived_source_is_distinct_and_generation_independent() { + let raw = config(); + let raw_id = SummaryDefinitionId::from(raw.policy_fingerprint()); + let mut derived = raw.clone(); + derived.derived_input = Some(DerivedInputIdentity { + inputs: BTreeSet::from([raw_id]), + program_sha256: "a".repeat(64), + }); + assert_ne!(raw.policy_fingerprint(), derived.policy_fingerprint()); + let a = + SummaryCatalog::from_materializations(1, 1, &[raw.clone(), derived.clone()]).unwrap(); + let b = SummaryCatalog::from_materializations(2, 9, &[raw, derived.clone()]).unwrap(); + assert_eq!(a.materializations, b.materializations); + assert_eq!(a.data_descriptors, b.data_descriptors); + let mut renamed = derived.clone(); + renamed.metric = "output_alias".into(); + assert_eq!(renamed.policy_fingerprint(), derived.policy_fingerprint()); + let json = serde_json::to_value(&derived).unwrap(); + let decoded: PrecomputeMaterialization = serde_json::from_value(json).unwrap(); + assert_eq!(decoded.policy_fingerprint(), derived.policy_fingerprint()); + } + + #[test] + fn catalog_rejects_missing_derived_dependencies_and_raw_source_conflicts() { + let mut derived = config(); + derived.derived_input = Some(DerivedInputIdentity { + inputs: BTreeSet::from([SummaryDefinitionId::from(derived.policy_fingerprint())]), + program_sha256: "b".repeat(64), + }); + assert!(SummaryCatalog::from_materializations(1, 1, &[derived.clone()]).is_err()); + derived.table_name = Some("table".into()); + assert!(derived.population_filter_canonical().is_err()); + } + fn program(source: u32, root: u32) -> OwnedPostAsapDag { + use crate::executable_plan::{OwnedPostAsapEdge, OwnedPostAsapNode}; + use planner_types::post_asap::{ + EdgeRole, ExecutableOperator, ExecutionDataState, GroupingEdgeCompatibility, + WindowEdgeCompatibility, + }; + let state = ExecutionDataState::MAINTENANCE_SUMMARY; + OwnedPostAsapDag { + schema_version: 1, + query_id: "query-a".into(), + root: PostAsapNodeId(root), + nodes: [source, root] + .into_iter() + .map(|id| OwnedPostAsapNode { + id: PostAsapNodeId(id), + operator: ExecutableOperator::SummaryMerge, + payload: serde_json::json!({"kind":"summary_merge"}), + output_state: state, + output_schema: serde_json::json!({"fields":[],"time_index":null}), + guarantee: None, + }) + .collect(), + edges: vec![OwnedPostAsapEdge { + producer: PostAsapNodeId(source), + consumer: PostAsapNodeId(root), + role: EdgeRole::Input, + intermediate_schema: serde_json::json!({"fields":[],"time_index":null}), + data_state: state, + grouping: GroupingEdgeCompatibility::Identical, + window: WindowEdgeCompatibility::NotApplicable, + }], + } + } + + #[test] + fn semantic_signature_ignores_node_and_query_numbering_but_not_inputs() { + let source = SummaryDefinitionId::from(config().policy_fingerprint()); + let a = program(1, 2); + let first = DerivedInputIdentity::from_dag( + &a, + a.root, + &BTreeMap::from([(PostAsapNodeId(1), source)]), + ) + .unwrap(); + let mut b = program(900, 42); + b.query_id = "another-query".into(); + b.nodes.reverse(); + let second = DerivedInputIdentity::from_dag( + &b, + b.root, + &BTreeMap::from([(PostAsapNodeId(900), source)]), + ) + .unwrap(); + assert_eq!(first, second); + b.nodes + .iter_mut() + .find(|n| n.id == b.root) + .unwrap() + .output_schema = serde_json::json!({"fields":[],"time_index":0}); + assert_ne!( + first, + DerivedInputIdentity::from_dag( + &b, + b.root, + &BTreeMap::from([(PostAsapNodeId(900), source)]) + ) + .unwrap() + ); + assert!(DerivedInputIdentity::from_dag(&a, a.root, &BTreeMap::new()).is_err()); + let mut unsupported = a.clone(); + unsupported.schema_version = 999; + assert!(DerivedInputIdentity::from_dag( + &unsupported, + unsupported.root, + &BTreeMap::from([(PostAsapNodeId(1), source)]) + ) + .is_err()); + unsupported = a.clone(); + unsupported.nodes[1].payload = serde_json::json!({"kind":"unknown_operator"}); + assert!(DerivedInputIdentity::from_dag( + &unsupported, + unsupported.root, + &BTreeMap::from([(PostAsapNodeId(1), source)]) + ) + .is_err()); + let mut cycle = a.clone(); + cycle.edges[0].producer = cycle.root; + assert!(DerivedInputIdentity::from_dag(&cycle, cycle.root, &BTreeMap::new()).is_err()); + cycle.edges[0].producer = PostAsapNodeId(999); + assert!(DerivedInputIdentity::from_dag(&cycle, cycle.root, &BTreeMap::new()).is_err()); + } + + #[test] + fn catalog_rejects_self_referential_summary() { + use crate::sds::{ + DataDescriptor, DataSourceIdentity, SummaryDescriptor, ValueProjectionIdentity, + }; + let config = config(); + let input = DerivedInputIdentity { + inputs: BTreeSet::from([SummaryDefinitionId::from(config.policy_fingerprint())]), + program_sha256: "f".repeat(64), + }; + let data = DataDescriptor::new_typed( + DataSourceIdentity::Derived { input }, + ValueProjectionIdentity::SampleValue, + "", + Vec::::new(), + "derived", + ); + assert!(SummaryCatalog::build( + 1, + 1, + [( + config.policy_fingerprint(), + SummaryDescriptor::from_config(&config).unwrap(), + data, + config.window_layout + )] + ) + .is_err()); + } + #[test] + fn installation_rejects_derived_inputs_until_immutable_consumer_is_enabled() { + use crate::precompute_plan::{PlanEnvelope, PrecomputePlan}; + let mut config = config(); + config.derived_input = Some(DerivedInputIdentity { + inputs: BTreeSet::from([SummaryDefinitionId::from(config.policy_fingerprint())]), + program_sha256: "c".repeat(64), + }); + let envelope = PlanEnvelope { + plan_id: 1, + plan_version: 1, + generated_at_unix_ms: 0, + activation_unix_ms: 0, + expiry_unix_ms: None, + backend_compat: "asap-query-backend.v1".into(), + planner_revision: "test".into(), + capability_snapshot_id: "test".into(), + }; + let error = + PrecomputePlan::build(envelope, vec![config], &["producer".into()]).unwrap_err(); + assert!(error.to_string().contains("immutable maintenance consumer")); + } +} diff --git a/crates/asap_types/src/lib.rs b/crates/asap_types/src/lib.rs index 3c0a23997..0832dfb12 100644 --- a/crates/asap_types/src/lib.rs +++ b/crates/asap_types/src/lib.rs @@ -2,6 +2,7 @@ pub mod accumulator_spec; pub mod accuracy; pub mod aggregation_config; pub mod aggregation_type; +pub mod derived_input; pub mod enums; pub mod executable_plan; pub mod grouping_projection; diff --git a/crates/asap_types/src/policy_fingerprint.rs b/crates/asap_types/src/policy_fingerprint.rs index f9e751651..46837a95e 100644 --- a/crates/asap_types/src/policy_fingerprint.rs +++ b/crates/asap_types/src/policy_fingerprint.rs @@ -90,7 +90,14 @@ impl PolicyFingerprint { let mut buf: Vec = Vec::with_capacity(512); // 1. metric name - buf.extend_from_slice(cfg.metric.as_bytes()); + if cfg.derived_input.is_some() { + buf.extend_from_slice(b"derived-input-v1:"); + buf.extend_from_slice( + &serde_json::to_vec(&cfg.source_identity()).expect("typed source identity"), + ); + } else { + buf.extend_from_slice(cfg.metric.as_bytes()); + } buf.push(0); // 2. aggregation_type (Serialize impl is the stable form) diff --git a/crates/asap_types/src/precompute_plan.rs b/crates/asap_types/src/precompute_plan.rs index 257d99dd4..da7b07b0c 100644 --- a/crates/asap_types/src/precompute_plan.rs +++ b/crates/asap_types/src/precompute_plan.rs @@ -334,6 +334,17 @@ impl PrecomputePlan { if !valid_ingest { return Err(PrecomputePlanError::UnsupportedIngestEndpoint); } + // Derived input identity is portable, but raw ingress cannot execute it. + // Installation stays closed until the immutable maintenance consumer is wired. + if self + .materializations + .iter() + .any(|config| config.derived_input.is_some()) + { + return Err(PrecomputePlanError::CatalogContract( + "derived summary input requires an immutable maintenance consumer".into(), + )); + } for (query_id, installed) in &self.executable_dags { if query_id != &installed.document.query_id { return Err(PrecomputePlanError::CatalogContract( diff --git a/crates/asap_types/src/precompute_plan/catalog.rs b/crates/asap_types/src/precompute_plan/catalog.rs index 9ee907798..e9dd2a599 100644 --- a/crates/asap_types/src/precompute_plan/catalog.rs +++ b/crates/asap_types/src/precompute_plan/catalog.rs @@ -1,6 +1,6 @@ //! Catalog consistency checks for the precompute execution plan. use super::*; -use crate::sds::{DataSourceIdentity, SummaryDefinitionId, SummaryDescriptor}; +use crate::sds::{SummaryDefinitionId, SummaryDescriptor}; use crate::summary_catalog::SummaryCatalog; use planner_types::pre_asap::Source; use std::collections::BTreeSet; @@ -70,14 +70,7 @@ impl PrecomputePlan { return Err(invalid("pane origin differs from catalog definition")); } let data = &catalog.data_descriptors[&binding.data_descriptor_id]; - let expected_source = config.table_name.as_ref().map_or_else( - || DataSourceIdentity::TimeSeries { - metric: config.metric.clone(), - }, - |table_ref| DataSourceIdentity::Table { - table_ref: table_ref.clone(), - }, - ); + let expected_source = config.source_identity(); if config.table_name.is_some() && !config.grouping_labels.is_empty() && data.observation_semantics diff --git a/crates/asap_types/src/sds.rs b/crates/asap_types/src/sds.rs index 23c3cc48b..b75fd88ca 100644 --- a/crates/asap_types/src/sds.rs +++ b/crates/asap_types/src/sds.rs @@ -729,8 +729,15 @@ impl FidelityGuarantee { #[serde(deny_unknown_fields)] #[serde(rename_all = "snake_case")] pub enum DataSourceIdentity { - TimeSeries { metric: String }, - Table { table_ref: String }, + TimeSeries { + metric: String, + }, + Table { + table_ref: String, + }, + Derived { + input: crate::derived_input::DerivedInputIdentity, + }, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -829,7 +836,7 @@ impl DataDescriptor { pub fn time_series_metric(&self) -> Option<&str> { match &self.source { DataSourceIdentity::TimeSeries { metric } => Some(metric), - DataSourceIdentity::Table { .. } => None, + DataSourceIdentity::Table { .. } | DataSourceIdentity::Derived { .. } => None, } } @@ -929,6 +936,9 @@ impl DataDescriptor { &self.id } pub fn validate(&self) -> Result<(), SdsError> { + if let DataSourceIdentity::Derived { input } = &self.source { + input.validate().map_err(SdsError)?; + } self.value_projection.validate().map_err(SdsError)?; self.group_by_keys.validate().map_err(SdsError)?; if matches!(self.source, DataSourceIdentity::TimeSeries { .. }) diff --git a/crates/asap_types/src/summary_catalog.rs b/crates/asap_types/src/summary_catalog.rs index 5f31ee43d..36468724d 100644 --- a/crates/asap_types/src/summary_catalog.rs +++ b/crates/asap_types/src/summary_catalog.rs @@ -103,14 +103,7 @@ impl SummaryCatalog { .map(|config| { let summary = SummaryDescriptor::from_config(config) .map_err(|error| SummaryCatalogError::Descriptor(error.to_string()))?; - let source = config.table_name.as_ref().map_or_else( - || DataSourceIdentity::TimeSeries { - metric: config.metric.clone(), - }, - |table_ref| DataSourceIdentity::Table { - table_ref: table_ref.clone(), - }, - ); + let source = config.source_identity(); let value_projection = config.effective_value_projection().clone(); let data = DataDescriptor::new_typed( source, @@ -253,6 +246,54 @@ impl SummaryCatalog { return Err(SummaryCatalogError::MissingDescriptor(id.as_u64())); } } + if !self + .data_descriptors + .values() + .any(|data| matches!(data.source, DataSourceIdentity::Derived { .. })) + { + return Ok(()); + } + // Dependencies must refer to this snapshot and form an acyclic graph. + let mut pending = std::collections::BTreeMap::new(); + let mut consumers: std::collections::BTreeMap<_, Vec<_>> = + std::collections::BTreeMap::new(); + for (id, binding) in &self.materializations { + let dependencies = match &self.data_descriptors[&binding.data_descriptor_id].source { + DataSourceIdentity::Derived { input } => input.inputs.clone(), + _ => Default::default(), + }; + for source in &dependencies { + if !self.materializations.contains_key(source) { + return Err(SummaryCatalogError::Descriptor( + "derived input references missing summary".into(), + )); + } + consumers.entry(*source).or_default().push(*id); + } + pending.insert(*id, dependencies.len()); + } + let mut ready: Vec<_> = pending + .iter() + .filter_map(|(id, count)| (*count == 0).then_some(*id)) + .collect(); + let mut visited = 0; + while let Some(id) = ready.pop() { + visited += 1; + for consumer in consumers.get(&id).into_iter().flatten() { + let count = pending + .get_mut(consumer) + .expect("catalog dependency target"); + *count -= 1; + if *count == 0 { + ready.push(*consumer); + } + } + } + if visited != pending.len() { + return Err(SummaryCatalogError::Descriptor( + "derived summary dependencies have a cycle".into(), + )); + } Ok(()) } } diff --git a/data_plane/src/drivers/ingest/prometheus_remote_write.rs b/data_plane/src/drivers/ingest/prometheus_remote_write.rs index c1ec49919..d0aea8b9e 100644 --- a/data_plane/src/drivers/ingest/prometheus_remote_write.rs +++ b/data_plane/src/drivers/ingest/prometheus_remote_write.rs @@ -988,6 +988,7 @@ mod tests { table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -1059,6 +1060,7 @@ mod tests { table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; diff --git a/data_plane/src/drivers/query/servers/http.rs b/data_plane/src/drivers/query/servers/http.rs index c456d8e3c..af0d3a07a 100644 --- a/data_plane/src/drivers/query/servers/http.rs +++ b/data_plane/src/drivers/query/servers/http.rs @@ -3681,6 +3681,7 @@ aggregations: table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; diff --git a/data_plane/src/precompute_engine/output_sink.rs b/data_plane/src/precompute_engine/output_sink.rs index b908f2a8b..46a83a42c 100644 --- a/data_plane/src/precompute_engine/output_sink.rs +++ b/data_plane/src/precompute_engine/output_sink.rs @@ -422,6 +422,7 @@ mod tests { table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, } diff --git a/data_plane/src/storage_engines/sketch_db/lifecycle/eviction.rs b/data_plane/src/storage_engines/sketch_db/lifecycle/eviction.rs index 174c4ae5c..30713e75a 100644 --- a/data_plane/src/storage_engines/sketch_db/lifecycle/eviction.rs +++ b/data_plane/src/storage_engines/sketch_db/lifecycle/eviction.rs @@ -276,6 +276,7 @@ mod tests { table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, } diff --git a/data_plane/src/storage_engines/types/streaming_config.rs b/data_plane/src/storage_engines/types/streaming_config.rs index 91a2dc1b3..2298955db 100644 --- a/data_plane/src/storage_engines/types/streaming_config.rs +++ b/data_plane/src/storage_engines/types/streaming_config.rs @@ -133,6 +133,11 @@ impl StreamingConfig { num_aggregates_to_retain, QueryLanguage::PromQl, )?; + if config.derived_input.is_some() { + anyhow::bail!( + "legacy streaming input cannot execute a derived summary program" + ); + } // PR 5: the map key IS the policy-fingerprint u64. // `AggregationConfig::policy_fp_u64()` is the canonical // accessor for this value. @@ -191,6 +196,17 @@ mod tests { assert_eq!(cfg.storage_backend(), StorageBackend::GorillaObjectStore); } + #[test] + fn legacy_yaml_rejects_derived_summary_input() { + let data = serde_yaml::from_str::(&format!( + "aggregations:\n- aggregationType: Sum\n aggregationSubType: ''\n metric: outer\n labels: {{grouping: [], rollup: [], aggregated: []}}\n parameters: {{}}\n windowSize: 10\n windowType: tumbling\n spatialFilter: ''\n derived_input:\n inputs: [1]\n program_sha256: '{}'\n", "a".repeat(64) + )).unwrap(); + let error = StreamingConfig::from_yaml_data(&data).unwrap_err(); + assert!(error + .to_string() + .contains("legacy streaming input cannot execute")); + } + /// PR 5: a streaming-config YAML that omits `aggregationId` /// parses correctly — the backend derives identity from content /// via `PolicyFingerprint::from_config`. The map key is the diff --git a/data_plane/src/tests/test_utilities/engine_factories.rs b/data_plane/src/tests/test_utilities/engine_factories.rs index 3fce2d44b..564d21d7c 100644 --- a/data_plane/src/tests/test_utilities/engine_factories.rs +++ b/data_plane/src/tests/test_utilities/engine_factories.rs @@ -109,6 +109,7 @@ pub fn create_engine_single_pop_with_aggregated( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -206,6 +207,7 @@ pub fn create_engine_dual_input( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -233,6 +235,7 @@ pub fn create_engine_dual_input( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -325,6 +328,7 @@ pub fn create_engine_two_metrics( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -351,6 +355,7 @@ pub fn create_engine_two_metrics( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -453,6 +458,7 @@ pub fn create_engine_three_metrics( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -532,6 +538,7 @@ pub fn create_engine_multi_timestamp( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; @@ -603,6 +610,7 @@ pub fn create_engine_multi_timestamp_with_window( table_name: None, value_projection: None, table_population: None, + derived_input: None, table_timestamp_column: None, partitioning: None, }; diff --git a/docs/design_docs/summary-catalog-sds-architecture.md b/docs/design_docs/summary-catalog-sds-architecture.md index 048dcbac4..39956f7fe 100644 --- a/docs/design_docs/summary-catalog-sds-architecture.md +++ b/docs/design_docs/summary-catalog-sds-architecture.md @@ -429,3 +429,24 @@ This is an explicit lifetime transition, not cross-generation recovery of arbitr summary state. Legacy records without trustworthy catalog provenance remain unbound. Tombstone reclamation still requires coordinated removal of old physical parts and is not implemented by this transition. + +### Derived summary input identity + +A summary computed from another summary has a different data source from the +original raw table or metric. `PrecomputeMaterialization.derived_input` and +`DataSourceIdentity::Derived` use the same `DerivedInputIdentity`: the referenced +`SummaryDefinitionId`s and a SHA-256 of the maintenance program. The executable +program remains in `OwnedPostAsapDag`; the catalog does not retain another copy. + +The signature replaces materialized input frontiers with stable summary IDs and +hashes the remaining node payloads, schemas, guarantees, and edge semantics. It +excludes query names, plan-local node numbering, and catalog generations. A changed +input definition or transformation creates a new identity. Existing raw-source +identities retain their previous byte representation. Catalog validation rejects +missing input definitions and dependency cycles. + +This contract is a prerequisite, not enabled summary-over-summary execution. +Installation currently rejects derived inputs so they cannot accidentally receive +raw samples through the legacy metric router. Enabling them requires the immutable +maintenance consumer and durable output deduplication protocol; neither raw-table +substitution nor treating late correction fragments as new observations is valid. From bcbb5a40129493e6b724c18916979704a4028537 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 08:44:47 -0600 Subject: [PATCH 03/29] fix(catalog): separate derived policy domain from raw metric names --- crates/asap_types/src/derived_input.rs | 71 +++++++++++++++++-- crates/asap_types/src/policy_fingerprint.rs | 4 +- .../summary-catalog-sds-architecture.md | 3 +- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/crates/asap_types/src/derived_input.rs b/crates/asap_types/src/derived_input.rs index 5aa86fbb2..a5f6d6dce 100644 --- a/crates/asap_types/src/derived_input.rs +++ b/crates/asap_types/src/derived_input.rs @@ -40,7 +40,20 @@ impl DerivedInputIdentity { if document.schema_version != crate::executable_plan::OWNED_POST_ASAP_DAG_SCHEMA_VERSION { return Err("unsupported derived program document version".into()); } - document.decode()?; + let decoded = document.decode()?; + let literals: BTreeSet<_> = decoded + .nodes + .iter() + .filter_map(|node| { + matches!( + &node.payload, + planner_types::post_asap::ExecutableOperatorPayload::Fallback { + expression: planner_types::pre_asap::QueryExpr::Literal(_), + } + ) + .then_some(node.id) + }) + .collect(); let mut incoming: BTreeMap<_, Vec<_>> = BTreeMap::new(); for edge in &document.edges { incoming.entry(edge.consumer).or_default().push(edge); @@ -70,10 +83,10 @@ impl DerivedInputIdentity { return Err("derived program has a cycle".into()); } stack.push((id, true)); - let edges = incoming - .get(&id) - .ok_or("derived program has an unbound input leaf")?; - for edge in edges { + if !incoming.contains_key(&id) && !literals.contains(&id) { + return Err("derived program has an unbound input leaf".into()); + } + for edge in incoming.get(&id).into_iter().flatten() { stack.push((edge.producer, false)); } continue; @@ -165,6 +178,22 @@ mod tests { assert_eq!(decoded.policy_fingerprint(), derived.policy_fingerprint()); } + #[test] + fn raw_utf8_metric_cannot_impersonate_derived_policy_domain() { + let mut derived = config(); + derived.derived_input = Some(DerivedInputIdentity { + inputs: BTreeSet::from([SummaryDefinitionId::from(derived.policy_fingerprint())]), + program_sha256: "d".repeat(64), + }); + let mut raw = derived.clone(); + raw.metric = format!( + "derived-input-v1:{}", + serde_json::to_string(&derived.source_identity()).unwrap() + ); + raw.derived_input = None; + assert_ne!(raw.policy_fingerprint(), derived.policy_fingerprint()); + } + #[test] fn catalog_rejects_missing_derived_dependencies_and_raw_source_conflicts() { let mut derived = config(); @@ -319,4 +348,36 @@ mod tests { PrecomputePlan::build(envelope, vec![config], &["producer".into()]).unwrap_err(); assert!(error.to_string().contains("immutable maintenance consumer")); } + #[test] + fn literal_leaves_are_hashed_without_inventing_materialization_references() { + use planner_types::{ + post_asap::{ExecutableOperator, ExecutableOperatorPayload}, + pre_asap::{QueryExpr, ScalarValue}, + }; + let mut dag = program(1, 2); + let mut literal = dag.nodes[0].clone(); + literal.id = PostAsapNodeId(3); + literal.operator = ExecutableOperator::Fallback; + literal.payload = serde_json::to_value(ExecutableOperatorPayload::Fallback { + expression: QueryExpr::Literal(ScalarValue::Int64(2)), + }) + .unwrap(); + dag.nodes.push(literal); + let mut edge = dag.edges[0].clone(); + edge.producer = PostAsapNodeId(3); + dag.edges.push(edge); + let source = SummaryDefinitionId::from(config().policy_fingerprint()); + let frontiers = BTreeMap::from([(PostAsapNodeId(1), source)]); + let first = DerivedInputIdentity::from_dag(&dag, dag.root, &frontiers).unwrap(); + assert_eq!(first.inputs, BTreeSet::from([source])); + dag.nodes[2].payload = serde_json::to_value(ExecutableOperatorPayload::Fallback { + expression: QueryExpr::Literal(ScalarValue::Int64(3)), + }) + .unwrap(); + assert_ne!( + first, + DerivedInputIdentity::from_dag(&dag, dag.root, &frontiers).unwrap() + ); + assert!(DerivedInputIdentity::from_dag(&dag, PostAsapNodeId(3), &frontiers).is_err()); + } } diff --git a/crates/asap_types/src/policy_fingerprint.rs b/crates/asap_types/src/policy_fingerprint.rs index 46837a95e..38afc45ff 100644 --- a/crates/asap_types/src/policy_fingerprint.rs +++ b/crates/asap_types/src/policy_fingerprint.rs @@ -91,7 +91,9 @@ impl PolicyFingerprint { // 1. metric name if cfg.derived_input.is_some() { - buf.extend_from_slice(b"derived-input-v1:"); + // Raw policies start with UTF-8 metric bytes; 0xff is impossible + // there, so a metric cannot impersonate this source domain. + buf.extend_from_slice(b"\xffderived-input-v1:"); buf.extend_from_slice( &serde_json::to_vec(&cfg.source_identity()).expect("typed source identity"), ); diff --git a/docs/design_docs/summary-catalog-sds-architecture.md b/docs/design_docs/summary-catalog-sds-architecture.md index 39956f7fe..4196f079a 100644 --- a/docs/design_docs/summary-catalog-sds-architecture.md +++ b/docs/design_docs/summary-catalog-sds-architecture.md @@ -440,7 +440,8 @@ program remains in `OwnedPostAsapDag`; the catalog does not retain another copy. The signature replaces materialized input frontiers with stable summary IDs and hashes the remaining node payloads, schemas, guarantees, and edge semantics. It -excludes query names, plan-local node numbering, and catalog generations. A changed +excludes query names, plan-local node numbering, and catalog generations. Literal +leaves are hashed directly; raw input leaves still require catalog frontiers. A changed input definition or transformation creates a new identity. Existing raw-source identities retain their previous byte representation. Catalog validation rejects missing input definitions and dependency cycles. From cf943d463f2ee431ee68dc3aae6e7d6fb08dd8db Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:06:42 -0600 Subject: [PATCH 04/29] feat(clickhouse): execute typed array element access --- Cargo.lock | 10 +- control_plane/Cargo.toml | 8 +- .../src/query_plan/clickhouse_exact.rs | 24 ++- crates/asap_types/Cargo.toml | 2 +- data_plane/Cargo.toml | 6 +- .../relational_adapter.rs | 194 +++++++++++++++++- 6 files changed, 221 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b68bc67f..c28242004 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=0402384e589df6e087d6d2d22b463ddc2eea0774#0402384e589df6e087d6d2d22b463ddc2eea0774" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" 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=0402384e589df6e087d6d2d22b463ddc2eea0774#0402384e589df6e087d6d2d22b463ddc2eea0774" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" dependencies = [ "asap-types", "promql-parser", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=0402384e589df6e087d6d2d22b463ddc2eea0774#0402384e589df6e087d6d2d22b463ddc2eea0774" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -415,12 +415,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=0402384e589df6e087d6d2d22b463ddc2eea0774#0402384e589df6e087d6d2d22b463ddc2eea0774" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=0402384e589df6e087d6d2d22b463ddc2eea0774#0402384e589df6e087d6d2d22b463ddc2eea0774" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" dependencies = [ "serde", "serde_json", diff --git a/control_plane/Cargo.toml b/control_plane/Cargo.toml index 7cb29606b..264534341 100644 --- a/control_plane/Cargo.toml +++ b/control_plane/Cargo.toml @@ -76,8 +76,8 @@ asap_types.workspace = true # scaffolding, unaware that `data_plane`'s `summary_executor.rs` in *this* # repo is a real one. Vendored locally instead of chased upstream -- see # `data_plane/src/query_engines/asap_query_engine/summary_exec.rs`. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } -asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } # L1 adoption (design-target-architecture.md Part B): the PromQL front # end itself, replacing control_plane's own query_parser/promql.rs. @@ -85,8 +85,8 @@ asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = # `planner-types`/`asap-aware-mapping` above -- these three MUST move # together (two revs of the same upstream repo's types in one workspace # resolve to distinct Rust types that won't unify). -asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } -asap-frontend-sql = { git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } +asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +asap-frontend-sql = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } [dev-dependencies] tokio = { version = "1", features = ["full", "test-util"] } diff --git a/control_plane/src/query_plan/clickhouse_exact.rs b/control_plane/src/query_plan/clickhouse_exact.rs index 9c8455429..654cc80c8 100644 --- a/control_plane/src/query_plan/clickhouse_exact.rs +++ b/control_plane/src/query_plan/clickhouse_exact.rs @@ -77,7 +77,7 @@ fn scalar(expr: &QueryExpr, schema: &Schema) -> Result { let function = match name.as_str() { "map" => "map", "mapconcat" => "mapConcat", - "asap_map_access" => "arrayElement", + "asap_map_access" | "asap_element_access" => "arrayElement", _ => return Err(format!("unsupported exact scalar function {name}")), }; expr.scalar_type(schema).map_err(|e| e.to_string())?; @@ -301,6 +301,28 @@ mod tests { assert!(!sql.contains("{from:")); assert!(!sql.contains("{to:")); } + #[test] + fn typed_list_access_renders_native_element_lookup() { + let schema = Schema::new(vec![Column::new( + "samples", + DataType::List { + element: Box::new(Column::new("item", DataType::Float64, false)), + }, + false, + )]); + let expr = QueryExpr::FunctionCall { + name: "asap_element_access".into(), + args: vec![ + QueryExpr::Column(0), + QueryExpr::Literal(ScalarValue::Int64(-1)), + ], + }; + assert_eq!( + scalar(&expr, &schema).unwrap(), + "arrayElement(`samples`, -1)" + ); + } + #[test] fn unsupported_scalar_is_not_forwarded_as_arbitrary_native_code() { let schema = Schema::new(vec![]); diff --git a/crates/asap_types/Cargo.toml b/crates/asap_types/Cargo.toml index a83ea156c..63c84655d 100644 --- a/crates/asap_types/Cargo.toml +++ b/crates/asap_types/Cargo.toml @@ -33,4 +33,4 @@ sha2 = "0.10" # exactly (`control_plane/Cargo.toml`) -- two different revs of the same # git dependency in one workspace resolve to two distinct Rust types that # won't unify. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } diff --git a/data_plane/Cargo.toml b/data_plane/Cargo.toml index 60ae2ef00..cc5310495 100644 --- a/data_plane/Cargo.toml +++ b/data_plane/Cargo.toml @@ -39,8 +39,8 @@ sha2 = "0.10" # reduction: Reduction, .. }`) are `pre_asap` types, in the same crate now # (not a separate `asap-ir` import). Query serving consumes the compiled # QueryPlan; these types are used at physical-plan compilation boundaries. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } -asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } # Shared external (workspace) serde.workspace = true @@ -133,7 +133,7 @@ fs2 = "0.4" # none of them. [dev-dependencies] -asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "0402384e589df6e087d6d2d22b463ddc2eea0774" } +asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } tempfile = "3.20.0" criterion = { version = "0.5", features = ["html_reports"] } tokio-tungstenite = "0.21" diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs index 15945be21..041961307 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs @@ -41,6 +41,7 @@ enum Cell { Bool(bool), Timestamp(i64), Map(Vec<(Cell, Cell)>), + List(Arc<[Cell]>), } fn json_cell( @@ -60,9 +61,23 @@ fn json_cell( match dtype { DataType::Null if value.is_null() => Ok(Cell::Null), DataType::Null => Err(invalid()), - DataType::List { .. } | DataType::Struct { .. } => Err( - ClickHouseRelationalError::Unsupported("collection value transport".into()), - ), + DataType::List { element } => { + let item_type = clickhouse_type + .strip_prefix("Array(") + .and_then(|inner| inner.strip_suffix(')')) + .ok_or_else(invalid)?; + let items = value.as_array().ok_or_else(invalid)?; + Ok(Cell::List( + items + .iter() + .map(|item| json_cell(item, &element.dtype, element.nullable, item_type)) + .collect::, _>>()? + .into(), + )) + } + DataType::Struct { .. } => Err(ClickHouseRelationalError::Unsupported( + "struct value transport".into(), + )), DataType::Int64 => value.as_i64().map(Cell::Int64).ok_or_else(invalid), DataType::Float64 => value.as_f64().map(Cell::Float64).ok_or_else(invalid), DataType::Utf8 => value @@ -307,7 +322,16 @@ fn clickhouse_type_matches(actual: Option<&str>, expected: &DataType, nullable: } match expected { DataType::Null => actual == "Nothing", - DataType::List { .. } | DataType::Struct { .. } => false, + DataType::List { element } => { + !nullable + && actual + .strip_prefix("Array(") + .and_then(|inner| inner.strip_suffix(')')) + .is_some_and(|inner| { + clickhouse_type_matches(Some(inner), &element.dtype, element.nullable) + }) + } + DataType::Struct { .. } => false, DataType::Int64 => actual == "Int64", DataType::Float64 => actual == "Float64", DataType::Utf8 => actual == "String", @@ -428,11 +452,17 @@ impl ClickHouseRelationalAdapter { } for row in &input.rows { for key in keys { - if contains_nan(&eval(&key.expr, row, &schema)?) { + let value = eval(&key.expr, row, &schema)?; + if contains_nan(&value) { return Err(ClickHouseRelationalError::Unsupported( "NaN sort key".into(), )); } + if !matches!(value, Cell::Null) && cell_cmp(&value, &value).is_none() { + return Err(ClickHouseRelationalError::Unsupported( + "unsupported sort key value type".into(), + )); + } } } input @@ -548,7 +578,50 @@ fn eval( } QueryExpr::FunctionCall { name, args } => { use planner_types::pre_asap::scalar_signature::MapScalarFunction; - let function = MapScalarFunction::from_name(name).ok_or_else(|| { + if name.eq_ignore_ascii_case("asap_element_access") { + let (output_type, _) = expr + .scalar_type(schema) + .map_err(|error| ClickHouseRelationalError::Invalid(error.to_string()))?; + if let DataType::List { element } = args[0] + .scalar_type(schema) + .map_err(|error| ClickHouseRelationalError::Invalid(error.to_string()))? + .0 + { + let Cell::List(values) = eval(&args[0], row, schema)? else { + return Err(ClickHouseRelationalError::Invalid( + "array access input".into(), + )); + }; + let index = match eval(&args[1], row, schema)? { + Cell::Null => return Ok(Cell::Null), + Cell::Int64(index) => index, + _ => { + return Err(ClickHouseRelationalError::Invalid( + "array access index".into(), + )) + } + }; + let offset = if index > 0 { + usize::try_from(index - 1).ok() + } else if index < 0 { + usize::try_from(index.unsigned_abs()) + .ok() + .and_then(|distance| values.len().checked_sub(distance)) + } else { + None + }; + return match offset.and_then(|offset| values.get(offset)) { + Some(value) => Ok(value.clone()), + None => default_collection_element(&output_type, element.nullable), + }; + } + } + let function = (if name.eq_ignore_ascii_case("asap_element_access") { + Some(MapScalarFunction::Access) + } else { + MapScalarFunction::from_name(name) + }) + .ok_or_else(|| { ClickHouseRelationalError::Unsupported(format!("scalar function {name}")) })?; expr.scalar_type(schema) @@ -619,7 +692,7 @@ fn eval( else { unreachable!() }; - default_map_value(&value, value_nullable) + default_collection_element(&value, value_nullable) } } } @@ -629,7 +702,7 @@ fn eval( } } -fn default_map_value(dtype: &DataType, nullable: bool) -> Result { +fn default_collection_element(dtype: &DataType, nullable: bool) -> Result { if nullable { return Ok(Cell::Null); } @@ -640,9 +713,10 @@ fn default_map_value(dtype: &DataType, nullable: bool) -> Result Cell::Utf8(String::new()), DataType::Bool => Cell::Bool(false), DataType::Map { .. } => Cell::Map(Vec::new()), + DataType::List { .. } => Cell::List(Arc::from([])), _ => { return Err(ClickHouseRelationalError::Unsupported( - "map missing-key default type".into(), + "collection missing-element default type".into(), )) } }) @@ -768,6 +842,7 @@ fn compare_sort_keys( fn contains_nan(value: &Cell) -> bool { match value { Cell::Float64(value) => value.is_nan(), + Cell::List(values) => values.iter().any(contains_nan), Cell::Map(entries) => entries .iter() .any(|(key, value)| contains_nan(key) || contains_nan(value)), @@ -974,6 +1049,107 @@ mod tests { }; use std::rc::Rc; + #[test] + fn decodes_declared_array_elements_without_losing_nullability() { + use planner_types::pre_asap::Column; + let dtype = DataType::List { + element: Box::new(Column { + name: "item".into(), + dtype: DataType::Int64, + nullable: true, + table: None, + }), + }; + assert!(clickhouse_type_matches( + Some("Array(Nullable(Int64))"), + &dtype, + false + )); + assert!(!clickhouse_type_matches( + Some("Array(Int64)"), + &dtype, + false + )); + assert!(!clickhouse_type_matches( + Some("Nullable(Array(Nullable(Int64)))"), + &dtype, + true + )); + let value = json_cell( + &serde_json::json!([9007199254740993_i64, null, -7]), + &dtype, + false, + "Array(Nullable(Int64))", + ) + .unwrap(); + let Cell::List(items) = &value else { + panic!("expected list") + }; + assert_eq!( + items.as_ref(), + &[Cell::Int64(9007199254740993), Cell::Null, Cell::Int64(-7)] + ); + let Cell::List(copy) = value.clone() else { + unreachable!() + }; + assert!(Arc::ptr_eq(items, ©)); + assert!(json_cell( + &serde_json::json!(["wrong"]), + &dtype, + false, + "Array(Nullable(Int64))" + ) + .is_err()); + } + + #[test] + fn array_access_uses_signed_indices_and_element_defaults() { + use planner_types::pre_asap::{Column, Schema}; + let function = |name: &str, args| QueryExpr::FunctionCall { + name: name.into(), + args, + }; + let dtype = DataType::List { + element: Box::new(Column::new("item", DataType::Int64, false)), + }; + let schema = Schema::new(vec![ + Column::new("items", dtype, false), + Column::new("index", DataType::Int64, true), + ]); + let access = function( + "asap_element_access", + vec![QueryExpr::Column(0), QueryExpr::Column(1)], + ); + let items = Cell::List(vec![Cell::Int64(10), Cell::Int64(20)].into()); + for (index, expected) in [ + (1, 10), + (2, 20), + (-1, 20), + (-2, 10), + (0, 0), + (3, 0), + (i64::MIN, 0), + (i64::MAX, 0), + ] { + assert_eq!( + eval(&access, &[items.clone(), Cell::Int64(index)], &schema).unwrap(), + Cell::Int64(expected) + ); + } + assert_eq!( + eval(&access, &[items, Cell::Null], &schema).unwrap(), + Cell::Null + ); + let zero = function( + "asap_element_access", + vec![ + QueryExpr::Column(0), + QueryExpr::Literal(ScalarValue::Int64(0)), + ], + ); + assert!(eval(&zero, &[Cell::List(Arc::from([])), Cell::Int64(0)], &schema).is_err()); + } + fn schema(fields: &[(&str, DataType)]) -> SummarySchema { SummarySchema { fields: fields From 32562405af8c62a7101f8a880618f3cf8ce686b6 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:10:56 -0600 Subject: [PATCH 05/29] style(clickhouse): format collection default helper --- .../asap_clickhouse_query_engine/relational_adapter.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs index 041961307..072c230a2 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs @@ -702,7 +702,10 @@ fn eval( } } -fn default_collection_element(dtype: &DataType, nullable: bool) -> Result { +fn default_collection_element( + dtype: &DataType, + nullable: bool, +) -> Result { if nullable { return Ok(Cell::Null); } From f6c59bfd798cf860c2e219be0e4140a18fdb3c7c Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:11:15 -0600 Subject: [PATCH 06/29] feat(clickhouse): read typed tuple fields inside query DAGs --- .../src/query_plan/clickhouse_exact.rs | 1 + .../relational_adapter.rs | 130 +++++++++++++++++- .../relational_adapter/collection.rs | 120 ++++++++++++++++ 3 files changed, 245 insertions(+), 6 deletions(-) create mode 100644 data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs diff --git a/control_plane/src/query_plan/clickhouse_exact.rs b/control_plane/src/query_plan/clickhouse_exact.rs index 654cc80c8..786943cec 100644 --- a/control_plane/src/query_plan/clickhouse_exact.rs +++ b/control_plane/src/query_plan/clickhouse_exact.rs @@ -78,6 +78,7 @@ fn scalar(expr: &QueryExpr, schema: &Schema) -> Result { "map" => "map", "mapconcat" => "mapConcat", "asap_map_access" | "asap_element_access" => "arrayElement", + "asap_struct_field" => "tupleElement", _ => return Err(format!("unsupported exact scalar function {name}")), }; expr.scalar_type(schema).map_err(|e| e.to_string())?; diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs index 041961307..04de311d6 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs @@ -1,6 +1,7 @@ //! ClickHouse row semantics for planner-owned relational wrappers. mod aggregate; +mod collection; use std::{cmp::Ordering, collections::BTreeMap, sync::Arc}; @@ -42,6 +43,7 @@ enum Cell { Timestamp(i64), Map(Vec<(Cell, Cell)>), List(Arc<[Cell]>), + Struct(Arc<[Cell]>), } fn json_cell( @@ -75,9 +77,25 @@ fn json_cell( .into(), )) } - DataType::Struct { .. } => Err(ClickHouseRelationalError::Unsupported( - "struct value transport".into(), - )), + DataType::Struct { fields } => { + let types = + collection::tuple_field_types(clickhouse_type, fields).ok_or_else(invalid)?; + let items = value + .as_array() + .filter(|items| items.len() == fields.len()) + .ok_or_else(invalid)?; + Ok(Cell::Struct( + items + .iter() + .zip(fields) + .zip(types) + .map(|((item, field), native)| { + json_cell(item, &field.dtype, field.nullable, native) + }) + .collect::, _>>()? + .into(), + )) + } DataType::Int64 => value.as_i64().map(Cell::Int64).ok_or_else(invalid), DataType::Float64 => value.as_f64().map(Cell::Float64).ok_or_else(invalid), DataType::Utf8 => value @@ -331,7 +349,9 @@ fn clickhouse_type_matches(actual: Option<&str>, expected: &DataType, nullable: clickhouse_type_matches(Some(inner), &element.dtype, element.nullable) }) } - DataType::Struct { .. } => false, + DataType::Struct { fields } => { + !nullable && collection::tuple_field_types(actual, fields).is_some() + } DataType::Int64 => actual == "Int64", DataType::Float64 => actual == "Float64", DataType::Utf8 => actual == "String", @@ -578,6 +598,37 @@ fn eval( } QueryExpr::FunctionCall { name, args } => { use planner_types::pre_asap::scalar_signature::MapScalarFunction; + if name.eq_ignore_ascii_case("asap_struct_field") { + expr.scalar_type(schema) + .map_err(|error| ClickHouseRelationalError::Invalid(error.to_string()))?; + let DataType::Struct { fields } = args[0] + .scalar_type(schema) + .map_err(|error| ClickHouseRelationalError::Invalid(error.to_string()))? + .0 + else { + unreachable!() + }; + let offset = match &args[1] { + QueryExpr::Literal(ScalarValue::Int64(index)) => { + usize::try_from(index - 1).ok() + } + QueryExpr::Literal(ScalarValue::Utf8(name)) => { + fields.iter().position(|field| &field.name == name) + } + _ => None, + } + .ok_or_else(|| { + ClickHouseRelationalError::Invalid("struct field selector".into()) + })?; + let Cell::Struct(values) = eval(&args[0], row, schema)? else { + return Err(ClickHouseRelationalError::Invalid( + "struct field input".into(), + )); + }; + return values.get(offset).cloned().ok_or_else(|| { + ClickHouseRelationalError::Invalid("struct field value".into()) + }); + } if name.eq_ignore_ascii_case("asap_element_access") { let (output_type, _) = expr .scalar_type(schema) @@ -702,7 +753,10 @@ fn eval( } } -fn default_collection_element(dtype: &DataType, nullable: bool) -> Result { +fn default_collection_element( + dtype: &DataType, + nullable: bool, +) -> Result { if nullable { return Ok(Cell::Null); } @@ -714,6 +768,13 @@ fn default_collection_element(dtype: &DataType, nullable: bool) -> Result Cell::Bool(false), DataType::Map { .. } => Cell::Map(Vec::new()), DataType::List { .. } => Cell::List(Arc::from([])), + DataType::Struct { fields } => Cell::Struct( + fields + .iter() + .map(|field| default_collection_element(&field.dtype, field.nullable)) + .collect::, _>>()? + .into(), + ), _ => { return Err(ClickHouseRelationalError::Unsupported( "collection missing-element default type".into(), @@ -842,7 +903,7 @@ fn compare_sort_keys( fn contains_nan(value: &Cell) -> bool { match value { Cell::Float64(value) => value.is_nan(), - Cell::List(values) => values.iter().any(contains_nan), + Cell::List(values) | Cell::Struct(values) => values.iter().any(contains_nan), Cell::Map(entries) => entries .iter() .any(|(key, value)| contains_nan(key) || contains_nan(value)), @@ -1150,6 +1211,63 @@ mod tests { assert!(eval(&zero, &[Cell::List(Arc::from([])), Cell::Int64(0)], &schema).is_err()); } + #[test] + fn nested_array_tuple_access_preserves_fields_and_defaults() { + use planner_types::pre_asap::{Column, Schema}; + let tuple = DataType::Struct { + fields: vec![ + Column::new("ts", DataType::Int64, false), + Column::new("value", DataType::Float64, true), + ], + }; + let dtype = DataType::List { + element: Box::new(Column::new("item", tuple, false)), + }; + let native = "Array(Tuple(ts Int64, value Nullable(Float64)))"; + assert!(clickhouse_type_matches(Some(native), &dtype, false)); + let samples = json_cell( + &serde_json::json!([[9007199254740993_i64, 2.5], [7, null]]), + &dtype, + false, + native, + ) + .unwrap(); + let schema = Schema::new(vec![Column::new("samples", dtype, false)]); + let field = |index, name: &str| QueryExpr::FunctionCall { + name: "asap_struct_field".into(), + args: vec![ + QueryExpr::FunctionCall { + name: "asap_element_access".into(), + args: vec![ + QueryExpr::Column(0), + QueryExpr::Literal(ScalarValue::Int64(index)), + ], + }, + QueryExpr::Literal(ScalarValue::Utf8(name.into())), + ], + }; + assert_eq!( + eval(&field(1, "ts"), std::slice::from_ref(&samples), &schema).unwrap(), + Cell::Int64(9007199254740993) + ); + assert_eq!( + eval(&field(1, "value"), std::slice::from_ref(&samples), &schema).unwrap(), + Cell::Float64(2.5) + ); + assert_eq!( + eval(&field(-1, "value"), std::slice::from_ref(&samples), &schema).unwrap(), + Cell::Null + ); + assert_eq!( + eval(&field(99, "ts"), std::slice::from_ref(&samples), &schema).unwrap(), + Cell::Int64(0) + ); + assert_eq!( + eval(&field(99, "value"), std::slice::from_ref(&samples), &schema).unwrap(), + Cell::Null + ); + } + fn schema(fields: &[(&str, DataType)]) -> SummarySchema { SummarySchema { fields: fields diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs new file mode 100644 index 000000000..e676f9b74 --- /dev/null +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs @@ -0,0 +1,120 @@ +//! Native collection metadata is checked against the existing shared schema. + +use planner_types::pre_asap::Column; + +/// Split native type arguments without splitting nested types or quoted names. +fn arguments(input: &str) -> Option> { + let mut result = Vec::new(); + let mut start = 0; + let mut depth = 0_usize; + let mut quote = None; + let mut chars = input.char_indices().peekable(); + while let Some((position, ch)) = chars.next() { + if let Some(delimiter) = quote { + if ch == '\\' { + chars.next()?; + } else if ch == delimiter { + if chars.peek().is_some_and(|(_, next)| *next == delimiter) { + chars.next(); + } else { + quote = None; + } + } + continue; + } + match ch { + '\'' | '`' | '"' => quote = Some(ch), + '(' => depth = depth.checked_add(1)?, + ')' => depth = depth.checked_sub(1)?, + ',' if depth == 0 => { + result.push(input[start..position].trim()); + start = position + 1; + } + _ => {} + } + } + if depth != 0 || quote.is_some() { + return None; + } + if !input.is_empty() { + result.push(input[start..].trim()); + } + (!result.iter().any(|argument| argument.is_empty())).then_some(result) +} + +/// Anonymous native Tuple fields have explicit one-based names in the shared +/// Struct schema. Named fields must match their native names exactly. Other +/// Arrow names are never interpreted as an anonymous Tuple. +pub(super) fn tuple_field_types<'a>(native: &'a str, fields: &[Column]) -> Option> { + let inner = native.strip_prefix("Tuple(")?.strip_suffix(')')?; + let args = arguments(inner)?; + if args.len() != fields.len() { + return None; + } + let mut types = Vec::with_capacity(fields.len()); + for (index, (argument, field)) in args.into_iter().zip(fields).enumerate() { + if field.table.is_some() { + return None; + } + if field.name == (index + 1).to_string() + && super::clickhouse_type_matches(Some(argument), &field.dtype, field.nullable) + { + types.push(argument); + continue; + } + // Initial named transport accepts ordinary identifiers. Quoted names + // remain unsupported until a native identifier-decoding contract exists. + let (name, native_type) = argument.split_once(char::is_whitespace)?; + if name != field.name + || name.is_empty() + || !name + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || ch == '_') + || !super::clickhouse_type_matches( + Some(native_type.trim()), + &field.dtype, + field.nullable, + ) + { + return None; + } + types.push(native_type.trim()); + } + Some(types) +} + +#[cfg(test)] +mod tests { + use super::*; + use planner_types::pre_asap::DataType; + + #[test] + fn tuple_metadata_preserves_names_order_and_nested_types() { + let fields = vec![ + Column::new("ts", DataType::Int64, false), + Column::new( + "samples", + DataType::List { + element: Box::new(Column::new("item", DataType::Float64, true)), + }, + false, + ), + ]; + assert_eq!( + tuple_field_types("Tuple(ts Int64, samples Array(Nullable(Float64)))", &fields), + Some(vec!["Int64", "Array(Nullable(Float64))"]) + ); + assert!( + tuple_field_types("Tuple(samples Int64, ts Array(Nullable(Float64)))", &fields) + .is_none() + ); + assert!(tuple_field_types("Tuple(Int64, Array(Nullable(Float64)))", &fields).is_none()); + let anonymous = vec![ + Column::new("1", DataType::Int64, false), + Column::new("2", DataType::Utf8, false), + ]; + assert!(tuple_field_types("Tuple(Int64, String)", &anonymous).is_some()); + assert!(arguments("Map(String, Tuple(Int64, String)), DateTime64(3, 'UTC')").is_some()); + assert!(arguments("Map(String, Tuple(Int64, String)").is_none()); + } +} From cce8a1e7710e1e47e8744b5090d251f168b343e6 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:13:09 -0600 Subject: [PATCH 07/29] refactor(clickhouse): share nested type argument parsing --- .../relational_adapter.rs | 19 +++++-------------- .../relational_adapter/collection.rs | 2 +- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs index 04de311d6..0bfe338a1 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs @@ -307,20 +307,11 @@ impl ClickHouseRelation { fn map_type_parts(actual: &str) -> Option<(&str, &str)> { let inner = actual.trim().strip_prefix("Map(")?.strip_suffix(')')?; - let mut depth = 0_i32; - let mut quoted = false; - for (index, ch) in inner.char_indices() { - match ch { - '\'' => quoted = !quoted, - '(' if !quoted => depth += 1, - ')' if !quoted => depth -= 1, - ',' if !quoted && depth == 0 => { - return Some((inner[..index].trim(), inner[index + 1..].trim())) - } - _ => {} - } - } - None + let args = collection::arguments(inner)?; + let [key, value] = args.as_slice() else { + return None; + }; + Some((*key, *value)) } fn clickhouse_type_matches(actual: Option<&str>, expected: &DataType, nullable: bool) -> bool { diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs index e676f9b74..2bb43e7bb 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter/collection.rs @@ -3,7 +3,7 @@ use planner_types::pre_asap::Column; /// Split native type arguments without splitting nested types or quoted names. -fn arguments(input: &str) -> Option> { +pub(super) fn arguments(input: &str) -> Option> { let mut result = Vec::new(); let mut start = 0; let mut depth = 0_usize; From 10909feabf05a88b60726402a0d701e4926eec82 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:13:53 -0600 Subject: [PATCH 08/29] fix(clickhouse): distinguish nonfinite exact values from nulls --- .../accelerator.rs | 3 +++ .../relational_adapter.rs | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+) 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 907b7fea5..8c8933750 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 @@ -111,6 +111,9 @@ impl CatalogClickHouseAccelerator { "0".into(), ); parameters.insert("output_format_json_quote_64bit_integers".into(), "0".into()); + // Preserve the distinction between NULL and unsupported NaN/Inf. + // The typed decoder rejects quoted non-finite values and falls back. + parameters.insert("output_format_json_quote_denormals".into(), "1".into()); if let Some(database) = request_context.database() { parameters.insert("database".into(), database.into()); } diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs index 072c230a2..ea399ba25 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs @@ -1105,6 +1105,33 @@ mod tests { .is_err()); } + #[test] + fn nonfinite_external_array_values_cannot_become_nulls() { + use planner_types::pre_asap::Column; + let dtype = DataType::List { + element: Box::new(Column::new("item", DataType::Float64, true)), + }; + for value in ["inf", "-inf", "nan"] { + assert!(json_cell( + &serde_json::json!([value]), + &dtype, + false, + "Array(Nullable(Float64))" + ) + .is_err()); + } + assert_eq!( + json_cell( + &serde_json::json!([null]), + &dtype, + false, + "Array(Nullable(Float64))" + ) + .unwrap(), + Cell::List(vec![Cell::Null].into()) + ); + } + #[test] fn array_access_uses_signed_indices_and_element_defaults() { use planner_types::pre_asap::{Column, Schema}; From f8261340035a85f1ec8f8abd16be20c9935192e8 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:14:14 -0600 Subject: [PATCH 09/29] test(clickhouse): verify exact denormal transport policy --- .../asap_clickhouse_query_engine/accelerator.rs | 4 ++++ .../asap_clickhouse_query_engine/relational_adapter.rs | 7 +++++++ 2 files changed, 11 insertions(+) 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 8c8933750..8e34ceafd 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 @@ -331,6 +331,10 @@ mod tests { request: &ClickHouseQueryRequest, ) -> Result { + assert_eq!( + request.parameters.get("output_format_json_quote_denormals"), + Some(&"1".into()) + ); assert_eq!(request.parameters.get("param_from"), Some(&"0".into())); assert_eq!(request.parameters.get("param_to"), Some(&"2000".into())); Ok(ClickHouseRawResponse { diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs index ea399ba25..cdecb8791 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/relational_adapter.rs @@ -1112,6 +1112,13 @@ mod tests { element: Box::new(Column::new("item", DataType::Float64, true)), }; for value in ["inf", "-inf", "nan"] { + assert!(json_cell( + &serde_json::json!(value), + &DataType::Float64, + true, + "Nullable(Float64)" + ) + .is_err()); assert!(json_cell( &serde_json::json!([value]), &dtype, From 05b57db76045298522ab7470e9e982e4506d3965 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:15:57 -0600 Subject: [PATCH 10/29] test(clickhouse): verify native tuple field rendering --- .../src/query_plan/clickhouse_exact.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/control_plane/src/query_plan/clickhouse_exact.rs b/control_plane/src/query_plan/clickhouse_exact.rs index 3f8161b32..fa227841f 100644 --- a/control_plane/src/query_plan/clickhouse_exact.rs +++ b/control_plane/src/query_plan/clickhouse_exact.rs @@ -324,6 +324,31 @@ mod tests { ); } + #[test] + fn typed_struct_field_renders_native_lookup() { + let schema = Schema::new(vec![Column::new( + "sample", + DataType::Struct { + fields: vec![ + Column::new("ts", DataType::Int64, false), + Column::new("value", DataType::Float64, true), + ], + }, + false, + )]); + let expr = QueryExpr::FunctionCall { + name: "asap_struct_field".into(), + args: vec![ + QueryExpr::Column(0), + QueryExpr::Literal(ScalarValue::Utf8("value".into())), + ], + }; + assert_eq!( + scalar(&expr, &schema).unwrap(), + "tupleElement(`sample`, 'value')" + ); + } + #[test] fn unsupported_scalar_is_not_forwarded_as_arbitrary_native_code() { let schema = Schema::new(vec![]); From f73835bef0440838b08d71c889d2344ac953608d Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:16:11 -0600 Subject: [PATCH 11/29] fix(storage): reserve immutable output publication durably --- .../sketch_db/persistence/flusher.rs | 37 +- .../sketch_db/persistence/immutable_output.rs | 512 ++++++++++++++++++ .../sketch_db/persistence/metadata.rs | 75 ++- .../sketch_db/persistence/mod.rs | 1 + .../sketch_db/persistence/recovery.rs | 12 +- 5 files changed, 629 insertions(+), 8 deletions(-) create mode 100644 data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs diff --git a/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs b/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs index e774b39e6..4c8307db2 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs @@ -32,7 +32,7 @@ use super::{PersistError, PersistResult}; /// Handle to a running flusher thread. Dropping the handle signals /// shutdown and joins the thread. pub struct FlusherHandle { - inner: Arc, + pub(super) inner: Arc, thread: Option>, } @@ -58,6 +58,14 @@ pub(crate) struct FlusherShared { pub back_pressure_wait_count: AtomicU64, } +impl FlusherShared { + pub(super) fn allocate_part_id(&self) -> PersistResult { + self.next_part_id + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |id| id.checked_add(1)) + .map_err(|_| PersistError::Internal("part ID exhausted".into())) + } +} + impl FlusherHandle { /// Start a flusher thread. Takes an `EpochSource` (typically the /// store itself, wrapped in `Arc`). @@ -84,12 +92,23 @@ impl FlusherHandle { { // Pick a starting part_id: one past the max currently in the // manifest (so IDs are monotonically increasing across restarts). + let reserved = sid_metadata.load_strict()?.into_iter().flat_map(|r| { + [r.pending_immutable, r.last_immutable] + .into_iter() + .flatten() + .map(|p| p.part_id) + }); let next_id = manifest .live_parts() .iter() .map(|p| p.part_id) + .chain(reserved) .max() - .map(|m| m + 1) + .map(|m| { + m.checked_add(1) + .ok_or_else(|| PersistError::Internal("part ID exhausted".into())) + }) + .transpose()? .unwrap_or(1); let shared = Arc::new(FlusherShared { @@ -369,7 +388,7 @@ fn run_tick(shared: &Arc, source: &S) -> PersistR if !snapshots.is_empty() { // Build one part for the tick. - let part_id = shared.next_part_id.fetch_add(1, Ordering::Relaxed); + let part_id = shared.allocate_part_id()?; let part_dir = part_dir_path(&parts_root(&cfg.disk_path), part_id); let entries_total: usize = snapshots.iter().map(|s| s.len()).sum(); let size_bytes_estimate: u64 = snapshots.iter().map(|s| s.approx_bytes as u64).sum(); @@ -428,6 +447,18 @@ fn run_tick(shared: &Arc, source: &S) -> PersistR .into_iter() .filter(|p| p.max_ts < cutoff) .collect(); + // Read reservations after capturing candidates: a newly published part + // cannot enter the older candidate set after this check. + let reserved: std::collections::HashSet<_> = shared + .sid_metadata + .load_strict()? + .into_iter() + .filter_map(|record| record.pending_immutable.map(|pending| pending.part_id)) + .collect(); + let expired: Vec<_> = expired + .into_iter() + .filter(|part| !reserved.contains(&part.part_id)) + .collect(); for p in &expired { shared.manifest.append_delete(p.part_id)?; let dir = part_dir_path(&parts_root(&cfg.disk_path), p.part_id); diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs new file mode 100644 index 000000000..7264f52b8 --- /dev/null +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -0,0 +1,512 @@ +//! Crash-resumable publication of one immutable output window. The SID sidecar +//! reserves the existing part ID before writing; no second payload store is used. +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; + +use super::flusher::{FlusherHandle, FlusherShared}; +use super::manifest::PartEntry; +use super::metadata::SidMetaRecord; +use super::part::{part_dir_path, PartReader, PartWriter}; +use super::source::{EpochSnapshot, EpochSnapshotEntry}; +use super::{PersistError, PersistResult}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ImmutableOutputReservation { + pub part_id: u64, + pub input_digest: [u8; 32], + pub payload_digest: [u8; 32], + pub start_ms: u64, + pub end_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ImmutableWindowPublication { + pub part_id: u64, + pub already_published: bool, +} + +fn invalid(message: &str) -> PersistError { + PersistError::Format(message.into()) +} + +fn fingerprint(snapshot: &EpochSnapshot) -> PersistResult<[u8; 32]> { + if snapshot.entries.is_empty() || snapshot.min_ts >= snapshot.max_ts { + return Err(invalid("immutable publication requires a nonempty window")); + } + let mut rows = Vec::with_capacity(snapshot.entries.len()); + for entry in &snapshot.entries { + if entry.start_ts != snapshot.min_ts || entry.end_ts != snapshot.max_ts { + return Err(invalid("immutable publication spans multiple windows")); + } + let label = entry.label.as_ref().map(|label| label.serialize_to_bytes()); + rows.push(( + label, + &entry.sketch_type_name, + entry.encoding_tag, + &entry.sketch_bytes, + )); + } + rows.sort(); + if rows.windows(2).any(|pair| pair[0].0 == pair[1].0) { + return Err(invalid( + "immutable publication contains duplicate label populations", + )); + } + let mut digest = Sha256::new(); + digest.update(b"asap-immutable-window-v1"); + for value in [ + snapshot.agg_id, + snapshot.min_ts, + snapshot.max_ts, + rows.len() as u64, + ] { + digest.update(value.to_le_bytes()); + } + for (label, kind, encoding, payload) in rows { + digest.update([u8::from(label.is_some())]); + let label = label.as_deref().unwrap_or_default(); + for bytes in [label, kind.as_bytes()] { + digest.update((bytes.len() as u64).to_le_bytes()); + digest.update(bytes); + } + digest.update([encoding]); + digest.update((payload.len() as u64).to_le_bytes()); + digest.update(payload); + } + Ok(digest.finalize().into()) +} + +impl FlusherHandle { + /// Reuse the existing persistence state without transferring ownership of + /// its worker thread. The store can retain a Weak reference to this Arc. + pub(crate) fn publication_handle(&self) -> std::sync::Arc { + std::sync::Arc::clone(&self.inner) + } + pub fn publish_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + snapshot: &EpochSnapshot, + ) -> PersistResult { + self.inner + .publish_immutable_window(record, input_digest, snapshot) + } + pub fn resume_pending_immutable_window( + &self, + sid: u64, + ) -> PersistResult> { + self.inner.resume_pending_immutable_window(sid) + } +} + +impl FlusherShared { + /// Publish a finalized window. A retry must present exactly the reserved + /// input and payload. This does not guarantee source availability after GC. + pub fn publish_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + snapshot: &EpochSnapshot, + ) -> PersistResult { + if snapshot.agg_id != record.sid || record.removed { + return Err(invalid("immutable publication SID is invalid or removed")); + } + let payload_digest = fingerprint(snapshot)?; + self.sid_metadata.transaction(|records, metadata| { + let key = record.sid.to_string(); + let mut current = records.get(&key).cloned().unwrap_or_else(|| record.clone()); + if current.removed + || current.retired_at_ms.is_some() + || current.expires_at_ms.is_some() + || current.summary_definition_id != record.summary_definition_id + || current.catalog_generation != record.catalog_generation + || current.metric_name != record.metric_name + || current.group_by_keys != record.group_by_keys + || !current.same_kind(record) + { + return Err(invalid("immutable publication metadata identity changed")); + } + if current + .completed_through_ms + .is_some_and(|end| snapshot.min_ts < end) + { + let Some(previous) = ¤t.last_immutable else { + return Err(invalid( + "completed immutable window has no retained lineage proof", + )); + }; + if previous.input_digest != input_digest + || previous.payload_digest != payload_digest + || previous.start_ms != snapshot.min_ts + || previous.end_ms != snapshot.max_ts + { + return Err(invalid( + "completed immutable retry differs from latest publication", + )); + } + self.validate_reserved_part(record.sid, previous)?; + return Ok(ImmutableWindowPublication { + part_id: previous.part_id, + already_published: true, + }); + } + let reservation = match current.pending_immutable.clone() { + Some(pending) => { + if pending.input_digest != input_digest + || pending.payload_digest != payload_digest + || pending.start_ms != snapshot.min_ts + || pending.end_ms != snapshot.max_ts + { + return Err(invalid( + "another immutable publication is pending for this SID", + )); + } + pending + } + None => { + let pending = ImmutableOutputReservation { + part_id: self.allocate_part_id()?, + input_digest, + payload_digest, + start_ms: snapshot.min_ts, + end_ms: snapshot.max_ts, + }; + current.pending_immutable = Some(pending.clone()); + records.insert(key.clone(), current.clone()); + metadata.write_records(records)?; + pending + } + }; + let path = part_dir_path(&self.cfg.disk_path.join("parts"), reservation.part_id); + if path.exists() { + // Never overwrite a reserved part silently: callers can recover a + // durable part without sources; damaged/partial parts fail closed. + self.validate_reserved_part(record.sid, &reservation)?; + } else { + PartWriter::write_part(&path, reservation.part_id, std::slice::from_ref(snapshot))?; + std::fs::File::open( + path.parent() + .ok_or_else(|| invalid("missing parts parent"))?, + )? + .sync_all()?; + self.validate_reserved_part(record.sid, &reservation)?; + } + self.finish_reserved_part(&mut current, &reservation)?; + records.insert(key, current); + metadata.write_records(records)?; + Ok(ImmutableWindowPublication { + part_id: reservation.part_id, + already_published: false, + }) + }) + } + + /// Complete a pending durable part without reconstructing its source input. + /// A reservation whose part was never completed returns an error, preserving + /// the reservation for an explicit recovery policy rather than losing data. + pub fn resume_pending_immutable_window( + &self, + sid: u64, + ) -> PersistResult> { + self.sid_metadata.transaction(|records, metadata| { + let key = sid.to_string(); + let Some(mut current) = records.get(&key).cloned() else { + return Ok(None); + }; + let Some(pending) = current.pending_immutable.clone() else { + return Ok(None); + }; + if current.removed || current.retired_at_ms.is_some() || current.expires_at_ms.is_some() + { + return Err(invalid("pending immutable SID was removed")); + } + self.validate_reserved_part(sid, &pending)?; + self.finish_reserved_part(&mut current, &pending)?; + records.insert(key, current); + metadata.write_records(records)?; + Ok(Some(ImmutableWindowPublication { + part_id: pending.part_id, + already_published: true, + })) + }) + } + + fn validate_reserved_part( + &self, + sid: u64, + pending: &ImmutableOutputReservation, + ) -> PersistResult<()> { + let reader = PartReader::open(&part_dir_path( + &self.cfg.disk_path.join("parts"), + pending.part_id, + ))?; + if reader.meta.part_id != pending.part_id { + return Err(invalid("reserved part ID mismatch")); + } + let mut entries = Vec::new(); + for index in reader.index_records() { + let row = reader.load_entry(&index)?; + if row.agg_id != sid { + return Err(invalid("reserved part contains another SID")); + } + entries.push(EpochSnapshotEntry { + start_ts: row.start_ts, + end_ts: row.end_ts, + label: row.label, + sketch_type_name: row.sketch_type_name, + encoding_tag: row.encoding_tag, + sketch_bytes: row.sketch_bytes, + }); + } + let snapshot = EpochSnapshot { + agg_id: sid, + epoch_id: 0, + min_ts: pending.start_ms, + max_ts: pending.end_ms, + entries, + approx_bytes: 0, + }; + if fingerprint(&snapshot)? != pending.payload_digest { + return Err(invalid("reserved immutable payload digest mismatch")); + } + Ok(()) + } + + fn finish_reserved_part( + &self, + current: &mut SidMetaRecord, + pending: &ImmutableOutputReservation, + ) -> PersistResult<()> { + let path = part_dir_path(&self.cfg.disk_path.join("parts"), pending.part_id); + // The reservation may have been recovered before its parent-directory + // entry was durable; make that durable before manifest publication too. + std::fs::File::open( + path.parent() + .ok_or_else(|| invalid("missing parts parent"))?, + )? + .sync_all()?; + if !self + .manifest + .live_parts() + .iter() + .any(|part| part.part_id == pending.part_id) + { + let size_bytes = std::fs::read_dir(&path)?.try_fold(0u64, |sum, entry| { + let length = entry?.metadata()?.len(); + sum.checked_add(length) + .ok_or_else(|| std::io::Error::other("part size overflow")) + })?; + self.manifest.append_add(PartEntry { + part_id: pending.part_id, + min_ts: pending.start_ms, + max_ts: pending.end_ms, + size_bytes, + })?; + } + current.completed_through_ms = Some( + current + .completed_through_ms + .unwrap_or(0) + .max(pending.end_ms), + ); + current.last_immutable = Some(pending.clone()); + current.pending_immutable = None; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::super::{ + config::SketchStorePersistenceConfig, + manifest::Manifest, + source::{EpochSource, SealedEpochRef}, + }; + use super::*; + use crate::storage_engines::{sketch_db::data::AggKind, types::AggregationType}; + use std::sync::Arc; + + struct EmptySource; + impl EpochSource for EmptySource { + fn list_sealed_epochs(&self) -> Vec { + vec![] + } + fn snapshot_sealed_epoch(&self, _: u64, _: u64) -> PersistResult> { + Ok(None) + } + fn evict_sealed_epoch(&self, _: u64, _: u64) {} + fn approx_memory_bytes(&self) -> usize { + 0 + } + } + fn handle(path: &std::path::Path) -> FlusherHandle { + let mut config = SketchStorePersistenceConfig::with_memory_limit(1024, path.into()); + config.hot_window_ms = None; + config.delete_older_than_ms = None; + FlusherHandle::start( + config, + Arc::new(Manifest::open_or_init(path).unwrap()), + Arc::new(EmptySource), + ) + .unwrap() + } + fn record() -> SidMetaRecord { + SidMetaRecord::new( + 1, + "derived".into(), + vec![], + &AggKind::ExactAgg { + agg_type: AggregationType::Sum, + parameters_canonical: String::new(), + spatial_filter_canonical: String::new(), + }, + 0, + ) + } + fn snapshot() -> EpochSnapshot { + EpochSnapshot { + agg_id: 1, + epoch_id: 0, + min_ts: 10, + max_ts: 20, + approx_bytes: 8, + entries: vec![EpochSnapshotEntry { + start_ts: 10, + end_ts: 20, + label: None, + sketch_type_name: "SumAccumulator".into(), + encoding_tag: 0, + sketch_bytes: vec![1, 2, 3], + }], + } + } + fn reserve(handle: &FlusherHandle, snapshot: &EpochSnapshot) -> ImmutableOutputReservation { + let pending = ImmutableOutputReservation { + part_id: handle.inner.allocate_part_id().unwrap(), + input_digest: [7; 32], + payload_digest: fingerprint(snapshot).unwrap(), + start_ms: 10, + end_ms: 20, + }; + handle + .inner + .sid_metadata + .transaction(|records, metadata| { + let mut record = record(); + record.pending_immutable = Some(pending.clone()); + records.insert("1".into(), record); + metadata.write_records(records) + }) + .unwrap(); + pending + } + #[test] + fn completed_retry_and_concurrent_publications_do_not_duplicate_parts() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + std::thread::scope(|scope| { + let workers: Vec<_> = (0..4) + .map(|_| { + scope.spawn(|| { + handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .unwrap() + }) + }) + .collect(); + let ids: Vec<_> = workers + .into_iter() + .map(|worker| worker.join().unwrap().part_id) + .collect(); + assert!(ids.iter().all(|id| *id == ids[0])); + }); + assert_eq!(handle.manifest().live_parts().len(), 1); + assert!(handle + .publish_immutable_window(&record(), [8; 32], &snapshot()) + .is_err()); + let metadata = handle.inner.sid_metadata.load_strict().unwrap(); + assert_eq!(metadata[0].completed_through_ms, Some(20)); + assert!(metadata[0].pending_immutable.is_none()); + let mut different = snapshot(); + different.entries[0].sketch_bytes.push(4); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &different) + .is_err()); + } + #[test] + fn restart_preserves_reserved_part_and_resumes_without_source() { + let temp = tempfile::tempdir().unwrap(); + let mut first = handle(temp.path()); + let state = snapshot(); + let pending = reserve(&first, &state); + let path = part_dir_path(&temp.path().join("parts"), pending.part_id); + PartWriter::write_part(&path, pending.part_id, &[state]).unwrap(); + first.shutdown(); + drop(first); + let (_, recovery) = super::super::recovery::recover(temp.path()).unwrap(); + assert_eq!(recovery.orphan_parts_removed, 0); + let resumed = handle(temp.path()); + assert!(resumed.inner.allocate_part_id().unwrap() > pending.part_id); + let result = resumed.resume_pending_immutable_window(1).unwrap().unwrap(); + assert_eq!(result.part_id, pending.part_id); + assert_eq!(resumed.manifest().live_parts().len(), 1); + assert!(resumed + .resume_pending_immutable_window(1) + .unwrap() + .is_none()); + } + #[test] + fn pending_missing_part_requires_source_and_stale_upsert_preserves_reservation() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + let pending = reserve(&handle, &snapshot()); + handle.inner.sid_metadata.upsert_all(&[record()]).unwrap(); + assert_eq!( + handle.inner.sid_metadata.load_strict().unwrap()[0].pending_immutable, + Some(pending.clone()) + ); + assert!(handle.resume_pending_immutable_window(1).is_err()); + assert_eq!( + handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .unwrap() + .part_id, + pending.part_id + ); + handle.inner.sid_metadata.upsert_all(&[record()]).unwrap(); + assert!(handle.inner.sid_metadata.load_strict().unwrap()[0] + .pending_immutable + .is_none()); + } + #[test] + fn malformed_metadata_fails_before_part_publication() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + std::fs::write(handle.inner.sid_metadata.path(), b"broken").unwrap(); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .is_err()); + assert!(handle.manifest().live_parts().is_empty()); + } + #[test] + fn failed_manifest_leaves_recoverable_payload_and_no_completion() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + let log = handle.manifest().log_path(); + let backup = log.with_extension("saved"); + std::fs::rename(&log, &backup).unwrap(); + std::fs::create_dir(&log).unwrap(); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .is_err()); + let records = handle.inner.sid_metadata.load_strict().unwrap(); + let pending = records[0].pending_immutable.clone().unwrap(); + assert_eq!(records[0].completed_through_ms, None); + assert!(handle.manifest().live_parts().is_empty()); + std::fs::remove_dir(&log).unwrap(); + std::fs::rename(&backup, &log).unwrap(); + let published = handle.resume_pending_immutable_window(1).unwrap().unwrap(); + assert_eq!(published.part_id, pending.part_id); + assert_eq!(handle.manifest().live_parts().len(), 1); + } +} diff --git a/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs b/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs index c432e07f2..dfdf7b4bc 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs @@ -322,6 +322,10 @@ pub struct SidMetaRecord { /// No further publication may change a window ending at or before this bound. #[serde(default)] pub completed_through_ms: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pending_immutable: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub last_immutable: Option, } impl SidMetaRecord { @@ -347,9 +351,15 @@ impl SidMetaRecord { expires_at_ms: None, removed: false, completed_through_ms: None, + pending_immutable: None, + last_immutable: None, } } + pub(super) fn same_kind(&self, other: &Self) -> bool { + self.agg_kind == other.agg_kind + } + /// Reconstruct the structured [`AggKind`], or `None` for an /// unrecognized sketch kind. pub fn agg_kind(&self) -> Option { @@ -416,6 +426,10 @@ struct SidBindingRec { removed: bool, #[serde(default)] completed_through_ms: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pending_immutable: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + last_immutable: Option, } /// Version-3 normalized sidecar with authoritative catalog provenance. Descriptors appear once and SeriesId bindings hold @@ -487,6 +501,8 @@ impl SdsSidecar { expires_at_ms: record.expires_at_ms, removed: record.removed, completed_through_ms: record.completed_through_ms, + pending_immutable: record.pending_immutable, + last_immutable: record.last_immutable, }, ); } @@ -541,6 +557,8 @@ impl SdsSidecar { expires_at_ms: binding.expires_at_ms, removed: binding.removed, completed_through_ms: binding.completed_through_ms, + pending_immutable: binding.pending_immutable, + last_immutable: binding.last_immutable, }) }) .collect() @@ -646,7 +664,7 @@ impl SidMetadataStore { return Ok(()); } let mut map: HashMap = self - .load()? + .load_strict()? .into_iter() .map(|r| (r.sid.to_string(), r)) .collect(); @@ -657,6 +675,8 @@ impl SidMetadataStore { if let Some(existing) = map.get(&key) { // Lifecycle is monotone for a SeriesId. An older flush snapshot // must not resurrect a retired or removed persisted instance. + next.pending_immutable = existing.pending_immutable.clone(); + next.last_immutable = existing.last_immutable.clone(); next.removed |= existing.removed; next.completed_through_ms = existing.completed_through_ms.max(next.completed_through_ms); @@ -680,6 +700,55 @@ impl SidMetadataStore { self.write_atomic(json.as_bytes()) } + pub fn load_strict(&self) -> PersistResult> { + let bytes = match fs::read(&self.path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(error.into()), + }; + let value: serde_json::Value = serde_json::from_slice(&bytes) + .map_err(|error| PersistError::Format(format!("invalid SID metadata: {error}")))?; + if let Some(version) = value.get("schema_version") { + if !matches!(version.as_u64(), Some(2 | 3)) { + return Err(PersistError::Format( + "unsupported SID metadata version".into(), + )); + } + let sidecar: SdsSidecar = serde_json::from_value(value) + .map_err(|error| PersistError::Format(error.to_string()))?; + sidecar.into_records() + } else { + let records: HashMap = serde_json::from_value(value) + .map_err(|error| PersistError::Format(error.to_string()))?; + Ok(records.into_values().collect()) + } + } + + pub(super) fn transaction( + &self, + operation: impl FnOnce(&mut HashMap, &Self) -> PersistResult, + ) -> PersistResult { + let _writer = self + .writer + .lock() + .map_err(|_| PersistError::Internal("SID metadata writer poisoned".into()))?; + let mut records = self + .load_strict()? + .into_iter() + .map(|r| (r.sid.to_string(), r)) + .collect(); + operation(&mut records, self) + } + pub(super) fn write_records( + &self, + records: &HashMap, + ) -> PersistResult<()> { + let sidecar = SdsSidecar::from_records(records.values().cloned()); + let bytes = + serde_json::to_vec(&sidecar).map_err(|e| PersistError::Serialize(e.to_string()))?; + self.write_atomic(&bytes) + } + fn write_atomic(&self, bytes: &[u8]) -> PersistResult<()> { if let Some(parent) = self.path.parent() { fs::create_dir_all(parent)?; @@ -696,9 +765,7 @@ impl SidMetadataStore { } fs::rename(&tmp, &self.path)?; if let Some(parent) = self.path.parent() { - if let Ok(dir) = File::open(parent) { - let _ = dir.sync_all(); - } + File::open(parent)?.sync_all()?; } Ok(()) } diff --git a/data_plane/src/storage_engines/sketch_db/persistence/mod.rs b/data_plane/src/storage_engines/sketch_db/persistence/mod.rs index 349feb92e..552fe4e03 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/mod.rs @@ -27,6 +27,7 @@ pub mod source; pub mod cache; pub mod flusher; +pub mod immutable_output; pub mod recovery; pub use config::SketchStorePersistenceConfig; diff --git a/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs b/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs index 7fb800946..a21402608 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs @@ -34,7 +34,12 @@ pub fn recover(disk_path: &Path) -> PersistResult<(Manifest, RecoveryReport)> { // Validate every live part by reading its meta.bin header. let mut to_drop: Vec = Vec::new(); - let mut referenced: HashSet = HashSet::new(); + let pending: HashSet = super::metadata::SidMetadataStore::new(disk_path) + .load_strict()? + .into_iter() + .filter_map(|record| record.pending_immutable.map(|pending| pending.part_id)) + .collect(); + let mut referenced = pending.clone(); for entry in manifest.live_parts() { referenced.insert(entry.part_id); let part_dir = super::part::part_dir_path(&parts_root, entry.part_id); @@ -61,6 +66,11 @@ pub fn recover(disk_path: &Path) -> PersistResult<(Manifest, RecoveryReport)> { } } + if to_drop.iter().any(|part_id| pending.contains(part_id)) { + return Err(super::PersistError::Format( + "corrupt reserved immutable part".into(), + )); + } for part_id in to_drop { manifest.append_delete(part_id)?; let dir = super::part::part_dir_path(&parts_root, part_id); From 8b0d9effc4fd1e670a0a8691a4fcd6d986179fea Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:16:11 -0600 Subject: [PATCH 12/29] fix(storage): reserve immutable output publication durably --- .../sketch_db/persistence/flusher.rs | 37 +- .../sketch_db/persistence/immutable_output.rs | 512 ++++++++++++++++++ .../sketch_db/persistence/metadata.rs | 75 ++- .../sketch_db/persistence/mod.rs | 1 + .../sketch_db/persistence/recovery.rs | 12 +- 5 files changed, 629 insertions(+), 8 deletions(-) create mode 100644 data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs diff --git a/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs b/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs index e774b39e6..4c8307db2 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/flusher.rs @@ -32,7 +32,7 @@ use super::{PersistError, PersistResult}; /// Handle to a running flusher thread. Dropping the handle signals /// shutdown and joins the thread. pub struct FlusherHandle { - inner: Arc, + pub(super) inner: Arc, thread: Option>, } @@ -58,6 +58,14 @@ pub(crate) struct FlusherShared { pub back_pressure_wait_count: AtomicU64, } +impl FlusherShared { + pub(super) fn allocate_part_id(&self) -> PersistResult { + self.next_part_id + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |id| id.checked_add(1)) + .map_err(|_| PersistError::Internal("part ID exhausted".into())) + } +} + impl FlusherHandle { /// Start a flusher thread. Takes an `EpochSource` (typically the /// store itself, wrapped in `Arc`). @@ -84,12 +92,23 @@ impl FlusherHandle { { // Pick a starting part_id: one past the max currently in the // manifest (so IDs are monotonically increasing across restarts). + let reserved = sid_metadata.load_strict()?.into_iter().flat_map(|r| { + [r.pending_immutable, r.last_immutable] + .into_iter() + .flatten() + .map(|p| p.part_id) + }); let next_id = manifest .live_parts() .iter() .map(|p| p.part_id) + .chain(reserved) .max() - .map(|m| m + 1) + .map(|m| { + m.checked_add(1) + .ok_or_else(|| PersistError::Internal("part ID exhausted".into())) + }) + .transpose()? .unwrap_or(1); let shared = Arc::new(FlusherShared { @@ -369,7 +388,7 @@ fn run_tick(shared: &Arc, source: &S) -> PersistR if !snapshots.is_empty() { // Build one part for the tick. - let part_id = shared.next_part_id.fetch_add(1, Ordering::Relaxed); + let part_id = shared.allocate_part_id()?; let part_dir = part_dir_path(&parts_root(&cfg.disk_path), part_id); let entries_total: usize = snapshots.iter().map(|s| s.len()).sum(); let size_bytes_estimate: u64 = snapshots.iter().map(|s| s.approx_bytes as u64).sum(); @@ -428,6 +447,18 @@ fn run_tick(shared: &Arc, source: &S) -> PersistR .into_iter() .filter(|p| p.max_ts < cutoff) .collect(); + // Read reservations after capturing candidates: a newly published part + // cannot enter the older candidate set after this check. + let reserved: std::collections::HashSet<_> = shared + .sid_metadata + .load_strict()? + .into_iter() + .filter_map(|record| record.pending_immutable.map(|pending| pending.part_id)) + .collect(); + let expired: Vec<_> = expired + .into_iter() + .filter(|part| !reserved.contains(&part.part_id)) + .collect(); for p in &expired { shared.manifest.append_delete(p.part_id)?; let dir = part_dir_path(&parts_root(&cfg.disk_path), p.part_id); diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs new file mode 100644 index 000000000..7264f52b8 --- /dev/null +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -0,0 +1,512 @@ +//! Crash-resumable publication of one immutable output window. The SID sidecar +//! reserves the existing part ID before writing; no second payload store is used. +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; + +use super::flusher::{FlusherHandle, FlusherShared}; +use super::manifest::PartEntry; +use super::metadata::SidMetaRecord; +use super::part::{part_dir_path, PartReader, PartWriter}; +use super::source::{EpochSnapshot, EpochSnapshotEntry}; +use super::{PersistError, PersistResult}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ImmutableOutputReservation { + pub part_id: u64, + pub input_digest: [u8; 32], + pub payload_digest: [u8; 32], + pub start_ms: u64, + pub end_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ImmutableWindowPublication { + pub part_id: u64, + pub already_published: bool, +} + +fn invalid(message: &str) -> PersistError { + PersistError::Format(message.into()) +} + +fn fingerprint(snapshot: &EpochSnapshot) -> PersistResult<[u8; 32]> { + if snapshot.entries.is_empty() || snapshot.min_ts >= snapshot.max_ts { + return Err(invalid("immutable publication requires a nonempty window")); + } + let mut rows = Vec::with_capacity(snapshot.entries.len()); + for entry in &snapshot.entries { + if entry.start_ts != snapshot.min_ts || entry.end_ts != snapshot.max_ts { + return Err(invalid("immutable publication spans multiple windows")); + } + let label = entry.label.as_ref().map(|label| label.serialize_to_bytes()); + rows.push(( + label, + &entry.sketch_type_name, + entry.encoding_tag, + &entry.sketch_bytes, + )); + } + rows.sort(); + if rows.windows(2).any(|pair| pair[0].0 == pair[1].0) { + return Err(invalid( + "immutable publication contains duplicate label populations", + )); + } + let mut digest = Sha256::new(); + digest.update(b"asap-immutable-window-v1"); + for value in [ + snapshot.agg_id, + snapshot.min_ts, + snapshot.max_ts, + rows.len() as u64, + ] { + digest.update(value.to_le_bytes()); + } + for (label, kind, encoding, payload) in rows { + digest.update([u8::from(label.is_some())]); + let label = label.as_deref().unwrap_or_default(); + for bytes in [label, kind.as_bytes()] { + digest.update((bytes.len() as u64).to_le_bytes()); + digest.update(bytes); + } + digest.update([encoding]); + digest.update((payload.len() as u64).to_le_bytes()); + digest.update(payload); + } + Ok(digest.finalize().into()) +} + +impl FlusherHandle { + /// Reuse the existing persistence state without transferring ownership of + /// its worker thread. The store can retain a Weak reference to this Arc. + pub(crate) fn publication_handle(&self) -> std::sync::Arc { + std::sync::Arc::clone(&self.inner) + } + pub fn publish_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + snapshot: &EpochSnapshot, + ) -> PersistResult { + self.inner + .publish_immutable_window(record, input_digest, snapshot) + } + pub fn resume_pending_immutable_window( + &self, + sid: u64, + ) -> PersistResult> { + self.inner.resume_pending_immutable_window(sid) + } +} + +impl FlusherShared { + /// Publish a finalized window. A retry must present exactly the reserved + /// input and payload. This does not guarantee source availability after GC. + pub fn publish_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + snapshot: &EpochSnapshot, + ) -> PersistResult { + if snapshot.agg_id != record.sid || record.removed { + return Err(invalid("immutable publication SID is invalid or removed")); + } + let payload_digest = fingerprint(snapshot)?; + self.sid_metadata.transaction(|records, metadata| { + let key = record.sid.to_string(); + let mut current = records.get(&key).cloned().unwrap_or_else(|| record.clone()); + if current.removed + || current.retired_at_ms.is_some() + || current.expires_at_ms.is_some() + || current.summary_definition_id != record.summary_definition_id + || current.catalog_generation != record.catalog_generation + || current.metric_name != record.metric_name + || current.group_by_keys != record.group_by_keys + || !current.same_kind(record) + { + return Err(invalid("immutable publication metadata identity changed")); + } + if current + .completed_through_ms + .is_some_and(|end| snapshot.min_ts < end) + { + let Some(previous) = ¤t.last_immutable else { + return Err(invalid( + "completed immutable window has no retained lineage proof", + )); + }; + if previous.input_digest != input_digest + || previous.payload_digest != payload_digest + || previous.start_ms != snapshot.min_ts + || previous.end_ms != snapshot.max_ts + { + return Err(invalid( + "completed immutable retry differs from latest publication", + )); + } + self.validate_reserved_part(record.sid, previous)?; + return Ok(ImmutableWindowPublication { + part_id: previous.part_id, + already_published: true, + }); + } + let reservation = match current.pending_immutable.clone() { + Some(pending) => { + if pending.input_digest != input_digest + || pending.payload_digest != payload_digest + || pending.start_ms != snapshot.min_ts + || pending.end_ms != snapshot.max_ts + { + return Err(invalid( + "another immutable publication is pending for this SID", + )); + } + pending + } + None => { + let pending = ImmutableOutputReservation { + part_id: self.allocate_part_id()?, + input_digest, + payload_digest, + start_ms: snapshot.min_ts, + end_ms: snapshot.max_ts, + }; + current.pending_immutable = Some(pending.clone()); + records.insert(key.clone(), current.clone()); + metadata.write_records(records)?; + pending + } + }; + let path = part_dir_path(&self.cfg.disk_path.join("parts"), reservation.part_id); + if path.exists() { + // Never overwrite a reserved part silently: callers can recover a + // durable part without sources; damaged/partial parts fail closed. + self.validate_reserved_part(record.sid, &reservation)?; + } else { + PartWriter::write_part(&path, reservation.part_id, std::slice::from_ref(snapshot))?; + std::fs::File::open( + path.parent() + .ok_or_else(|| invalid("missing parts parent"))?, + )? + .sync_all()?; + self.validate_reserved_part(record.sid, &reservation)?; + } + self.finish_reserved_part(&mut current, &reservation)?; + records.insert(key, current); + metadata.write_records(records)?; + Ok(ImmutableWindowPublication { + part_id: reservation.part_id, + already_published: false, + }) + }) + } + + /// Complete a pending durable part without reconstructing its source input. + /// A reservation whose part was never completed returns an error, preserving + /// the reservation for an explicit recovery policy rather than losing data. + pub fn resume_pending_immutable_window( + &self, + sid: u64, + ) -> PersistResult> { + self.sid_metadata.transaction(|records, metadata| { + let key = sid.to_string(); + let Some(mut current) = records.get(&key).cloned() else { + return Ok(None); + }; + let Some(pending) = current.pending_immutable.clone() else { + return Ok(None); + }; + if current.removed || current.retired_at_ms.is_some() || current.expires_at_ms.is_some() + { + return Err(invalid("pending immutable SID was removed")); + } + self.validate_reserved_part(sid, &pending)?; + self.finish_reserved_part(&mut current, &pending)?; + records.insert(key, current); + metadata.write_records(records)?; + Ok(Some(ImmutableWindowPublication { + part_id: pending.part_id, + already_published: true, + })) + }) + } + + fn validate_reserved_part( + &self, + sid: u64, + pending: &ImmutableOutputReservation, + ) -> PersistResult<()> { + let reader = PartReader::open(&part_dir_path( + &self.cfg.disk_path.join("parts"), + pending.part_id, + ))?; + if reader.meta.part_id != pending.part_id { + return Err(invalid("reserved part ID mismatch")); + } + let mut entries = Vec::new(); + for index in reader.index_records() { + let row = reader.load_entry(&index)?; + if row.agg_id != sid { + return Err(invalid("reserved part contains another SID")); + } + entries.push(EpochSnapshotEntry { + start_ts: row.start_ts, + end_ts: row.end_ts, + label: row.label, + sketch_type_name: row.sketch_type_name, + encoding_tag: row.encoding_tag, + sketch_bytes: row.sketch_bytes, + }); + } + let snapshot = EpochSnapshot { + agg_id: sid, + epoch_id: 0, + min_ts: pending.start_ms, + max_ts: pending.end_ms, + entries, + approx_bytes: 0, + }; + if fingerprint(&snapshot)? != pending.payload_digest { + return Err(invalid("reserved immutable payload digest mismatch")); + } + Ok(()) + } + + fn finish_reserved_part( + &self, + current: &mut SidMetaRecord, + pending: &ImmutableOutputReservation, + ) -> PersistResult<()> { + let path = part_dir_path(&self.cfg.disk_path.join("parts"), pending.part_id); + // The reservation may have been recovered before its parent-directory + // entry was durable; make that durable before manifest publication too. + std::fs::File::open( + path.parent() + .ok_or_else(|| invalid("missing parts parent"))?, + )? + .sync_all()?; + if !self + .manifest + .live_parts() + .iter() + .any(|part| part.part_id == pending.part_id) + { + let size_bytes = std::fs::read_dir(&path)?.try_fold(0u64, |sum, entry| { + let length = entry?.metadata()?.len(); + sum.checked_add(length) + .ok_or_else(|| std::io::Error::other("part size overflow")) + })?; + self.manifest.append_add(PartEntry { + part_id: pending.part_id, + min_ts: pending.start_ms, + max_ts: pending.end_ms, + size_bytes, + })?; + } + current.completed_through_ms = Some( + current + .completed_through_ms + .unwrap_or(0) + .max(pending.end_ms), + ); + current.last_immutable = Some(pending.clone()); + current.pending_immutable = None; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::super::{ + config::SketchStorePersistenceConfig, + manifest::Manifest, + source::{EpochSource, SealedEpochRef}, + }; + use super::*; + use crate::storage_engines::{sketch_db::data::AggKind, types::AggregationType}; + use std::sync::Arc; + + struct EmptySource; + impl EpochSource for EmptySource { + fn list_sealed_epochs(&self) -> Vec { + vec![] + } + fn snapshot_sealed_epoch(&self, _: u64, _: u64) -> PersistResult> { + Ok(None) + } + fn evict_sealed_epoch(&self, _: u64, _: u64) {} + fn approx_memory_bytes(&self) -> usize { + 0 + } + } + fn handle(path: &std::path::Path) -> FlusherHandle { + let mut config = SketchStorePersistenceConfig::with_memory_limit(1024, path.into()); + config.hot_window_ms = None; + config.delete_older_than_ms = None; + FlusherHandle::start( + config, + Arc::new(Manifest::open_or_init(path).unwrap()), + Arc::new(EmptySource), + ) + .unwrap() + } + fn record() -> SidMetaRecord { + SidMetaRecord::new( + 1, + "derived".into(), + vec![], + &AggKind::ExactAgg { + agg_type: AggregationType::Sum, + parameters_canonical: String::new(), + spatial_filter_canonical: String::new(), + }, + 0, + ) + } + fn snapshot() -> EpochSnapshot { + EpochSnapshot { + agg_id: 1, + epoch_id: 0, + min_ts: 10, + max_ts: 20, + approx_bytes: 8, + entries: vec![EpochSnapshotEntry { + start_ts: 10, + end_ts: 20, + label: None, + sketch_type_name: "SumAccumulator".into(), + encoding_tag: 0, + sketch_bytes: vec![1, 2, 3], + }], + } + } + fn reserve(handle: &FlusherHandle, snapshot: &EpochSnapshot) -> ImmutableOutputReservation { + let pending = ImmutableOutputReservation { + part_id: handle.inner.allocate_part_id().unwrap(), + input_digest: [7; 32], + payload_digest: fingerprint(snapshot).unwrap(), + start_ms: 10, + end_ms: 20, + }; + handle + .inner + .sid_metadata + .transaction(|records, metadata| { + let mut record = record(); + record.pending_immutable = Some(pending.clone()); + records.insert("1".into(), record); + metadata.write_records(records) + }) + .unwrap(); + pending + } + #[test] + fn completed_retry_and_concurrent_publications_do_not_duplicate_parts() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + std::thread::scope(|scope| { + let workers: Vec<_> = (0..4) + .map(|_| { + scope.spawn(|| { + handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .unwrap() + }) + }) + .collect(); + let ids: Vec<_> = workers + .into_iter() + .map(|worker| worker.join().unwrap().part_id) + .collect(); + assert!(ids.iter().all(|id| *id == ids[0])); + }); + assert_eq!(handle.manifest().live_parts().len(), 1); + assert!(handle + .publish_immutable_window(&record(), [8; 32], &snapshot()) + .is_err()); + let metadata = handle.inner.sid_metadata.load_strict().unwrap(); + assert_eq!(metadata[0].completed_through_ms, Some(20)); + assert!(metadata[0].pending_immutable.is_none()); + let mut different = snapshot(); + different.entries[0].sketch_bytes.push(4); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &different) + .is_err()); + } + #[test] + fn restart_preserves_reserved_part_and_resumes_without_source() { + let temp = tempfile::tempdir().unwrap(); + let mut first = handle(temp.path()); + let state = snapshot(); + let pending = reserve(&first, &state); + let path = part_dir_path(&temp.path().join("parts"), pending.part_id); + PartWriter::write_part(&path, pending.part_id, &[state]).unwrap(); + first.shutdown(); + drop(first); + let (_, recovery) = super::super::recovery::recover(temp.path()).unwrap(); + assert_eq!(recovery.orphan_parts_removed, 0); + let resumed = handle(temp.path()); + assert!(resumed.inner.allocate_part_id().unwrap() > pending.part_id); + let result = resumed.resume_pending_immutable_window(1).unwrap().unwrap(); + assert_eq!(result.part_id, pending.part_id); + assert_eq!(resumed.manifest().live_parts().len(), 1); + assert!(resumed + .resume_pending_immutable_window(1) + .unwrap() + .is_none()); + } + #[test] + fn pending_missing_part_requires_source_and_stale_upsert_preserves_reservation() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + let pending = reserve(&handle, &snapshot()); + handle.inner.sid_metadata.upsert_all(&[record()]).unwrap(); + assert_eq!( + handle.inner.sid_metadata.load_strict().unwrap()[0].pending_immutable, + Some(pending.clone()) + ); + assert!(handle.resume_pending_immutable_window(1).is_err()); + assert_eq!( + handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .unwrap() + .part_id, + pending.part_id + ); + handle.inner.sid_metadata.upsert_all(&[record()]).unwrap(); + assert!(handle.inner.sid_metadata.load_strict().unwrap()[0] + .pending_immutable + .is_none()); + } + #[test] + fn malformed_metadata_fails_before_part_publication() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + std::fs::write(handle.inner.sid_metadata.path(), b"broken").unwrap(); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .is_err()); + assert!(handle.manifest().live_parts().is_empty()); + } + #[test] + fn failed_manifest_leaves_recoverable_payload_and_no_completion() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + let log = handle.manifest().log_path(); + let backup = log.with_extension("saved"); + std::fs::rename(&log, &backup).unwrap(); + std::fs::create_dir(&log).unwrap(); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .is_err()); + let records = handle.inner.sid_metadata.load_strict().unwrap(); + let pending = records[0].pending_immutable.clone().unwrap(); + assert_eq!(records[0].completed_through_ms, None); + assert!(handle.manifest().live_parts().is_empty()); + std::fs::remove_dir(&log).unwrap(); + std::fs::rename(&backup, &log).unwrap(); + let published = handle.resume_pending_immutable_window(1).unwrap().unwrap(); + assert_eq!(published.part_id, pending.part_id); + assert_eq!(handle.manifest().live_parts().len(), 1); + } +} diff --git a/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs b/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs index c432e07f2..dfdf7b4bc 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/metadata.rs @@ -322,6 +322,10 @@ pub struct SidMetaRecord { /// No further publication may change a window ending at or before this bound. #[serde(default)] pub completed_through_ms: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pending_immutable: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub last_immutable: Option, } impl SidMetaRecord { @@ -347,9 +351,15 @@ impl SidMetaRecord { expires_at_ms: None, removed: false, completed_through_ms: None, + pending_immutable: None, + last_immutable: None, } } + pub(super) fn same_kind(&self, other: &Self) -> bool { + self.agg_kind == other.agg_kind + } + /// Reconstruct the structured [`AggKind`], or `None` for an /// unrecognized sketch kind. pub fn agg_kind(&self) -> Option { @@ -416,6 +426,10 @@ struct SidBindingRec { removed: bool, #[serde(default)] completed_through_ms: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pending_immutable: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + last_immutable: Option, } /// Version-3 normalized sidecar with authoritative catalog provenance. Descriptors appear once and SeriesId bindings hold @@ -487,6 +501,8 @@ impl SdsSidecar { expires_at_ms: record.expires_at_ms, removed: record.removed, completed_through_ms: record.completed_through_ms, + pending_immutable: record.pending_immutable, + last_immutable: record.last_immutable, }, ); } @@ -541,6 +557,8 @@ impl SdsSidecar { expires_at_ms: binding.expires_at_ms, removed: binding.removed, completed_through_ms: binding.completed_through_ms, + pending_immutable: binding.pending_immutable, + last_immutable: binding.last_immutable, }) }) .collect() @@ -646,7 +664,7 @@ impl SidMetadataStore { return Ok(()); } let mut map: HashMap = self - .load()? + .load_strict()? .into_iter() .map(|r| (r.sid.to_string(), r)) .collect(); @@ -657,6 +675,8 @@ impl SidMetadataStore { if let Some(existing) = map.get(&key) { // Lifecycle is monotone for a SeriesId. An older flush snapshot // must not resurrect a retired or removed persisted instance. + next.pending_immutable = existing.pending_immutable.clone(); + next.last_immutable = existing.last_immutable.clone(); next.removed |= existing.removed; next.completed_through_ms = existing.completed_through_ms.max(next.completed_through_ms); @@ -680,6 +700,55 @@ impl SidMetadataStore { self.write_atomic(json.as_bytes()) } + pub fn load_strict(&self) -> PersistResult> { + let bytes = match fs::read(&self.path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(error.into()), + }; + let value: serde_json::Value = serde_json::from_slice(&bytes) + .map_err(|error| PersistError::Format(format!("invalid SID metadata: {error}")))?; + if let Some(version) = value.get("schema_version") { + if !matches!(version.as_u64(), Some(2 | 3)) { + return Err(PersistError::Format( + "unsupported SID metadata version".into(), + )); + } + let sidecar: SdsSidecar = serde_json::from_value(value) + .map_err(|error| PersistError::Format(error.to_string()))?; + sidecar.into_records() + } else { + let records: HashMap = serde_json::from_value(value) + .map_err(|error| PersistError::Format(error.to_string()))?; + Ok(records.into_values().collect()) + } + } + + pub(super) fn transaction( + &self, + operation: impl FnOnce(&mut HashMap, &Self) -> PersistResult, + ) -> PersistResult { + let _writer = self + .writer + .lock() + .map_err(|_| PersistError::Internal("SID metadata writer poisoned".into()))?; + let mut records = self + .load_strict()? + .into_iter() + .map(|r| (r.sid.to_string(), r)) + .collect(); + operation(&mut records, self) + } + pub(super) fn write_records( + &self, + records: &HashMap, + ) -> PersistResult<()> { + let sidecar = SdsSidecar::from_records(records.values().cloned()); + let bytes = + serde_json::to_vec(&sidecar).map_err(|e| PersistError::Serialize(e.to_string()))?; + self.write_atomic(&bytes) + } + fn write_atomic(&self, bytes: &[u8]) -> PersistResult<()> { if let Some(parent) = self.path.parent() { fs::create_dir_all(parent)?; @@ -696,9 +765,7 @@ impl SidMetadataStore { } fs::rename(&tmp, &self.path)?; if let Some(parent) = self.path.parent() { - if let Ok(dir) = File::open(parent) { - let _ = dir.sync_all(); - } + File::open(parent)?.sync_all()?; } Ok(()) } diff --git a/data_plane/src/storage_engines/sketch_db/persistence/mod.rs b/data_plane/src/storage_engines/sketch_db/persistence/mod.rs index 349feb92e..552fe4e03 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/mod.rs @@ -27,6 +27,7 @@ pub mod source; pub mod cache; pub mod flusher; +pub mod immutable_output; pub mod recovery; pub use config::SketchStorePersistenceConfig; diff --git a/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs b/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs index 7fb800946..a21402608 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/recovery.rs @@ -34,7 +34,12 @@ pub fn recover(disk_path: &Path) -> PersistResult<(Manifest, RecoveryReport)> { // Validate every live part by reading its meta.bin header. let mut to_drop: Vec = Vec::new(); - let mut referenced: HashSet = HashSet::new(); + let pending: HashSet = super::metadata::SidMetadataStore::new(disk_path) + .load_strict()? + .into_iter() + .filter_map(|record| record.pending_immutable.map(|pending| pending.part_id)) + .collect(); + let mut referenced = pending.clone(); for entry in manifest.live_parts() { referenced.insert(entry.part_id); let part_dir = super::part::part_dir_path(&parts_root, entry.part_id); @@ -61,6 +66,11 @@ pub fn recover(disk_path: &Path) -> PersistResult<(Manifest, RecoveryReport)> { } } + if to_drop.iter().any(|part_id| pending.contains(part_id)) { + return Err(super::PersistError::Format( + "corrupt reserved immutable part".into(), + )); + } for part_id in to_drop { manifest.append_delete(part_id)?; let dir = super::part::part_dir_path(&parts_root, part_id); From c9d0f088db60eb91b9ba92d75b4376f022f5a317 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:16:59 -0600 Subject: [PATCH 13/29] fix(storage): reject retry after manifest retirement --- .../sketch_db/persistence/immutable_output.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs index 7264f52b8..117b350bc 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -145,6 +145,14 @@ impl FlusherShared { )); } self.validate_reserved_part(record.sid, previous)?; + if !self + .manifest + .live_parts() + .iter() + .any(|part| part.part_id == previous.part_id) + { + return Err(invalid("completed immutable part is no longer published")); + } return Ok(ImmutableWindowPublication { part_id: previous.part_id, already_published: true, @@ -432,6 +440,11 @@ mod tests { assert!(handle .publish_immutable_window(&record(), [7; 32], &different) .is_err()); + let part_id = handle.manifest().live_parts()[0].part_id; + handle.manifest().append_delete(part_id).unwrap(); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .is_err()); } #[test] fn restart_preserves_reserved_part_and_resumes_without_source() { From d02ca36c7417187ce0670bb98ae4d83d7edfd008 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:16:59 -0600 Subject: [PATCH 14/29] fix(storage): reject retry after manifest retirement --- .../sketch_db/persistence/immutable_output.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs index 7264f52b8..117b350bc 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -145,6 +145,14 @@ impl FlusherShared { )); } self.validate_reserved_part(record.sid, previous)?; + if !self + .manifest + .live_parts() + .iter() + .any(|part| part.part_id == previous.part_id) + { + return Err(invalid("completed immutable part is no longer published")); + } return Ok(ImmutableWindowPublication { part_id: previous.part_id, already_published: true, @@ -432,6 +440,11 @@ mod tests { assert!(handle .publish_immutable_window(&record(), [7; 32], &different) .is_err()); + let part_id = handle.manifest().live_parts()[0].part_id; + handle.manifest().append_delete(part_id).unwrap(); + assert!(handle + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .is_err()); } #[test] fn restart_preserves_reserved_part_and_resumes_without_source() { From 2fc455a5d24a847a4047ed01de9fdf0a12070224 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:24:19 -0600 Subject: [PATCH 15/29] docs(sds): separate immutable window section --- docs/design_docs/summary-catalog-sds-architecture.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/design_docs/summary-catalog-sds-architecture.md b/docs/design_docs/summary-catalog-sds-architecture.md index e2fa87306..14b388e5c 100644 --- a/docs/design_docs/summary-catalog-sds-architecture.md +++ b/docs/design_docs/summary-catalog-sds-architecture.md @@ -462,6 +462,7 @@ Installation currently rejects derived inputs so they cannot accidentally receiv raw samples through the legacy metric router. Enabling them requires the immutable maintenance consumer and durable output deduplication protocol; neither raw-table substitution nor treating late correction fragments as new observations is valid. + ### Immutable completed windows Finite Remote Write completion now fences the SummaryStore append boundary, From 1bdd1e497615297bee44959491b679a9440a92df Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:28:47 -0600 Subject: [PATCH 16/29] fix(storage): look up committed lineage before sketch evaluation --- .../sketch_db/persistence/immutable_output.rs | 96 ++++++++++++++++--- 1 file changed, 85 insertions(+), 11 deletions(-) diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs index 117b350bc..468c26f02 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -29,6 +29,22 @@ fn invalid(message: &str) -> PersistError { PersistError::Format(message.into()) } +fn validate_identity(current: &SidMetaRecord, record: &SidMetaRecord) -> PersistResult<()> { + if current.sid != record.sid + || current.removed + || current.retired_at_ms.is_some() + || current.expires_at_ms.is_some() + || current.summary_definition_id != record.summary_definition_id + || current.catalog_generation != record.catalog_generation + || current.metric_name != record.metric_name + || current.group_by_keys != record.group_by_keys + || !current.same_kind(record) + { + return Err(invalid("immutable publication metadata identity changed")); + } + Ok(()) +} + fn fingerprint(snapshot: &EpochSnapshot) -> PersistResult<[u8; 32]> { if snapshot.entries.is_empty() || snapshot.min_ts >= snapshot.max_ts { return Err(invalid("immutable publication requires a nonempty window")); @@ -100,6 +116,45 @@ impl FlusherHandle { } impl FlusherShared { + /// Find the latest committed result before evaluating a potentially + /// randomized sketch again. This never creates or replaces a payload. + pub fn lookup_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + start_ms: u64, + end_ms: u64, + ) -> PersistResult> { + self.sid_metadata.transaction(|records, _| { + let Some(current) = records.get(&record.sid.to_string()) else { + return Ok(None); + }; + validate_identity(current, record)?; + let Some(previous) = ¤t.last_immutable else { + return Ok(None); + }; + if previous.start_ms != start_ms || previous.end_ms != end_ms { + return Ok(None); + } + if previous.input_digest != input_digest { + return Err(invalid("immutable lookup input lineage differs")); + } + self.validate_reserved_part(record.sid, previous)?; + if !self + .manifest + .live_parts() + .iter() + .any(|part| part.part_id == previous.part_id) + { + return Err(invalid("completed immutable part is no longer published")); + } + Ok(Some(ImmutableWindowPublication { + part_id: previous.part_id, + already_published: true, + })) + }) + } + /// Publish a finalized window. A retry must present exactly the reserved /// input and payload. This does not guarantee source availability after GC. pub fn publish_immutable_window( @@ -115,17 +170,7 @@ impl FlusherShared { self.sid_metadata.transaction(|records, metadata| { let key = record.sid.to_string(); let mut current = records.get(&key).cloned().unwrap_or_else(|| record.clone()); - if current.removed - || current.retired_at_ms.is_some() - || current.expires_at_ms.is_some() - || current.summary_definition_id != record.summary_definition_id - || current.catalog_generation != record.catalog_generation - || current.metric_name != record.metric_name - || current.group_by_keys != record.group_by_keys - || !current.same_kind(record) - { - return Err(invalid("immutable publication metadata identity changed")); - } + validate_identity(¤t, record)?; if current .completed_through_ms .is_some_and(|end| snapshot.min_ts < end) @@ -522,4 +567,33 @@ mod tests { assert_eq!(published.part_id, pending.part_id); assert_eq!(handle.manifest().live_parts().len(), 1); } + #[test] + fn committed_lookup_reuses_payload_without_recomputing_randomized_state() { + let temp = tempfile::tempdir().unwrap(); + let mut first = handle(temp.path()); + let publication = first + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .unwrap(); + first.shutdown(); + drop(first); + let reopened = handle(temp.path()); + let found = reopened + .inner + .lookup_immutable_window(&record(), [7; 32], 10, 20) + .unwrap() + .unwrap(); + assert_eq!(found.part_id, publication.part_id); + assert!(reopened + .inner + .lookup_immutable_window(&record(), [8; 32], 10, 20) + .is_err()); + reopened + .manifest() + .append_delete(publication.part_id) + .unwrap(); + assert!(reopened + .inner + .lookup_immutable_window(&record(), [7; 32], 10, 20) + .is_err()); + } } From abb10bae9d65f32137cde6c9e4f4fc055456c8e5 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:28:47 -0600 Subject: [PATCH 17/29] fix(storage): look up committed lineage before sketch evaluation --- .../sketch_db/persistence/immutable_output.rs | 96 ++++++++++++++++--- 1 file changed, 85 insertions(+), 11 deletions(-) diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs index 117b350bc..468c26f02 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -29,6 +29,22 @@ fn invalid(message: &str) -> PersistError { PersistError::Format(message.into()) } +fn validate_identity(current: &SidMetaRecord, record: &SidMetaRecord) -> PersistResult<()> { + if current.sid != record.sid + || current.removed + || current.retired_at_ms.is_some() + || current.expires_at_ms.is_some() + || current.summary_definition_id != record.summary_definition_id + || current.catalog_generation != record.catalog_generation + || current.metric_name != record.metric_name + || current.group_by_keys != record.group_by_keys + || !current.same_kind(record) + { + return Err(invalid("immutable publication metadata identity changed")); + } + Ok(()) +} + fn fingerprint(snapshot: &EpochSnapshot) -> PersistResult<[u8; 32]> { if snapshot.entries.is_empty() || snapshot.min_ts >= snapshot.max_ts { return Err(invalid("immutable publication requires a nonempty window")); @@ -100,6 +116,45 @@ impl FlusherHandle { } impl FlusherShared { + /// Find the latest committed result before evaluating a potentially + /// randomized sketch again. This never creates or replaces a payload. + pub fn lookup_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + start_ms: u64, + end_ms: u64, + ) -> PersistResult> { + self.sid_metadata.transaction(|records, _| { + let Some(current) = records.get(&record.sid.to_string()) else { + return Ok(None); + }; + validate_identity(current, record)?; + let Some(previous) = ¤t.last_immutable else { + return Ok(None); + }; + if previous.start_ms != start_ms || previous.end_ms != end_ms { + return Ok(None); + } + if previous.input_digest != input_digest { + return Err(invalid("immutable lookup input lineage differs")); + } + self.validate_reserved_part(record.sid, previous)?; + if !self + .manifest + .live_parts() + .iter() + .any(|part| part.part_id == previous.part_id) + { + return Err(invalid("completed immutable part is no longer published")); + } + Ok(Some(ImmutableWindowPublication { + part_id: previous.part_id, + already_published: true, + })) + }) + } + /// Publish a finalized window. A retry must present exactly the reserved /// input and payload. This does not guarantee source availability after GC. pub fn publish_immutable_window( @@ -115,17 +170,7 @@ impl FlusherShared { self.sid_metadata.transaction(|records, metadata| { let key = record.sid.to_string(); let mut current = records.get(&key).cloned().unwrap_or_else(|| record.clone()); - if current.removed - || current.retired_at_ms.is_some() - || current.expires_at_ms.is_some() - || current.summary_definition_id != record.summary_definition_id - || current.catalog_generation != record.catalog_generation - || current.metric_name != record.metric_name - || current.group_by_keys != record.group_by_keys - || !current.same_kind(record) - { - return Err(invalid("immutable publication metadata identity changed")); - } + validate_identity(¤t, record)?; if current .completed_through_ms .is_some_and(|end| snapshot.min_ts < end) @@ -522,4 +567,33 @@ mod tests { assert_eq!(published.part_id, pending.part_id); assert_eq!(handle.manifest().live_parts().len(), 1); } + #[test] + fn committed_lookup_reuses_payload_without_recomputing_randomized_state() { + let temp = tempfile::tempdir().unwrap(); + let mut first = handle(temp.path()); + let publication = first + .publish_immutable_window(&record(), [7; 32], &snapshot()) + .unwrap(); + first.shutdown(); + drop(first); + let reopened = handle(temp.path()); + let found = reopened + .inner + .lookup_immutable_window(&record(), [7; 32], 10, 20) + .unwrap() + .unwrap(); + assert_eq!(found.part_id, publication.part_id); + assert!(reopened + .inner + .lookup_immutable_window(&record(), [8; 32], 10, 20) + .is_err()); + reopened + .manifest() + .append_delete(publication.part_id) + .unwrap(); + assert!(reopened + .inner + .lookup_immutable_window(&record(), [7; 32], 10, 20) + .is_err()); + } } From 0bc25d92913869209c76d7229c9cdeff2c8ebe84 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:32:31 -0600 Subject: [PATCH 18/29] feat(precompute): execute immutable summary input windows --- .../precompute_engine/maintenance_runtime.rs | 1030 ++++++++++++++++- .../sketch_db/index/maintenance.rs | 261 +++++ .../storage_engines/sketch_db/index/mod.rs | 61 +- .../summary-catalog-sds-architecture.md | 34 + 4 files changed, 1337 insertions(+), 49 deletions(-) create mode 100644 data_plane/src/storage_engines/sketch_db/index/maintenance.rs diff --git a/data_plane/src/precompute_engine/maintenance_runtime.rs b/data_plane/src/precompute_engine/maintenance_runtime.rs index 3d64b8eeb..1278e979d 100644 --- a/data_plane/src/precompute_engine/maintenance_runtime.rs +++ b/data_plane/src/precompute_engine/maintenance_runtime.rs @@ -13,6 +13,43 @@ use std::collections::{BTreeMap, BTreeSet}; use std::sync::{Arc, Mutex}; type SummaryState = Arc; + +#[derive(Clone)] +enum MaintenanceValue { + Summary { + state: SummaryState, + family: Option, + }, + // A collection is retained until the DAG explicitly reduces it. Evaluating + // the whole DAG once per source pane would change nested reductions. + SummaryWindows { + states: Arc<[(i64, SummaryState)]>, + family: planner_types::post_asap::SummaryFamilyType, + }, + Rows { + values: Vec<(i64, f64)>, + name: String, + }, +} + +impl MaintenanceValue { + fn summary(state: SummaryState) -> Self { + Self::Summary { + state, + family: None, + } + } + + fn state(&self) -> Result<&SummaryState, String> { + match self { + Self::Summary { state, .. } => Ok(state), + Self::SummaryWindows { states, .. } if states.len() == 1 => Ok(&states[0].1), + Self::Rows { .. } | Self::SummaryWindows { .. } => { + Err("maintenance sink requires an explicit reduction to one summary state".into()) + } + } + } +} type PendingOutput = ( Option<(MaterializationCommitKey, u64)>, PrecomputedOutput, @@ -23,30 +60,132 @@ struct OperatorAdapter<'a> { binding: &'a BackendExecutableBinding, source_definition: asap_types::sds::SummaryDefinitionId, source: SummaryState, + configs: &'a [asap_types::aggregation_config::AggregationConfig], + immutable_windows: Option>, } -impl PrecomputeOperatorRegistry for OperatorAdapter<'_> { +impl PrecomputeOperatorRegistry for OperatorAdapter<'_> { type Error = String; - fn materialized_input(&self, node: &ExecutableDagNode) -> Result, String> { - Ok(matches!( + fn materialized_input( + &self, + node: &ExecutableDagNode, + ) -> Result, String> { + if !matches!( self.binding.node(node.id), Some(BackendNodeBinding::Materialization { summary_definition }) if *summary_definition == self.source_definition - ) - .then(|| Arc::clone(&self.source))) + ) { + return Ok(None); + } + let family = node.output_schema.fields.iter().find_map(|field| { + (!matches!( + field.dtype, + planner_types::post_asap::SummaryFamilyType::Plain(_) + )) + .then(|| field.dtype.clone()) + }); + if let Some(states) = &self.immutable_windows { + return Ok(Some(MaintenanceValue::SummaryWindows { + states: Arc::clone(states), + family: family.ok_or("immutable source lacks a summary schema")?, + })); + } + Ok(Some(MaintenanceValue::Summary { + state: Arc::clone(&self.source), + family, + })) } fn execute( &self, node: &ExecutableDagNode, - inputs: &[Arc], - ) -> Result { + inputs: &[Arc], + ) -> Result { match &node.payload { ExecutableOperatorPayload::SummaryMerge => merge_inputs(inputs), - ExecutableOperatorPayload::SummaryAgg { .. } => Err( - "maintenance SummaryAgg requires a typed update evaluator; merging input state does not execute its update expression".into(), - ), + ExecutableOperatorPayload::Value { + operation: planner_types::post_asap::ValueOperation::FinalizeExactAccumulator, + timing: planner_types::post_asap::ExecutionTiming::MaintenanceTime, + } => { + if self.immutable_windows.is_none() { + return Err( + "maintenance finalization requires immutable completed input windows" + .into(), + ); + } + finalize_exact(node, inputs) + } + ExecutableOperatorPayload::SummaryAgg { family, input, .. } => { + let [value] = inputs else { + return Err("maintenance SummaryAgg requires exactly one row input".into()); + }; + let MaintenanceValue::Rows { values, name } = value.as_ref() else { + return Err("maintenance SummaryAgg requires a typed update evaluator; finalize summary state before applying an update".into()); + }; + if self.immutable_windows.is_none() { + return Err( + "maintenance aggregation requires immutable completed input windows".into(), + ); + } + let target = match self.binding.node(node.id) { + Some(BackendNodeBinding::Materialization { summary_definition }) => { + summary_definition + } + _ => { + return Err( + "maintenance SummaryAgg lacks installed materialization binding".into(), + ) + } + }; + let config = self + .configs + .iter() + .find(|config| config.policy_fingerprint() == target.fingerprint()) + .ok_or("maintenance SummaryAgg lacks installed accumulator configuration")?; + let source_config = self + .configs + .iter() + .find(|config| { + config.policy_fingerprint() == self.source_definition.fingerprint() + }) + .ok_or("maintenance input lacks installed source configuration")?; + if config.grouping_labels != source_config.grouping_labels + || config.partitioning != source_config.partitioning + { + return Err( + "maintenance transform requires explicit target window/group routing" + .into(), + ); + } + if config.accumulator_spec().map_err(|e| e.to_string())?.family != *family { + return Err( + "maintenance SummaryAgg family differs from installed configuration".into(), + ); + } + if input.item.is_some() { + return Err( + "keyed maintenance updates require explicit row identity routing".into(), + ); + } + let mut updater = super::accumulator_factory::create_accumulator_updater(config); + if updater.is_keyed() { + return Err("keyed maintenance accumulator requires an item expression".into()); + } + for (timestamp_ms, value) in values { + let weight = evaluate_weight(&input.weight, *value, name)?; + updater.update_single(weight, *timestamp_ms); + } + let timestamp = values + .iter() + .map(|(timestamp, _)| *timestamp) + .max() + .ok_or("maintenance aggregation has no input rows")?; + Ok(MaintenanceValue::SummaryWindows { + states: vec![(timestamp, Arc::from(updater.into_accumulator()))].into(), + family: family.clone(), + }) + } payload => Err(format!( "maintenance operator {:?} has no summary-state implementation", payload.operator() @@ -55,21 +194,407 @@ impl PrecomputeOperatorRegistry for OperatorAdapter<'_> { } } -fn merge_inputs(inputs: &[Arc]) -> Result { - let Some(first) = inputs.first() else { +fn evaluate_weight( + expression: &planner_types::post_asap::SummaryInputExpr, + value: f64, + name: &str, +) -> Result { + use planner_types::{post_asap::SummaryInputExpr, pre_asap::ColumnRef}; + let weight = match expression { + SummaryInputExpr::Constant(value) => *value, + SummaryInputExpr::Column(ColumnRef::SampleValue) => value, + SummaryInputExpr::Column(ColumnRef::Named(column)) if column == name => value, + _ => { + return Err("maintenance update does not resolve against the supplied typed row".into()) + } + }; + if !weight.is_finite() { + return Err("maintenance update weight is not finite".into()); + } + Ok(weight) +} + +fn finalize_exact( + node: &ExecutableDagNode, + inputs: &[Arc], +) -> Result { + use planner_types::post_asap::{ExactKind, SummaryFamilyType}; + let [input] = inputs else { + return Err("exact maintenance finalization requires one summary input".into()); + }; + let (states, family): (Vec<(i64, &SummaryState)>, _) = match input.as_ref() { + MaintenanceValue::Summary { + state, + family: Some(family), + } => { + // Single-state merge execution has no row timestamp. Immutable + // execution supplies SummaryWindows with the actual window ends. + (vec![(0, state)], family) + } + MaintenanceValue::SummaryWindows { states, family } => ( + states.iter().map(|(time, state)| (*time, state)).collect(), + family, + ), + _ => { + return Err("exact maintenance finalization requires a typed exact accumulator".into()) + } + }; + let SummaryFamilyType::ExactAggregate(kind, _) = family else { + return Err("exact maintenance finalization requires a typed exact accumulator".into()); + }; + let statistic = match kind { + ExactKind::Sum => asap_types::Statistic::Sum, + ExactKind::Count => asap_types::Statistic::Count, + _ => { + return Err( + "exact maintenance readout requires explicit operator/time semantics".into(), + ) + } + }; + let [field] = node.output_schema.fields.as_slice() else { + return Err("exact maintenance finalization requires one scalar output column".into()); + }; + if !matches!(field.dtype, SummaryFamilyType::Plain(_)) { + return Err("exact maintenance finalization output must be a plain value".into()); + } + let values = states + .into_iter() + .map(|(timestamp, state)| { + let value = state + .query_statistic(statistic, &None, &std::collections::HashMap::new()) + .map_err(|error| error.to_string())?; + if !value.is_finite() { + return Err("exact maintenance finalization produced a non-finite value".into()); + } + Ok((timestamp, value)) + }) + .collect::, String>>()?; + Ok(MaintenanceValue::Rows { + values, + name: field.name.clone(), + }) +} + +fn merge_inputs(inputs: &[Arc]) -> Result { + let mut states = Vec::new(); + let mut family = None; + let mut end_timestamp = None; + for input in inputs { + let input_family = match input.as_ref() { + MaintenanceValue::Summary { state, family } => { + states.push(state); + family.as_ref() + } + MaintenanceValue::SummaryWindows { + states: windows, + family, + } => { + states.extend(windows.iter().map(|(_, state)| state)); + end_timestamp = end_timestamp + .into_iter() + .chain(windows.iter().map(|(time, _)| *time)) + .max(); + Some(family) + } + MaintenanceValue::Rows { .. } => return Err("summary merge cannot consume rows".into()), + }; + if let Some(input_family) = input_family { + if family.as_ref().is_some_and(|family| family != input_family) { + return Err("summary merge input families differ".into()); + } + family = Some(input_family.clone()); + } + } + let Some((first, rest)) = states.split_first() else { return Err("summary maintenance node has no input state".into()); }; - let mut merged: Box = (**first).clone_boxed_core(); - for input in &inputs[1..] { + let mut merged = first.clone_boxed_core(); + for state in rest { merged = merged - .merge_with(input.as_ref().as_ref()) - .map_err(|error| error.to_string())?; + .merge_with(state.as_ref()) + .map_err(|e| e.to_string())?; + } + if let Some(timestamp) = end_timestamp { + return Ok(MaintenanceValue::SummaryWindows { + states: vec![(timestamp, Arc::from(merged))].into(), + family: family.ok_or("merged immutable state lacks a family")?, + }); + } + Ok(MaintenanceValue::Summary { + state: Arc::from(merged), + family, + }) +} + +/// Evaluate one installed maintenance sink over an immutable physical source +/// incarnation. Durable publication is a separate existing-store transaction; +/// the in-memory scheduler cache here never claims durable exactly-once writes. +fn prepare_frozen_maintenance_sink( + installed: &asap_types::executable_plan::InstalledPostAsapDag, + configs: &[asap_types::PrecomputeMaterialization], + sink: PostAsapNodeId, + input: &crate::storage_engines::sketch_db::index::FrozenExactWindows, + output_window: (u64, u64), +) -> Result< + ( + planner_types::post_asap::ExecutableDag, + MaterializationCommitKey, + Vec<(i64, SummaryState)>, + ), + String, +> { + installed.validate()?; + let target = match installed.binding.node(sink) { + Some(BackendNodeBinding::Materialization { summary_definition }) => *summary_definition, + _ => return Err("immutable sink lacks a materialization binding".into()), + }; + let config = configs + .iter() + .find(|config| config.policy_fingerprint() == target.fingerprint()) + .ok_or("immutable sink lacks its installed configuration")?; + let expected_input = config + .derived_input + .as_ref() + .ok_or("immutable sink is not a derived materialization")?; + if expected_input.inputs != BTreeSet::from([input.definition]) { + return Err("immutable sink requires synchronized input definitions".into()); + } + if output_window.0 >= output_window.1 + || output_window.1 - output_window.0 != config.stored_window_ms() + || input + .windows + .keys() + .any(|(start, end)| *start < output_window.0 || *end > output_window.1) + { + return Err("immutable inputs do not fit the installed output window".into()); + } + let dag = installed.document.decode()?; + let producers: Vec<_> = dag + .edges + .iter() + .filter(|edge| edge.consumer == sink) + .collect(); + let [producer] = producers.as_slice() else { + return Err("immutable sink requires one input relation".into()); + }; + let frontiers = installed + .binding + .nodes + .iter() + .filter_map(|(node, binding)| match binding { + BackendNodeBinding::Materialization { summary_definition } + if *summary_definition == input.definition => + { + Some((*node, *summary_definition)) + } + _ => None, + }) + .collect(); + let actual_input = asap_types::derived_input::DerivedInputIdentity::from_dag( + &installed.document, + producer.producer, + &frontiers, + )?; + if &actual_input != expected_input { + return Err("immutable sink input program differs from its catalog identity".into()); + } + let mut lineage = Sha256::new(); + lineage.update(b"immutable-maintenance-input-v1"); + lineage.update(input.sid.to_be_bytes()); + lineage.update( + serde_json::to_vec(&( + &input.definition, + &input.generation, + &input.group, + expected_input, + )) + .map_err(|error| error.to_string())?, + ); + let mut states = Vec::with_capacity(input.windows.len()); + for ((start, end), state) in &input.windows { + lineage.update(start.to_be_bytes()); + lineage.update(end.to_be_bytes()); + let bytes = state.serialize_to_bytes(); + lineage.update((bytes.len() as u64).to_be_bytes()); + lineage.update(&bytes); + states.push((*end as i64, Arc::clone(state))); } - Ok(Arc::from(merged)) + let digest: [u8; 32] = lineage.finalize().into(); + let key = MaterializationCommitKey { + plan_id: input.generation.plan_id, + plan_version: input.generation.plan_version, + summary_definition: target, + window_start_ms: i64::try_from(output_window.0) + .map_err(|_| "output window exceeds timestamp range")?, + window_end_ms: i64::try_from(output_window.1) + .map_err(|_| "output window exceeds timestamp range")?, + input_lineage: digest.to_vec(), + }; + Ok((dag, key, states)) +} + +fn execute_prepared_frozen_sink( + installed: &asap_types::executable_plan::InstalledPostAsapDag, + configs: &[asap_types::PrecomputeMaterialization], + sink: PostAsapNodeId, + input: &crate::storage_engines::sketch_db::index::FrozenExactWindows, + dag: &planner_types::post_asap::ExecutableDag, + key: MaterializationCommitKey, + states: Vec<(i64, SummaryState)>, +) -> Result { + let first = states.first().ok_or("immutable input is empty")?; + let adapter = OperatorAdapter { + binding: &installed.binding, + source_definition: input.definition, + source: Arc::clone(&first.1), + configs, + immutable_windows: Some(states.into()), + }; + let value = execute_precompute_sink( + dag, + &installed.binding, + sink, + key, + &adapter, + &CommitRegistry::default(), + ) + .map_err(schedule_error)?; + Ok(Arc::clone(value.state()?)) +} + +#[cfg(test)] +fn evaluate_frozen_maintenance_sink( + installed: &asap_types::executable_plan::InstalledPostAsapDag, + configs: &[asap_types::PrecomputeMaterialization], + sink: PostAsapNodeId, + input: &crate::storage_engines::sketch_db::index::FrozenExactWindows, + output_window: (u64, u64), +) -> Result<(SummaryState, [u8; 32]), String> { + let (dag, key, states) = + prepare_frozen_maintenance_sink(installed, configs, sink, input, output_window)?; + let digest = key + .input_lineage + .as_slice() + .try_into() + .map_err(|_| "invalid input digest")?; + let state = execute_prepared_frozen_sink(installed, configs, sink, input, &dag, key, states)?; + Ok((state, digest)) +} + +/// Execute an installed, single-population maintenance subDAG from frozen +/// base panes and publish its complete output through the durable part path. +/// This initial entry point accepts non-overlapping output windows; sliding +/// replacement and cross-population shuffles require their own scheduling +/// proof and are rejected, rather than treating corrections as observations. +#[allow(clippy::too_many_arguments)] +pub fn execute_completed_maintenance( + store: &crate::storage_engines::sketch_db::index::SketchStore, + installed: &asap_types::executable_plan::InstalledPostAsapDag, + configs: &[asap_types::PrecomputeMaterialization], + sink: planner_types::post_asap::PostAsapNodeId, + source_sid: u64, + target_sid: u64, + window: (u64, u64), + group: &BTreeMap, +) -> Result { + use asap_types::executable_plan::BackendNodeBinding; + let target = match installed.binding.node(sink) { + Some(BackendNodeBinding::Materialization { summary_definition }) => *summary_definition, + _ => return Err("maintenance sink lacks an installed output identity".into()), + }; + let target_config = configs + .iter() + .find(|config| config.policy_fingerprint() == target.fingerprint()) + .ok_or("maintenance output configuration is absent")?; + let derived = target_config + .derived_input + .as_ref() + .ok_or("maintenance output has no derived input")?; + if derived.inputs.len() != 1 { + return Err("maintenance execution requires synchronized multi-source scheduling".into()); + } + let source = *derived.inputs.first().unwrap(); + let source_config = configs + .iter() + .find(|config| config.policy_fingerprint() == source.fingerprint()) + .ok_or("maintenance source configuration is absent")?; + let source_width = source_config.stored_window_ms(); + let target_width = target_config.stored_window_ms(); + let origin = source_config.pane_origin_ms.unwrap_or(0); + if source_width == 0 + || target_width == 0 + || window.0 >= window.1 + || window.1 - window.0 != target_width + || target_width % source_width != 0 + || target_width / source_width > 65_536 + || source_config + .slide_interval + .checked_mul(1000) + .is_none_or(|slide| slide < source_width) + || target_config + .slide_interval + .checked_mul(1000) + .is_none_or(|slide| slide < target_width) + || window.1 > i64::MAX as u64 + || (window.0 as i128 - origin as i128).rem_euclid(source_width as i128) != 0 + || (window.0 as i128 - target_config.pane_origin_ms.unwrap_or(0) as i128) + .rem_euclid(target_width as i128) + != 0 + { + return Err("maintenance window requires unsupported overlap, phase, or extent".into()); + } + let expected = (0..target_width / source_width) + .map(|index| { + let start = window.0 + index * source_width; + (start, start + source_width) + }) + .collect(); + let generation = store + .active_catalog_generation() + .ok_or("maintenance requires an authoritative catalog")?; + let frozen = + store.read_frozen_exact_windows(source_sid, source, &generation, &expected, group)?; + let (dag, key, states) = + prepare_frozen_maintenance_sink(installed, configs, sink, &frozen, window)?; + let digest = key + .input_lineage + .as_slice() + .try_into() + .map_err(|_| "invalid input digest")?; + if store.lookup_frozen_maintenance_output(target_sid, target_config, &frozen, digest, window)? { + return Ok(false); + } + let state = execute_prepared_frozen_sink(installed, configs, sink, &frozen, &dag, key, states)?; + let mut output = crate::storage_engines::types::PrecomputedOutput::new( + window.0, + window.1, + Some(crate::storage_engines::types::KeyByLabelValues { + labels: target_config + .grouping_labels + .iter() + .map(|name| { + group + .get(name) + .cloned() + .ok_or("maintenance population is missing an output grouping key") + }) + .collect::, _>>()?, + }), + target.fingerprint(), + ); + output.catalog_generation = Some(generation); + store.publish_frozen_maintenance_output( + target_sid, + target_config, + &output, + state.as_ref(), + &frozen, + digest, + ) } struct CommittedState { - value: Option>, + value: Option>, published: bool, } @@ -245,13 +770,13 @@ impl CommitRegistry { } } -impl IdempotentCommitSink for CommitRegistry { +impl IdempotentCommitSink for CommitRegistry { type Error = String; fn get( &self, key: &MaterializationCommitKey, - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { let state = self.0.lock().map_err(|_| "commit registry poisoned")?; state.validate_key(key)?; Ok(state @@ -263,8 +788,8 @@ impl IdempotentCommitSink for CommitRegistry { fn commit_if_absent( &self, key: MaterializationCommitKey, - value: Arc, - ) -> Result, Self::Error> { + value: Arc, + ) -> Result, Self::Error> { let mut commits = self.0.lock().map_err(|_| "commit registry poisoned")?; commits.validate_key(&key)?; let committed = commits @@ -347,6 +872,8 @@ impl MaintenanceDagSink { binding: &installed.binding, source_definition, source: Arc::clone(&source), + configs: &plan.precompute_plan.materializations, + immutable_windows: None, }; for sink_node in &installed.binding.precompute_sinks { if !depends_on_any(&dag, *sink_node, &source_nodes) { @@ -435,7 +962,7 @@ impl MaintenanceDagSink { derived.push(( Some((key, horizon_ms)), target_output, - value.as_ref().as_ref().clone_boxed_core(), + value.state()?.clone_boxed_core(), )); } } @@ -644,6 +1171,443 @@ mod tests { Arc::new(accumulator) } + #[test] + fn finalized_summary_update_builds_a_different_installed_family() { + use planner_types::post_asap::{ + ExactKind, ExactParams, GroupingStrategy, SummaryFamilyType, SummaryField, + SummaryInputExpr, SummaryUpdate, + }; + use planner_types::pre_asap::{DataType, Reduction}; + let mut snapshot: serde_json::Value = serde_json::from_str(include_str!( + "../../../docs/examples/asapquery-planning-snapshot.json" + )) + .unwrap(); + snapshot["query_workload"]["repeating_queries"][0]["query"] = + "sum(sum_over_time(m[1m]))".into(); + let snapshot: control_plane::physical::compiler::BackendLocalPlanningSnapshot = + serde_json::from_value(snapshot).unwrap(); + let bundle = snapshot.compile().unwrap(); + let mut source_config = bundle.precompute_plan.materializations[0].clone(); + source_config.window_size = 2; + source_config.slide_interval = 2; + source_config.window_layout = + asap_types::aggregation_config::WindowMaterializationLayout::Pane { pane_secs: 1 }; + let source_definition = source_config.policy_fingerprint().into(); + let mut target_config = source_config.clone(); + target_config.window_layout = + asap_types::aggregation_config::WindowMaterializationLayout::FullWindow; + target_config.aggregation_type = asap_types::AggregationType::DatasketchesKLL; + target_config.aggregation_sub_type = "quantile".into(); + target_config + .parameters + .insert("k".into(), serde_json::json!(200)); + let target = target_config.policy_fingerprint().into(); + let target_family = target_config.accumulator_spec().unwrap().family; + let configs = [source_config, target_config]; + let binding = BackendExecutableBinding { + nodes: BTreeMap::from([ + ( + PostAsapNodeId(1), + BackendNodeBinding::Materialization { + summary_definition: source_definition, + }, + ), + (PostAsapNodeId(2), BackendNodeBinding::MaintenanceInput), + ( + PostAsapNodeId(3), + BackendNodeBinding::Materialization { + summary_definition: target, + }, + ), + ]), + query_sink: PostAsapNodeId(3), + query_plan_sink: control_plane::query_plan::QueryNodeId(3), + precompute_sinks: vec![PostAsapNodeId(3)], + }; + let adapter = OperatorAdapter { + binding: &binding, + source_definition, + source: sum(7.0), + configs: &configs, + immutable_windows: Some(vec![(1000, sum(7.0))].into()), + }; + let mut read = node(2); + read.payload = ExecutableOperatorPayload::Value { + operation: planner_types::post_asap::ValueOperation::FinalizeExactAccumulator, + timing: planner_types::post_asap::ExecutionTiming::MaintenanceTime, + }; + read.output_schema.fields = vec![SummaryField { + name: "value".into(), + dtype: SummaryFamilyType::Plain(DataType::Float64), + nullable: false, + }]; + let source = Arc::new(MaintenanceValue::Summary { + state: sum(7.0), + family: Some(SummaryFamilyType::ExactAggregate( + ExactKind::Sum, + ExactParams::Sum, + )), + }); + let row = adapter.execute(&read, &[source]).unwrap(); + let mut aggregate = node(3); + aggregate.payload = ExecutableOperatorPayload::SummaryAgg { + family: target_family, + input: SummaryUpdate { + item: None, + weight: SummaryInputExpr::Constant(3.0), + weight_domain: Default::default(), + }, + reduction: Reduction::by(vec![]), + grouping: GroupingStrategy::default(), + }; + let result = adapter.execute(&aggregate, &[Arc::new(row)]).unwrap(); + let mut kwargs = std::collections::HashMap::new(); + kwargs.insert("quantile".into(), "0.5".into()); + assert_eq!( + result + .state() + .unwrap() + .query_statistic(asap_types::Statistic::Quantile, &None, &kwargs) + .unwrap(), + 3.0 + ); + // Exercise the same registry through the production topological + // scheduler, including the precomputed source frontier and commit. + let mut source_node = node(1); + source_node.output_schema.fields = vec![SummaryField { + name: "state".into(), + dtype: SummaryFamilyType::ExactAggregate(ExactKind::Sum, ExactParams::Sum), + nullable: false, + }]; + read.operator = read.payload.operator(); + read.output_state = planner_types::post_asap::ExecutionDataState::MAINTENANCE_ROWS; + aggregate.operator = aggregate.payload.operator(); + aggregate.output_schema.fields = vec![SummaryField { + name: "state".into(), + dtype: configs[1].accumulator_spec().unwrap().family, + nullable: false, + }]; + let mut query = node(4); + query.output_state = planner_types::post_asap::ExecutionDataState::READ_ROWS; + let mut first_edge = edge(1, 2); + first_edge.intermediate_schema = source_node.output_schema.clone(); + let mut second_edge = edge(2, 3); + second_edge.intermediate_schema = read.output_schema.clone(); + second_edge.data_state = read.output_state; + let mut query_edge = edge(3, 4); + query_edge.intermediate_schema = aggregate.output_schema.clone(); + let dag = ExecutableDag { + nodes: vec![source_node, read, aggregate, query], + edges: vec![first_edge, second_edge, query_edge], + root: PostAsapNodeId(4), + }; + let mut scheduled_binding = binding.clone(); + scheduled_binding.nodes.insert( + PostAsapNodeId(1), + BackendNodeBinding::Materialization { + summary_definition: source_definition, + }, + ); + scheduled_binding + .nodes + .insert(PostAsapNodeId(2), BackendNodeBinding::MaintenanceInput); + scheduled_binding.nodes.insert( + PostAsapNodeId(4), + BackendNodeBinding::Query { + query_node: control_plane::query_plan::QueryNodeId(4), + }, + ); + scheduled_binding.query_sink = PostAsapNodeId(4); + scheduled_binding.query_plan_sink = control_plane::query_plan::QueryNodeId(4); + let scheduled_adapter = OperatorAdapter { + binding: &scheduled_binding, + ..adapter + }; + let key = MaterializationCommitKey { + plan_id: 1, + plan_version: 1, + summary_definition: target, + window_start_ms: 0, + window_end_ms: 1000, + input_lineage: vec![1], + }; + let committed = execute_precompute_sink( + &dag, + &scheduled_binding, + PostAsapNodeId(3), + key, + &scheduled_adapter, + &CommitRegistry::default(), + ) + .unwrap(); + assert_eq!( + committed + .state() + .unwrap() + .query_statistic(asap_types::Statistic::Quantile, &None, &kwargs) + .unwrap(), + 3.0 + ); + // The real store provides the completion proof. Execute, persist, + // restart, and retry the same installed subDAG without additive append. + use crate::storage_engines::sketch_db::index::{ + persistence::config::SketchStorePersistenceConfig, SketchStore, + }; + use asap_types::executable_plan::{InstalledPostAsapDag, OwnedPostAsapDag}; + let document = OwnedPostAsapDag::from_executable("immutable-chain".into(), &dag).unwrap(); + let mut durable_configs = configs.to_vec(); + durable_configs[1].derived_input = Some( + asap_types::derived_input::DerivedInputIdentity::from_dag( + &document, + PostAsapNodeId(2), + &BTreeMap::from([(PostAsapNodeId(1), source_definition)]), + ) + .unwrap(), + ); + let mut durable_binding = scheduled_binding.clone(); + durable_binding.nodes.insert( + PostAsapNodeId(3), + BackendNodeBinding::Materialization { + summary_definition: durable_configs[1].policy_fingerprint().into(), + }, + ); + let installed = InstalledPostAsapDag { + document, + binding: durable_binding, + }; + let catalog = Arc::new( + asap_types::summary_catalog::SummaryCatalog::from_materializations( + 1, + 1, + &durable_configs, + ) + .unwrap(), + ); + let directory = tempfile::tempdir().unwrap(); + let persistence_config = || { + let mut config = SketchStorePersistenceConfig::with_memory_limit( + 1 << 24, + directory.path().to_path_buf(), + ); + config.delete_older_than_ms = None; + config.hot_window_ms = None; + config.flush_interval = std::time::Duration::from_millis(5); + config + }; + let expected = BTreeSet::from([(0, 1000), (1000, 2000)]); + let store = Arc::new(SketchStore::new()); + store.install_summary_catalog(Arc::clone(&catalog)).unwrap(); + let mut persistence = store.start_persistence(persistence_config()).unwrap(); + let generation = store.active_catalog_generation().unwrap(); + for ((start, end), value) in [((0, 1000), 2.0), ((1000, 2000), 7.0)] { + let coordinate = asap_types::sds::SummaryInstanceCoordinates { + summary_definition_id: source_definition, + time_range: asap_types::sds::HalfOpenTimeRange { + start_ms: start, + end_ms: end, + }, + group_values: BTreeMap::new(), + }; + let revision = store + .admit_summary_updates(&generation, BTreeSet::from([coordinate.clone()])) + .unwrap(); + let mut output = PrecomputedOutput::new( + start as u64, + end as u64, + None, + durable_configs[0].policy_fingerprint(), + ); + output.catalog_generation = Some(Arc::clone(&generation)); + store + .publish_admitted_summary_update( + &generation, + &coordinate, + revision, + revision, + 120_000, + || { + store.ingest_precompute_with_series_id( + 600, + &durable_configs[0], + &output, + sum(value).as_ref(), + ) + }, + ) + .unwrap(); + } + assert!(store + .read_frozen_exact_windows( + 600, + source_definition, + &generation, + &expected, + &BTreeMap::new() + ) + .is_err()); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !store.seal_finite_summary_input(&generation).unwrap() { + assert!(std::time::Instant::now() < deadline); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let frozen = store + .read_frozen_exact_windows( + 600, + source_definition, + &generation, + &expected, + &BTreeMap::new(), + ) + .unwrap(); + let (result, digest) = evaluate_frozen_maintenance_sink( + &installed, + &durable_configs, + PostAsapNodeId(3), + &frozen, + (0, 2000), + ) + .unwrap(); + let mut output = + PrecomputedOutput::new(0, 2000, None, durable_configs[1].policy_fingerprint()); + output.catalog_generation = Some(Arc::clone(&generation)); + assert!(execute_completed_maintenance( + &store, + &installed, + &durable_configs, + PostAsapNodeId(3), + 600, + 601, + (0, 2000), + &BTreeMap::new() + ) + .unwrap()); + assert!(!execute_completed_maintenance( + &store, + &installed, + &durable_configs, + PostAsapNodeId(3), + 600, + 601, + (0, 2000), + &BTreeMap::new() + ) + .unwrap()); + assert_eq!( + result + .query_statistic(asap_types::Statistic::Quantile, &None, &kwargs) + .unwrap(), + 3.0 + ); + let mut correction = + PrecomputedOutput::new(0, 1000, None, durable_configs[0].policy_fingerprint()); + correction.catalog_generation = Some(Arc::clone(&generation)); + assert!(store + .ingest_precompute_with_series_id( + 600, + &durable_configs[0], + &correction, + sum(100.0).as_ref() + ) + .is_none()); + persistence.shutdown(); + drop(store); + let restored = Arc::new(SketchStore::new()); + restored.install_summary_catalog(catalog).unwrap(); + let mut persistence = restored.start_persistence(persistence_config()).unwrap(); + let frozen = restored + .read_frozen_exact_windows( + 600, + source_definition, + &generation, + &expected, + &BTreeMap::new(), + ) + .unwrap(); + let (result, replay_digest) = evaluate_frozen_maintenance_sink( + &installed, + &durable_configs, + PostAsapNodeId(3), + &frozen, + (0, 2000), + ) + .unwrap(); + assert_eq!(digest, replay_digest); + assert!(!execute_completed_maintenance( + &restored, + &installed, + &durable_configs, + PostAsapNodeId(3), + 600, + 601, + (0, 2000), + &BTreeMap::new() + ) + .unwrap()); + assert!(restored + .ingest_precompute_with_series_id( + 600, + &durable_configs[0], + &correction, + sum(100.0).as_ref() + ) + .is_none()); + let target_entries = persistence + .manifest + .live_parts() + .iter() + .map(|part| { + let path = crate::storage_engines::sketch_db::persistence::part::part_dir_path( + &persistence.parts_root, + part.part_id, + ); + crate::storage_engines::sketch_db::persistence::part::PartReader::open(&path) + .unwrap() + .index_records() + .into_iter() + .filter(|entry| entry.agg_id == 601) + .count() + }) + .sum::(); + assert_eq!(target_entries, 1); + persistence.shutdown(); + assert!(evaluate_weight( + &SummaryInputExpr::Column(planner_types::pre_asap::ColumnRef::Named("missing".into())), + 7.0, + "value" + ) + .is_err()); + } + + #[test] + fn finalization_preserves_windows_until_an_explicit_merge() { + use planner_types::post_asap::{ExactKind, ExactParams, SummaryFamilyType, SummaryField}; + let family = SummaryFamilyType::ExactAggregate(ExactKind::Sum, ExactParams::Sum); + let inputs = Arc::new(MaintenanceValue::SummaryWindows { + states: vec![(1_000, sum(2.0)), (2_000, sum(7.0))].into(), + family, + }); + let mut read = node(2); + read.output_schema.fields = vec![SummaryField { + name: "value".into(), + dtype: SummaryFamilyType::Plain(planner_types::pre_asap::DataType::Float64), + nullable: false, + }]; + let MaintenanceValue::Rows { values, .. } = + finalize_exact(&read, &[inputs.clone()]).unwrap() + else { + panic!("expected finalized rows") + }; + assert_eq!(values, vec![(1_000, 2.0), (2_000, 7.0)]); + // Merge is a semantic DAG operation, not an implicit batch optimization. + // Finalizing after it emits exactly one value instead of two updates. + let merged = Arc::new(merge_inputs(&[inputs]).unwrap()); + let MaintenanceValue::Rows { values, .. } = finalize_exact(&read, &[merged]).unwrap() + else { + panic!("expected finalized row") + }; + assert_eq!(values, vec![(2_000, 9.0)]); + } + #[test] fn summary_aggregation_does_not_silently_reuse_input_family() { use planner_types::post_asap::{ @@ -661,6 +1625,8 @@ mod tests { binding: &binding, source_definition: definition(1), source: sum(7.0), + configs: &[], + immutable_windows: None, }; let mut aggregate = node(1); aggregate.operator = ExecutableOperator::SummaryAgg; @@ -670,7 +1636,7 @@ mod tests { reduction: Reduction::by(vec![]), grouping: GroupingStrategy::default(), }; - let error = adapter.execute(&aggregate, &[Arc::new(sum(7.0))]); + let error = adapter.execute(&aggregate, &[Arc::new(MaintenanceValue::summary(sum(7.0)))]); assert!(matches!(error, Err(reason) if reason.contains("typed update evaluator"))); } @@ -689,7 +1655,7 @@ mod tests { let fast = key(1000); commits.begin_batch([1; 32]).unwrap(); commits - .commit_if_absent(fast.clone(), Arc::new(sum(2.0))) + .commit_if_absent(fast.clone(), Arc::new(MaintenanceValue::summary(sum(2.0)))) .unwrap(); commits.publish(&fast, || Ok(())).unwrap(); commits.complete_batch([1; 32], &[(fast, 30)]).unwrap(); @@ -698,7 +1664,7 @@ mod tests { commits.begin_batch([2; 32]).unwrap(); commits.pin_admitted(&slow).unwrap(); commits - .commit_if_absent(slow.clone(), Arc::new(sum(3.0))) + .commit_if_absent(slow.clone(), Arc::new(MaintenanceValue::summary(sum(3.0)))) .unwrap(); commits.publish(&slow, || Ok(())).unwrap(); commits @@ -725,7 +1691,7 @@ mod tests { let key = key(end); commits.begin_batch([0; 32]).unwrap(); commits - .commit_if_absent(key.clone(), Arc::new(sum(2.0))) + .commit_if_absent(key.clone(), Arc::new(MaintenanceValue::summary(sum(2.0)))) .unwrap(); commits.publish(&key, || Ok(())).unwrap(); commits.complete_batch([0; 32], &[(key, 30)]).unwrap(); @@ -742,7 +1708,7 @@ mod tests { assert!(commits.is_published(&key(980)).unwrap()); commits.0.lock().unwrap().generation = Some((7, 2)); assert!(commits - .commit_if_absent(key(1_000), Arc::new(sum(2.0))) + .commit_if_absent(key(1_000), Arc::new(MaintenanceValue::summary(sum(2.0)))) .is_err()); } @@ -762,7 +1728,7 @@ mod tests { }; assert!(!commits.is_published(&key).unwrap()); commits - .commit_if_absent(key.clone(), Arc::new(sum(2.0))) + .commit_if_absent(key.clone(), Arc::new(MaintenanceValue::summary(sum(2.0)))) .unwrap(); commits.publish(&key, || Ok(())).unwrap(); } @@ -983,6 +1949,8 @@ mod tests { binding: &binding, source_definition: definition(1), source, + configs: &[], + immutable_windows: None, }; let commits = CommitRegistry::default(); let key = MaterializationCommitKey { @@ -1002,7 +1970,7 @@ mod tests { &commits, ) .unwrap(); - assert_eq!(result.as_ref().aux_stats().sum, Some(4.0)); + assert_eq!(result.state().unwrap().aux_stats().sum, Some(4.0)); commits.publish(&key, || Ok(())).unwrap(); assert!( commits.get(&key).unwrap().is_none(), @@ -1055,6 +2023,8 @@ mod tests { binding: &binding, source_definition: definition(1), source: sum(2.0), + configs: &[], + immutable_windows: None, }; let commits = CommitRegistry::default(); let key = MaterializationCommitKey { diff --git a/data_plane/src/storage_engines/sketch_db/index/maintenance.rs b/data_plane/src/storage_engines/sketch_db/index/maintenance.rs new file mode 100644 index 000000000..dba5700e8 --- /dev/null +++ b/data_plane/src/storage_engines/sketch_db/index/maintenance.rs @@ -0,0 +1,261 @@ +//! Immutable maintenance inputs read from the existing durable summary tier. +//! +//! Unlike query fallback helpers, this path must propagate corrupt/missing part +//! errors: silently omitting a source window would permanently corrupt an outer +//! summary. Completion is an admission barrier, not merely an emitted flag. +use super::*; +use crate::storage_engines::types::AggregateCore; + +pub(crate) struct FrozenExactWindows { + pub(crate) sid: u64, + pub(crate) definition: SummaryDefinitionId, + pub(crate) generation: Arc, + pub(crate) group: BTreeMap, + pub(crate) windows: BTreeMap<(u64, u64), Arc>, +} + +impl SketchStore { + pub(crate) fn read_frozen_exact_windows( + &self, + sid: u64, + definition: SummaryDefinitionId, + generation: &Arc, + expected_windows: &BTreeSet<(u64, u64)>, + group: &BTreeMap, + ) -> Result { + let start_ms = expected_windows + .iter() + .map(|window| window.0) + .min() + .ok_or("immutable input has no requested windows")?; + let end_ms = expected_windows + .iter() + .map(|window| window.1) + .max() + .unwrap(); + if expected_windows.iter().any(|(start, end)| start >= end) || end_ms > i64::MAX as u64 { + return Err("invalid immutable input window".into()); + } + self.validate_routed_catalog_generation(Some(generation.as_ref()))?; + // Do not retain either lock while opening parts. The immutable frontier + // is monotone, and final publication rechecks the catalog incarnation. + let keys = { + let bindings = self + .instances + .read() + .map_err(|_| "instance registry poisoned")?; + let binding = bindings.get(&sid).ok_or("immutable input SID is absent")?; + if binding.metadata.policy_fp != definition.fingerprint() + || binding.catalog_generation.as_deref() != Some(generation.as_ref()) + || binding.metadata.status() == AggStatus::Expired + { + return Err("immutable input identity or lifetime differs".into()); + } + binding.metadata.group_by_keys.clone() + }; + if group.keys().cloned().collect::>() != keys { + return Err("immutable input population does not match its descriptor".into()); + } + let keys: Vec<_> = keys.into_iter().collect(); + if self + .completed_windows + .read() + .map_err(|_| "completion registry poisoned")? + .get(&sid) + .is_none_or(|frontier| *frontier < end_ms) + { + return Err("immutable input window is not complete".into()); + } + let handle = self + .persistence_read + .read() + .map_err(|_| "persistence registry poisoned")? + .clone() + .ok_or("immutable maintenance requires durable input state")?; + let mut windows = BTreeMap::new(); + for part in handle.manifest.live_parts_overlapping(start_ms, end_ms) { + let reader = handle + .part_cache + .get_or_load(part.part_id) + .map_err(|e| e.to_string())?; + for record in reader.index_records() { + if record.agg_id != sid || record.start_ts < start_ms || record.end_ts > end_ms { + continue; + } + let entry = reader.load_entry(&record).map_err(|e| e.to_string())?; + if entry.label.as_ref().map_or(0, |label| label.labels.len()) != keys.len() { + return Err("immutable input label arity differs from its descriptor".into()); + } + if Self::rebuild_label_map(&keys, &entry.label) != *group { + continue; + } + let state = reconstruct_exact_agg(&entry.sketch_type_name, &entry.sketch_bytes) + .ok_or("immutable input accumulator cannot be decoded")?; + if windows + .insert((record.start_ts, record.end_ts), Arc::from(state)) + .is_some() + { + // No last-writer-wins or implicit merge at a frozen source + // boundary: the producer must publish one complete state. + return Err("immutable input has multiple states for one window".into()); + } + } + } + if windows.keys().copied().collect::>() != *expected_windows { + return Err("immutable input window coverage is missing, expired, or ambiguous".into()); + } + self.validate_routed_catalog_generation(Some(generation.as_ref()))?; + Ok(FrozenExactWindows { + sid, + definition, + generation: Arc::clone(generation), + group: group.clone(), + windows, + }) + } +} + +impl SketchStore { + pub(crate) fn lookup_frozen_maintenance_output( + &self, + sid: u64, + config: &asap_types::PrecomputeMaterialization, + source: &FrozenExactWindows, + digest: [u8; 32], + window: (u64, u64), + ) -> Result { + self.validate_routed_catalog_generation(Some(source.generation.as_ref()))?; + let instances = self + .instances + .read() + .map_err(|_| "instance registry poisoned")?; + let Some(binding) = instances.get(&sid) else { + return Ok(false); + }; + if binding.metadata.policy_fp != config.policy_fingerprint() + || binding.catalog_generation.as_deref() != Some(source.generation.as_ref()) + || !binding.metadata.is_writable() + { + return Err("immutable output identity or lifetime changed".into()); + } + let record = self + .metadata_record_without_completion(binding) + .ok_or("immutable output has no catalog metadata")?; + let publisher = self + .immutable_publisher + .read() + .map_err(|_| "publisher registry poisoned")? + .upgrade() + .ok_or("immutable publication requires active persistence")?; + Ok(publisher + .lookup_immutable_window(&record, digest, window.0, window.1) + .map_err(|error| error.to_string())? + .is_some()) + } + + /// Publish a derived result without passing it through additive hot-state + /// append. The existing part reservation commits it once and seals it. + pub(crate) fn publish_frozen_maintenance_output( + &self, + sid: u64, + config: &asap_types::PrecomputeMaterialization, + output: &crate::storage_engines::types::PrecomputedOutput, + state: &dyn AggregateCore, + source: &FrozenExactWindows, + input_digest: [u8; 32], + ) -> Result { + use persistence::source::{EpochSnapshot, EpochSnapshotEntry}; + if config + .derived_input + .as_ref() + .is_none_or(|derived| derived.inputs != BTreeSet::from([source.definition])) + || output.policy_fp != config.policy_fingerprint() + || output.catalog_generation.as_deref() != Some(source.generation.as_ref()) + { + return Err("derived publication differs from its installed input identity".into()); + } + let labels = self + .register_precompute_output(sid, config, output) + .ok_or("derived output registration failed")?; + let _mutation = self.begin_state_mutation(); + let instances = self + .instances + .read() + .map_err(|_| "instance registry poisoned")?; + let source_binding = instances + .get(&source.sid) + .ok_or("immutable source was removed")?; + let binding = instances.get(&sid).ok_or("derived output was removed")?; + if source_binding.metadata.policy_fp != source.definition.fingerprint() + || source_binding.catalog_generation.as_deref() != Some(source.generation.as_ref()) + || source_binding.metadata.status() == AggStatus::Expired + || !binding.metadata.is_writable() + || binding.metadata.policy_fp != output.policy_fp + || binding.catalog_generation.as_deref() != Some(source.generation.as_ref()) + { + return Err("derived publication physical lifetime changed".into()); + } + self.validate_routed_catalog_generation(Some(source.generation.as_ref()))?; + let mut completed = self + .completed_windows + .write() + .map_err(|_| "completion registry poisoned")?; + let record = self + .metadata_record_without_completion(binding) + .ok_or("derived publication lacks catalog metadata")?; + let publisher = self + .immutable_publisher + .read() + .map_err(|_| "publisher registry poisoned")? + .upgrade() + .ok_or("immutable publication requires active persistence")?; + let bytes = state.serialize_to_bytes(); + let snapshot = EpochSnapshot { + agg_id: sid, + epoch_id: 0, + min_ts: output.start_timestamp, + max_ts: output.end_timestamp, + approx_bytes: bytes.len(), + entries: vec![EpochSnapshotEntry { + start_ts: output.start_timestamp, + end_ts: output.end_timestamp, + label: Some(crate::storage_engines::types::KeyByLabelValues { + labels: labels.into_values().collect(), + }), + sketch_type_name: state.type_name().to_string(), + encoding_tag: 0, + sketch_bytes: bytes, + }], + }; + // Another executor may have completed the same input after our first + // lookup. Reuse its durable payload instead of comparing a newly built, + // potentially randomized sketch byte representation. + if publisher + .lookup_immutable_window( + &record, + input_digest, + output.start_timestamp, + output.end_timestamp, + ) + .map_err(|error| error.to_string())? + .is_some() + { + completed + .entry(sid) + .and_modify(|end| *end = (*end).max(output.end_timestamp)) + .or_insert(output.end_timestamp); + return Ok(false); + } + let published = publisher + .publish_immutable_window(&record, input_digest, &snapshot) + .map_err(|error| error.to_string())?; + completed + .entry(sid) + .and_modify(|end| *end = (*end).max(output.end_timestamp)) + .or_insert(output.end_timestamp); + if !published.already_published { + crate::precompute_engine::metrics::record_materialized_outputs(1); + } + Ok(!published.already_published) + } +} 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 e6f355a4b..4c6181a15 100644 --- a/data_plane/src/storage_engines/sketch_db/index/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/index/mod.rs @@ -693,6 +693,7 @@ pub struct SketchStore { /// flush-then-evict loop is the memory bound). persistence_read: RwLock>>, persistence_metadata: RwLock>>, + immutable_publisher: RwLock>, removed_sids: RwLock< BTreeMap< u64, @@ -2472,6 +2473,15 @@ impl SketchStore { } fn metadata_record(&self, m: &SdsBinding) -> Option { + let mut record = self.metadata_record_without_completion(m)?; + record.completed_through_ms = self.completed_windows.read().unwrap().get(&m.sid).copied(); + Some(record) + } + + fn metadata_record_without_completion( + &self, + m: &SdsBinding, + ) -> Option { let mut record = crate::storage_engines::sketch_db::index::persistence::metadata::SidMetaRecord::new( m.sid, @@ -2484,7 +2494,6 @@ impl SketchStore { record.summary_definition_id = Some(SummaryDefinitionId::from(m.policy_fp)); record.catalog_generation = Some(Arc::clone(m.catalog_generation.as_ref()?)); } - record.completed_through_ms = self.completed_windows.read().unwrap().get(&m.sid).copied(); record.retired_at_ms = m.retired_at_ms; record.expires_at_ms = m.expires_at_ms; Some(record) @@ -2752,27 +2761,12 @@ impl SketchStore { self.ingest_precompute_with_series_id(sid, agg_cfg, output, accumulator) } - /// B7.7 sid-direct sibling of [`Self::ingest_precompute_for_agg_config`]. - /// - /// Callers that already hold the bucket sid (the live worker after - /// B7.6 reshaped its `WorkerMessage`, and the backfill processor - /// after B7.7 rekeyed its per-window grouping from group_key to - /// sid) skip the mint round-trip by handing the sid in directly. - /// The mint-driven [`Self::ingest_precompute_for_agg_config`] is - /// content-addressed and idempotent with this method — passing the - /// resolver-minted sid here yields the same state under the same - /// sid — so both methods can coexist while migration finishes. - /// - /// The §6.3 ingest barrier (`Retired` / `Expired` sids reject - /// writes) and first-sight metadata registration are identical to - /// the mint-driven path. - pub fn ingest_precompute_with_series_id( + fn register_precompute_output( &self, sid: u64, - agg_cfg: &asap_types::aggregation_config::AggregationConfig, + agg_cfg: &asap_types::PrecomputeMaterialization, output: &crate::storage_engines::types::PrecomputedOutput, - accumulator: &dyn crate::storage_engines::types::AggregateCore, - ) -> Option { + ) -> Option> { let (_attrs_fp, label_values_map) = build_attrs_fp_and_label_map(agg_cfg, output); let key_names = &agg_cfg.grouping_labels.names(); let agg_kind = crate::storage_engines::sketch_db::data::agg_kind_for_config(agg_cfg); @@ -2819,6 +2813,32 @@ impl SketchStore { Some(_) => {} } + Some(label_values_map) + } + + /// B7.7 sid-direct sibling of [`Self::ingest_precompute_for_agg_config`]. + /// + /// Callers that already hold the bucket sid (the live worker after + /// B7.6 reshaped its `WorkerMessage`, and the backfill processor + /// after B7.7 rekeyed its per-window grouping from group_key to + /// sid) skip the mint round-trip by handing the sid in directly. + /// The mint-driven [`Self::ingest_precompute_for_agg_config`] is + /// content-addressed and idempotent with this method — passing the + /// resolver-minted sid here yields the same state under the same + /// sid — so both methods can coexist while migration finishes. + /// + /// The §6.3 ingest barrier (`Retired` / `Expired` sids reject + /// writes) and first-sight metadata registration are identical to + /// the mint-driven path. + pub fn ingest_precompute_with_series_id( + &self, + sid: u64, + agg_cfg: &asap_types::aggregation_config::AggregationConfig, + output: &crate::storage_engines::types::PrecomputedOutput, + accumulator: &dyn crate::storage_engines::types::AggregateCore, + ) -> Option { + let label_values_map = self.register_precompute_output(sid, agg_cfg, output)?; + // Keep the physical lifetime alive through publication. Removal takes // this same lock exclusively, so it cannot race metadata validation and // recreate orphan payload after the tombstone commits. @@ -3041,6 +3061,7 @@ impl SketchStore { metadata_writer, )?; + *self.immutable_publisher.write().unwrap() = Arc::downgrade(&flusher.publication_handle()); Ok(SketchIndexPersistence { manifest, part_cache, @@ -5776,6 +5797,8 @@ mod tests { // 2026-05 reorg: generic epoch-partitioned columnar storage lives // alongside the store that uses it. mod admission; +mod maintenance; +pub(crate) use maintenance::FrozenExactWindows; pub mod epoch_columnar; // `persistence` moved up to `sketch_db::persistence`. Re-exported here diff --git a/docs/design_docs/summary-catalog-sds-architecture.md b/docs/design_docs/summary-catalog-sds-architecture.md index 0b813c392..30f7a87c2 100644 --- a/docs/design_docs/summary-catalog-sds-architecture.md +++ b/docs/design_docs/summary-catalog-sds-architecture.md @@ -475,3 +475,37 @@ Installation currently rejects derived inputs so they cannot accidentally receiv raw samples through the legacy metric router. Enabling them requires the immutable maintenance consumer and durable output deduplication protocol; neither raw-table substitution nor treating late correction fragments as new observations is valid. + +### Executing an immutable maintenance sink + +`precompute_engine::maintenance_runtime::execute_completed_maintenance` executes +one installed semantic subDAG from a physical source whose required base windows +are durably complete. SummaryStore validates the catalog generation, physical +SeriesId, population, exact window coverage, and each part read. Missing, corrupt, +or duplicate source windows are errors; this path cannot silently omit a pane as +a query fallback helper might. + +The existing maintenance operator registry preserves a collection of source +states until the DAG explicitly merges or finalizes it. Exact Sum/Count +finalization produces one row per source window; an unkeyed SummaryAgg consumes +those rows together. Consequently `Finalize -> SummaryAgg` does not accidentally +become one complete DAG evaluation per correction fragment. Live worker fragments +remain ineligible for finalization. + +The engine looks up the stored input digest before computing a potentially +randomized sketch. The existing flusher publishes a new result through its part +reservation protocol; SummaryStore fences query reads and physical lifetime +changes during publication. A concurrent identical completion reuses the durable +result instead of comparing newly randomized bytes. The latest committed window +can be retried after restart without adding another part. + +This is an explicit maintenance entry point, not automatic workload coverage. +The initial consumer supports one source definition and population, complete +non-overlapping base panes, Sum/Count finalization, and unkeyed aggregate updates. +Compiler construction and automatic scheduling must use this entry point before +derived installation is enabled. Cross-population reductions, synchronized +multiple sources, general row operators, overlapping output-window replacement, +and continuous producer watermarks remain unsupported. In particular, the SQL +subquery's timestamp grouping and sampling predicate must not be replaced with an +arbitrary tumbling aggregate. Historical completion-metadata GC and pinning source +parts for recovery before a reserved output part exists remain lifecycle work. From 5f2a15779bb26ad37c7105ecf24af1d686e16260 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:34:24 -0600 Subject: [PATCH 19/29] fix(precompute): reject unsupported maintenance row types --- .../precompute_engine/maintenance_runtime.rs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/data_plane/src/precompute_engine/maintenance_runtime.rs b/data_plane/src/precompute_engine/maintenance_runtime.rs index c9e9f4d76..29a9af29e 100644 --- a/data_plane/src/precompute_engine/maintenance_runtime.rs +++ b/data_plane/src/precompute_engine/maintenance_runtime.rs @@ -254,8 +254,13 @@ fn finalize_exact( let [field] = node.output_schema.fields.as_slice() else { return Err("exact maintenance finalization requires one scalar output column".into()); }; - if !matches!(field.dtype, SummaryFamilyType::Plain(_)) { - return Err("exact maintenance finalization output must be a plain value".into()); + if !matches!( + field.dtype, + SummaryFamilyType::Plain(planner_types::pre_asap::DataType::Float64) + ) { + return Err( + "exact maintenance finalization currently requires a Float64 output column".into(), + ); } let values = states .into_iter() @@ -1606,6 +1611,18 @@ mod tests { panic!("expected finalized row") }; assert_eq!(values, vec![(2_000, 9.0)]); + read.output_schema.fields[0].dtype = + SummaryFamilyType::Plain(planner_types::pre_asap::DataType::Int64); + let integer_state = Arc::new(MaintenanceValue::Summary { + state: sum(9.0), + family: Some(SummaryFamilyType::ExactAggregate( + ExactKind::Sum, + ExactParams::Sum, + )), + }); + assert!( + matches!(finalize_exact(&read, &[integer_state]), Err(error) if error.contains("Float64")) + ); } #[test] From 03398cbd696d03dd2285471d31122ec82136f8fe Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:36:52 -0600 Subject: [PATCH 20/29] fix(storage): match pending lineage atomically during recovery --- .../sketch_db/persistence/immutable_output.rs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs index 468c26f02..0ac85e833 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -254,12 +254,32 @@ impl FlusherShared { }) } + /// Resume only the caller's pending lineage, atomically with metadata + /// validation. A different pending input cannot be acknowledged by this call. + pub fn resume_matching_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + start_ms: u64, + end_ms: u64, + ) -> PersistResult> { + self.resume_immutable_window(record.sid, Some((record, input_digest, start_ms, end_ms))) + } + /// Complete a pending durable part without reconstructing its source input. /// A reservation whose part was never completed returns an error, preserving /// the reservation for an explicit recovery policy rather than losing data. pub fn resume_pending_immutable_window( &self, sid: u64, + ) -> PersistResult> { + self.resume_immutable_window(sid, None) + } + + fn resume_immutable_window( + &self, + sid: u64, + expected: Option<(&SidMetaRecord, [u8; 32], u64, u64)>, ) -> PersistResult> { self.sid_metadata.transaction(|records, metadata| { let key = sid.to_string(); @@ -273,6 +293,15 @@ impl FlusherShared { { return Err(invalid("pending immutable SID was removed")); } + if let Some((record, digest, start, end)) = expected { + validate_identity(¤t, record)?; + if pending.input_digest != digest + || pending.start_ms != start + || pending.end_ms != end + { + return Err(invalid("pending immutable recovery lineage differs")); + } + } self.validate_reserved_part(sid, &pending)?; self.finish_reserved_part(&mut current, &pending)?; records.insert(key, current); @@ -596,4 +625,35 @@ mod tests { .lookup_immutable_window(&record(), [7; 32], 10, 20) .is_err()); } + #[test] + fn matching_resume_rejects_other_lineage_without_mutating_reservation() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + let state = snapshot(); + let pending = reserve(&handle, &state); + let path = part_dir_path(&temp.path().join("parts"), pending.part_id); + PartWriter::write_part(&path, pending.part_id, &[state]).unwrap(); + assert!(handle + .inner + .resume_matching_immutable_window(&record(), [8; 32], 10, 20) + .is_err()); + assert!(handle + .inner + .resume_matching_immutable_window(&record(), [7; 32], 20, 30) + .is_err()); + assert!(handle.manifest().live_parts().is_empty()); + assert_eq!( + handle.inner.sid_metadata.load_strict().unwrap()[0].pending_immutable, + Some(pending.clone()) + ); + assert_eq!( + handle + .inner + .resume_matching_immutable_window(&record(), [7; 32], 10, 20) + .unwrap() + .unwrap() + .part_id, + pending.part_id + ); + } } From 93f2efa21ce8ad3ddd28a032a7359e4c74a17025 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:36:52 -0600 Subject: [PATCH 21/29] fix(storage): match pending lineage atomically during recovery --- .../sketch_db/persistence/immutable_output.rs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs index 468c26f02..0ac85e833 100644 --- a/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs +++ b/data_plane/src/storage_engines/sketch_db/persistence/immutable_output.rs @@ -254,12 +254,32 @@ impl FlusherShared { }) } + /// Resume only the caller's pending lineage, atomically with metadata + /// validation. A different pending input cannot be acknowledged by this call. + pub fn resume_matching_immutable_window( + &self, + record: &SidMetaRecord, + input_digest: [u8; 32], + start_ms: u64, + end_ms: u64, + ) -> PersistResult> { + self.resume_immutable_window(record.sid, Some((record, input_digest, start_ms, end_ms))) + } + /// Complete a pending durable part without reconstructing its source input. /// A reservation whose part was never completed returns an error, preserving /// the reservation for an explicit recovery policy rather than losing data. pub fn resume_pending_immutable_window( &self, sid: u64, + ) -> PersistResult> { + self.resume_immutable_window(sid, None) + } + + fn resume_immutable_window( + &self, + sid: u64, + expected: Option<(&SidMetaRecord, [u8; 32], u64, u64)>, ) -> PersistResult> { self.sid_metadata.transaction(|records, metadata| { let key = sid.to_string(); @@ -273,6 +293,15 @@ impl FlusherShared { { return Err(invalid("pending immutable SID was removed")); } + if let Some((record, digest, start, end)) = expected { + validate_identity(¤t, record)?; + if pending.input_digest != digest + || pending.start_ms != start + || pending.end_ms != end + { + return Err(invalid("pending immutable recovery lineage differs")); + } + } self.validate_reserved_part(sid, &pending)?; self.finish_reserved_part(&mut current, &pending)?; records.insert(key, current); @@ -596,4 +625,35 @@ mod tests { .lookup_immutable_window(&record(), [7; 32], 10, 20) .is_err()); } + #[test] + fn matching_resume_rejects_other_lineage_without_mutating_reservation() { + let temp = tempfile::tempdir().unwrap(); + let handle = handle(temp.path()); + let state = snapshot(); + let pending = reserve(&handle, &state); + let path = part_dir_path(&temp.path().join("parts"), pending.part_id); + PartWriter::write_part(&path, pending.part_id, &[state]).unwrap(); + assert!(handle + .inner + .resume_matching_immutable_window(&record(), [8; 32], 10, 20) + .is_err()); + assert!(handle + .inner + .resume_matching_immutable_window(&record(), [7; 32], 20, 30) + .is_err()); + assert!(handle.manifest().live_parts().is_empty()); + assert_eq!( + handle.inner.sid_metadata.load_strict().unwrap()[0].pending_immutable, + Some(pending.clone()) + ); + assert_eq!( + handle + .inner + .resume_matching_immutable_window(&record(), [7; 32], 10, 20) + .unwrap() + .unwrap() + .part_id, + pending.part_id + ); + } } From 31bdeb8f65353291c4f9492f76f2d587ca3d85ee Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:37:36 -0600 Subject: [PATCH 22/29] fix(clickhouse): preserve SQL nulls in result formats --- .../accelerator.rs | 9 ++-- .../clickhouse_result_adapter.rs | 46 +++++++++++++++++-- 2 files changed, 47 insertions(+), 8 deletions(-) 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 8e34ceafd..dc7740561 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 @@ -147,11 +147,9 @@ impl CatalogClickHouseAccelerator { } fn requested_format(request: &ClickHouseQueryRequest) -> Result { - if let Some(setting) = request - .parameters - .keys() - .find(|key| key.starts_with("output_format_")) - { + if let Some(setting) = request.parameters.keys().find(|key| { + key.starts_with("output_format_") || key.as_str() == "format_tsv_null_representation" + }) { return Err(format!("unsupported output setting {setting}")); } match request @@ -302,6 +300,7 @@ mod tests { for setting in [ "output_format_json_map_as_array_of_tuples", "output_format_json_quote_64bit_integers", + "format_tsv_null_representation", ] { request.parameters.insert(setting.into(), "1".into()); assert!(requested_format(&request).is_err()); diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs index a829c548f..61d0558b5 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs @@ -2,7 +2,7 @@ use super::fallback::ClickHouseRawResponse; use arrow::{ array::{Array, Float64Array, StringArray, TimestampMillisecondArray}, datatypes::{DataType, Field, Schema}, - json::LineDelimitedWriter, + json::{LineDelimitedWriter, WriterBuilder}, record_batch::RecordBatch, util::display::array_value_to_string, }; @@ -118,6 +118,10 @@ impl ClickHouseQueryResult { if column > 0 { output.push(b'\t'); } + if batch.column(column).is_null(row) { + output.extend_from_slice(b"\\N"); + continue; + } if matches!(batch.column(column).data_type(), DataType::Map(..)) { output.extend_from_slice( map_literal(batch.column(column).as_ref(), row)?.as_bytes(), @@ -145,7 +149,9 @@ impl ClickHouseQueryResult { } let row = batch.slice(row, 1); - let mut writer = LineDelimitedWriter::new(&mut output); + let mut writer: LineDelimitedWriter<_> = WriterBuilder::new() + .with_explicit_nulls(true) + .build(&mut output); writer .write_batches(&[&row]) .map_err(|error| ClickHouseResultError::Arrow(error.to_string()))?; @@ -204,7 +210,9 @@ impl ClickHouseQueryResult { let mut rows = Vec::new(); for batch in &self.batches { let mut encoded = Vec::new(); - let mut writer = LineDelimitedWriter::new(&mut encoded); + let mut writer: LineDelimitedWriter<_> = WriterBuilder::new() + .with_explicit_nulls(true) + .build(&mut encoded); writer .write_batches(&[batch]) .map_err(|error| ClickHouseResultError::Arrow(error.to_string()))?; @@ -309,6 +317,38 @@ mod tests { }; use std::sync::Arc; + #[test] + fn nullable_fields_remain_explicit_in_json_and_tsv() { + let batch = RecordBatch::try_new( + Arc::new(Schema::new(vec![ + Field::new("value", DataType::Float64, true), + Field::new("label", DataType::Utf8, true), + ])), + vec![ + Arc::new(Float64Array::from(vec![Some(1.25), None])), + Arc::new(StringArray::from(vec![Some(""), None])), + ], + ) + .unwrap(); + let result = ClickHouseQueryResult { + batches: vec![batch], + }; + let json: serde_json::Value = + serde_json::from_slice(&result.encode(ClickHouseFormat::Json).unwrap()).unwrap(); + assert_eq!( + json["data"], + serde_json::json!([{ "value":1.25,"label":"" }, { "value":null,"label":null }]) + ); + let lines = result.encode(ClickHouseFormat::JsonEachRow).unwrap(); + let null_row: serde_json::Value = + serde_json::from_slice(lines.split(|byte| *byte == b'\n').nth(1).unwrap()).unwrap(); + assert_eq!(null_row, serde_json::json!({"value":null,"label":null})); + assert_eq!( + result.encode(ClickHouseFormat::TabSeparated).unwrap(), + b"1.25\t\n\\N\t\\N\n" + ); + } + #[test] fn empty_map_bottom_type_uses_clickhouse_nothing() { let entries = DataType::Struct( From 8e0ddd6f87c959775e91337f67933c1fbeb2fd56 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:37:36 -0600 Subject: [PATCH 23/29] fix(clickhouse): preserve SQL nulls in result formats --- .../accelerator.rs | 9 ++-- .../clickhouse_result_adapter.rs | 46 +++++++++++++++++-- 2 files changed, 47 insertions(+), 8 deletions(-) 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 8e34ceafd..dc7740561 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 @@ -147,11 +147,9 @@ impl CatalogClickHouseAccelerator { } fn requested_format(request: &ClickHouseQueryRequest) -> Result { - if let Some(setting) = request - .parameters - .keys() - .find(|key| key.starts_with("output_format_")) - { + if let Some(setting) = request.parameters.keys().find(|key| { + key.starts_with("output_format_") || key.as_str() == "format_tsv_null_representation" + }) { return Err(format!("unsupported output setting {setting}")); } match request @@ -302,6 +300,7 @@ mod tests { for setting in [ "output_format_json_map_as_array_of_tuples", "output_format_json_quote_64bit_integers", + "format_tsv_null_representation", ] { request.parameters.insert(setting.into(), "1".into()); assert!(requested_format(&request).is_err()); diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs index a829c548f..61d0558b5 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs @@ -2,7 +2,7 @@ use super::fallback::ClickHouseRawResponse; use arrow::{ array::{Array, Float64Array, StringArray, TimestampMillisecondArray}, datatypes::{DataType, Field, Schema}, - json::LineDelimitedWriter, + json::{LineDelimitedWriter, WriterBuilder}, record_batch::RecordBatch, util::display::array_value_to_string, }; @@ -118,6 +118,10 @@ impl ClickHouseQueryResult { if column > 0 { output.push(b'\t'); } + if batch.column(column).is_null(row) { + output.extend_from_slice(b"\\N"); + continue; + } if matches!(batch.column(column).data_type(), DataType::Map(..)) { output.extend_from_slice( map_literal(batch.column(column).as_ref(), row)?.as_bytes(), @@ -145,7 +149,9 @@ impl ClickHouseQueryResult { } let row = batch.slice(row, 1); - let mut writer = LineDelimitedWriter::new(&mut output); + let mut writer: LineDelimitedWriter<_> = WriterBuilder::new() + .with_explicit_nulls(true) + .build(&mut output); writer .write_batches(&[&row]) .map_err(|error| ClickHouseResultError::Arrow(error.to_string()))?; @@ -204,7 +210,9 @@ impl ClickHouseQueryResult { let mut rows = Vec::new(); for batch in &self.batches { let mut encoded = Vec::new(); - let mut writer = LineDelimitedWriter::new(&mut encoded); + let mut writer: LineDelimitedWriter<_> = WriterBuilder::new() + .with_explicit_nulls(true) + .build(&mut encoded); writer .write_batches(&[batch]) .map_err(|error| ClickHouseResultError::Arrow(error.to_string()))?; @@ -309,6 +317,38 @@ mod tests { }; use std::sync::Arc; + #[test] + fn nullable_fields_remain_explicit_in_json_and_tsv() { + let batch = RecordBatch::try_new( + Arc::new(Schema::new(vec![ + Field::new("value", DataType::Float64, true), + Field::new("label", DataType::Utf8, true), + ])), + vec![ + Arc::new(Float64Array::from(vec![Some(1.25), None])), + Arc::new(StringArray::from(vec![Some(""), None])), + ], + ) + .unwrap(); + let result = ClickHouseQueryResult { + batches: vec![batch], + }; + let json: serde_json::Value = + serde_json::from_slice(&result.encode(ClickHouseFormat::Json).unwrap()).unwrap(); + assert_eq!( + json["data"], + serde_json::json!([{ "value":1.25,"label":"" }, { "value":null,"label":null }]) + ); + let lines = result.encode(ClickHouseFormat::JsonEachRow).unwrap(); + let null_row: serde_json::Value = + serde_json::from_slice(lines.split(|byte| *byte == b'\n').nth(1).unwrap()).unwrap(); + assert_eq!(null_row, serde_json::json!({"value":null,"label":null})); + assert_eq!( + result.encode(ClickHouseFormat::TabSeparated).unwrap(), + b"1.25\t\n\\N\t\\N\n" + ); + } + #[test] fn empty_map_bottom_type_uses_clickhouse_nothing() { let entries = DataType::Struct( From f6aa7feb082ff4326e992f725d34a9fbb1e1b4c2 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:39:04 -0600 Subject: [PATCH 24/29] test(clickhouse): distinguish literal null marker strings --- .../clickhouse_result_adapter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs index 61d0558b5..cdc46d262 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs @@ -325,8 +325,8 @@ mod tests { Field::new("label", DataType::Utf8, true), ])), vec![ - Arc::new(Float64Array::from(vec![Some(1.25), None])), - Arc::new(StringArray::from(vec![Some(""), None])), + Arc::new(Float64Array::from(vec![Some(1.25), None, Some(2.5)])), + Arc::new(StringArray::from(vec![Some(""), None, Some("\\N")])), ], ) .unwrap(); @@ -337,7 +337,7 @@ mod tests { serde_json::from_slice(&result.encode(ClickHouseFormat::Json).unwrap()).unwrap(); assert_eq!( json["data"], - serde_json::json!([{ "value":1.25,"label":"" }, { "value":null,"label":null }]) + serde_json::json!([{ "value":1.25,"label":"" }, { "value":null,"label":null }, {"value":2.5,"label":"\\N"}]) ); let lines = result.encode(ClickHouseFormat::JsonEachRow).unwrap(); let null_row: serde_json::Value = @@ -345,7 +345,7 @@ mod tests { assert_eq!(null_row, serde_json::json!({"value":null,"label":null})); assert_eq!( result.encode(ClickHouseFormat::TabSeparated).unwrap(), - b"1.25\t\n\\N\t\\N\n" + b"1.25\t\n\\N\t\\N\n2.5\t\\\\N\n" ); } From 2754be8ed9e6e44b731ca2cd8d4f854b9f7b48c9 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:39:04 -0600 Subject: [PATCH 25/29] test(clickhouse): distinguish literal null marker strings --- .../clickhouse_result_adapter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs index 61d0558b5..cdc46d262 100644 --- a/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs +++ b/data_plane/src/query_engines/asap_clickhouse_query_engine/clickhouse_result_adapter.rs @@ -325,8 +325,8 @@ mod tests { Field::new("label", DataType::Utf8, true), ])), vec![ - Arc::new(Float64Array::from(vec![Some(1.25), None])), - Arc::new(StringArray::from(vec![Some(""), None])), + Arc::new(Float64Array::from(vec![Some(1.25), None, Some(2.5)])), + Arc::new(StringArray::from(vec![Some(""), None, Some("\\N")])), ], ) .unwrap(); @@ -337,7 +337,7 @@ mod tests { serde_json::from_slice(&result.encode(ClickHouseFormat::Json).unwrap()).unwrap(); assert_eq!( json["data"], - serde_json::json!([{ "value":1.25,"label":"" }, { "value":null,"label":null }]) + serde_json::json!([{ "value":1.25,"label":"" }, { "value":null,"label":null }, {"value":2.5,"label":"\\N"}]) ); let lines = result.encode(ClickHouseFormat::JsonEachRow).unwrap(); let null_row: serde_json::Value = @@ -345,7 +345,7 @@ mod tests { assert_eq!(null_row, serde_json::json!({"value":null,"label":null})); assert_eq!( result.encode(ClickHouseFormat::TabSeparated).unwrap(), - b"1.25\t\n\\N\t\\N\n" + b"1.25\t\n\\N\t\\N\n2.5\t\\\\N\n" ); } From b5322d07118cb4610190d8b7a304e763db9d95cd Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:41:18 -0600 Subject: [PATCH 26/29] fix(precompute): recover pending output before sketch evaluation --- .../precompute_engine/maintenance_runtime.rs | 36 +++++++++++++++++-- .../sketch_db/index/maintenance.rs | 18 +++++++++- .../summary-catalog-sds-architecture.md | 6 ++-- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/data_plane/src/precompute_engine/maintenance_runtime.rs b/data_plane/src/precompute_engine/maintenance_runtime.rs index 29a9af29e..bc2e2db49 100644 --- a/data_plane/src/precompute_engine/maintenance_runtime.rs +++ b/data_plane/src/precompute_engine/maintenance_runtime.rs @@ -566,7 +566,13 @@ pub fn execute_completed_maintenance( .as_slice() .try_into() .map_err(|_| "invalid input digest")?; - if store.lookup_frozen_maintenance_output(target_sid, target_config, &frozen, digest, window)? { + if store.recover_frozen_maintenance_output( + target_sid, + target_config, + &frozen, + digest, + window, + )? { return Ok(false); } let state = execute_prepared_frozen_sink(installed, configs, sink, &frozen, &dag, key, states)?; @@ -1400,7 +1406,7 @@ mod tests { config }; let expected = BTreeSet::from([(0, 1000), (1000, 2000)]); - let store = Arc::new(SketchStore::new()); + let mut store = Arc::new(SketchStore::new()); store.install_summary_catalog(Arc::clone(&catalog)).unwrap(); let mut persistence = store.start_persistence(persistence_config()).unwrap(); let generation = store.active_catalog_generation().unwrap(); @@ -1475,6 +1481,10 @@ mod tests { let mut output = PrecomputedOutput::new(0, 2000, None, durable_configs[1].policy_fingerprint()); output.catalog_generation = Some(Arc::clone(&generation)); + let log = persistence.manifest.log_path(); + let backup = log.with_extension("saved"); + std::fs::rename(&log, &backup).unwrap(); + std::fs::create_dir(&log).unwrap(); assert!(execute_completed_maintenance( &store, &installed, @@ -1485,6 +1495,26 @@ mod tests { (0, 2000), &BTreeMap::new() ) + .is_err()); + std::fs::remove_dir(&log).unwrap(); + std::fs::rename(&backup, &log).unwrap(); + persistence.shutdown(); + drop(store); + store = Arc::new(SketchStore::new()); + store.install_summary_catalog(Arc::clone(&catalog)).unwrap(); + persistence = store.start_persistence(persistence_config()).unwrap(); + // The already durable pending KLL part is completed before a new + // randomized sketch can be built after restart. + assert!(!execute_completed_maintenance( + &store, + &installed, + &durable_configs, + PostAsapNodeId(3), + 600, + 601, + (0, 2000), + &BTreeMap::new(), + ) .unwrap()); assert!(!execute_completed_maintenance( &store, @@ -1528,7 +1558,7 @@ mod tests { &BTreeMap::new(), ) .unwrap(); - let (result, replay_digest) = evaluate_frozen_maintenance_sink( + let (_result, replay_digest) = evaluate_frozen_maintenance_sink( &installed, &durable_configs, PostAsapNodeId(3), diff --git a/data_plane/src/storage_engines/sketch_db/index/maintenance.rs b/data_plane/src/storage_engines/sketch_db/index/maintenance.rs index dba5700e8..bc5fa822d 100644 --- a/data_plane/src/storage_engines/sketch_db/index/maintenance.rs +++ b/data_plane/src/storage_engines/sketch_db/index/maintenance.rs @@ -116,7 +116,7 @@ impl SketchStore { } impl SketchStore { - pub(crate) fn lookup_frozen_maintenance_output( + pub(crate) fn recover_frozen_maintenance_output( &self, sid: u64, config: &asap_types::PrecomputeMaterialization, @@ -147,6 +147,22 @@ impl SketchStore { .map_err(|_| "publisher registry poisoned")? .upgrade() .ok_or("immutable publication requires active persistence")?; + let _mutation = self.begin_state_mutation(); + let mut completed = self + .completed_windows + .write() + .map_err(|_| "completion registry poisoned")?; + if publisher + .resume_matching_immutable_window(&record, digest, window.0, window.1) + .map_err(|error| error.to_string())? + .is_some() + { + completed + .entry(sid) + .and_modify(|end| *end = (*end).max(window.1)) + .or_insert(window.1); + return Ok(true); + } Ok(publisher .lookup_immutable_window(&record, digest, window.0, window.1) .map_err(|error| error.to_string())? diff --git a/docs/design_docs/summary-catalog-sds-architecture.md b/docs/design_docs/summary-catalog-sds-architecture.md index b20461934..f8ac9f702 100644 --- a/docs/design_docs/summary-catalog-sds-architecture.md +++ b/docs/design_docs/summary-catalog-sds-architecture.md @@ -497,13 +497,13 @@ a query fallback helper might. The existing maintenance operator registry preserves a collection of source states until the DAG explicitly merges or finalizes it. Exact Sum/Count -finalization produces one row per source window; an unkeyed SummaryAgg consumes +finalization with a declared Float64 output produces one row per source window; an unkeyed SummaryAgg consumes those rows together. Consequently `Finalize -> SummaryAgg` does not accidentally become one complete DAG evaluation per correction fragment. Live worker fragments remain ineligible for finalization. -The engine looks up the stored input digest before computing a potentially -randomized sketch. The existing flusher publishes a new result through its part +The engine resumes a matching durable pending part and looks up the stored input +digest before computing a potentially randomized sketch. The existing flusher publishes a new result through its part reservation protocol; SummaryStore fences query reads and physical lifetime changes during publication. A concurrent identical completion reuses the durable result instead of comparing newly randomized bytes. The latest committed window From fe0ba21bb5d888e8bd4fdc720cfe7a78966021e4 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:43:51 -0600 Subject: [PATCH 27/29] test(clickhouse): execute planner-selected array SQL in a process --- Cargo.lock | 10 +- control_plane/Cargo.toml | 8 +- crates/asap_types/Cargo.toml | 2 +- data_plane/Cargo.toml | 6 +- .../tests/clickhouse_differential_e2e.rs | 278 +++++++++++++++--- 5 files changed, 246 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c0dfaf2b9..13a4be1ff 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=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" 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=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" dependencies = [ "asap-types", "promql-parser", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -415,12 +415,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=5e9396033a5a5d5f350bfa675d12770c972ee991#5e9396033a5a5d5f350bfa675d12770c972ee991" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" dependencies = [ "serde", "serde_json", diff --git a/control_plane/Cargo.toml b/control_plane/Cargo.toml index 264534341..fb664da32 100644 --- a/control_plane/Cargo.toml +++ b/control_plane/Cargo.toml @@ -76,8 +76,8 @@ asap_types.workspace = true # scaffolding, unaware that `data_plane`'s `summary_executor.rs` in *this* # repo is a real one. Vendored locally instead of chased upstream -- see # `data_plane/src/query_engines/asap_query_engine/summary_exec.rs`. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } -asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } # L1 adoption (design-target-architecture.md Part B): the PromQL front # end itself, replacing control_plane's own query_parser/promql.rs. @@ -85,8 +85,8 @@ asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = # `planner-types`/`asap-aware-mapping` above -- these three MUST move # together (two revs of the same upstream repo's types in one workspace # resolve to distinct Rust types that won't unify). -asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } -asap-frontend-sql = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +asap-frontend-sql = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } [dev-dependencies] tokio = { version = "1", features = ["full", "test-util"] } diff --git a/crates/asap_types/Cargo.toml b/crates/asap_types/Cargo.toml index d6480e602..16ed0aba9 100644 --- a/crates/asap_types/Cargo.toml +++ b/crates/asap_types/Cargo.toml @@ -34,4 +34,4 @@ sha2 = "0.10" # exactly (`control_plane/Cargo.toml`) -- two different revs of the same # git dependency in one workspace resolve to two distinct Rust types that # won't unify. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } diff --git a/data_plane/Cargo.toml b/data_plane/Cargo.toml index cc5310495..536f24fe7 100644 --- a/data_plane/Cargo.toml +++ b/data_plane/Cargo.toml @@ -39,8 +39,8 @@ sha2 = "0.10" # reduction: Reduction, .. }`) are `pre_asap` types, in the same crate now # (not a separate `asap-ir` import). Query serving consumes the compiled # QueryPlan; these types are used at physical-plan compilation boundaries. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } -asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } # Shared external (workspace) serde.workspace = true @@ -133,7 +133,7 @@ fs2 = "0.4" # none of them. [dev-dependencies] -asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "5e9396033a5a5d5f350bfa675d12770c972ee991" } +asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } tempfile = "3.20.0" criterion = { version = "0.5", features = ["html_reports"] } tokio-tungstenite = "0.21" diff --git a/data_plane/tests/clickhouse_differential_e2e.rs b/data_plane/tests/clickhouse_differential_e2e.rs index 75a07fd46..8023fd229 100644 --- a/data_plane/tests/clickhouse_differential_e2e.rs +++ b/data_plane/tests/clickhouse_differential_e2e.rs @@ -51,6 +51,67 @@ async fn wait_http(client: &reqwest::Client, url: &str, child: &mut Child) { panic!("data plane did not become ready at {url}"); } +async fn spawn_backend( + clickhouse_url: &str, + user: Option<&str>, + password: Option<&str>, + output: &std::path::Path, + bootstrap: &std::path::Path, + api_port: u16, + sql_port: u16, +) -> ChildGuard { + let client = reqwest::Client::new(); + let mut command = Command::new(env!("CARGO_BIN_EXE_data_plane")); + command + .arg("--streaming-config") + .arg(bootstrap) + .arg("--http-port") + .arg(api_port.to_string()) + .arg("--clickhouse-http-port") + .arg(sql_port.to_string()) + .arg("--clickhouse-url") + .arg(&clickhouse_url) + .arg("--clickhouse-backfill-table") + .arg("deployment_default_not_the_job_table") + .arg("--clickhouse-backfill-database") + .arg("default") + .arg("--clickhouse-backfill-value-column") + .arg("wrong_value") + .arg("--enable-backfill-worker") + .arg("--precompute-allowed-lateness-ms") + .arg("0") + .arg("--precompute-flush-interval-ms") + .arg("50") + .arg("--persistence-delete-older-than-secs") + .arg("0") + .arg("--output-dir") + .arg(output) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .env("RUST_LOG", "data_plane=info"); + if let Some(user) = user { + command.arg("--clickhouse-user").arg(user); + } + if let Some(password) = password { + command.arg("--clickhouse-password").arg(password); + } + let mut process = ChildGuard(command.spawn().unwrap()); + wait_http( + &client, + &format!("http://127.0.0.1:{api_port}/api/v1/health"), + &mut process.0, + ) + .await; + wait_http( + &client, + &format!("http://127.0.0.1:{sql_port}/ping"), + &mut process.0, + ) + .await; + + process +} + fn mixed_workload(sql: &str) -> control_plane::clickhouse::ClickHouseSqlAutomaticWorkload { use control_plane::physical::compiler::{PlanEnvelope, BACKEND_COMPAT, PLANNER_REVISION}; use planner_types::pre_asap::{Column, DataType, Schema}; @@ -285,51 +346,14 @@ async fn run_mixed_aggregate(aggregate: &str) { let output = tempfile::tempdir().unwrap(); let mut bootstrap = tempfile::NamedTempFile::new().unwrap(); writeln!(bootstrap, "aggregations: []").unwrap(); - let mut command = Command::new(env!("CARGO_BIN_EXE_data_plane")); - command - .arg("--streaming-config") - .arg(bootstrap.path()) - .arg("--http-port") - .arg(api_port.to_string()) - .arg("--clickhouse-http-port") - .arg(sql_port.to_string()) - .arg("--clickhouse-url") - .arg(&clickhouse_url) - .arg("--clickhouse-backfill-table") - .arg("deployment_default_not_the_job_table") - .arg("--clickhouse-backfill-database") - .arg("default") - .arg("--clickhouse-backfill-value-column") - .arg("wrong_value") - .arg("--enable-backfill-worker") - .arg("--precompute-allowed-lateness-ms") - .arg("0") - .arg("--precompute-flush-interval-ms") - .arg("50") - .arg("--persistence-delete-older-than-secs") - .arg("0") - .arg("--output-dir") - .arg(output.path()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .env("RUST_LOG", "data_plane=info"); - if let Some(user) = &user { - command.arg("--clickhouse-user").arg(user); - } - if let Some(password) = &password { - command.arg("--clickhouse-password").arg(password); - } - let mut process = ChildGuard(command.spawn().unwrap()); - wait_http( - &client, - &format!("http://127.0.0.1:{api_port}/api/v1/health"), - &mut process.0, - ) - .await; - wait_http( - &client, - &format!("http://127.0.0.1:{sql_port}/ping"), - &mut process.0, + let _process = spawn_backend( + &clickhouse_url, + user.as_deref(), + password.as_deref(), + output.path(), + bootstrap.path(), + api_port, + sql_port, ) .await; @@ -466,3 +490,167 @@ async fn run_mixed_aggregate(aggregate: &str) { ); } } + +#[tokio::test] +async fn array_sql_executes_local_element_after_typed_exact_leaf() { + use asap_types::query_plan::QueryPlanNode; + use planner_types::{ + post_asap::ValueOperation, + pre_asap::{Column, DataType, QueryExpr, Schema}, + }; + let Ok(clickhouse_url) = std::env::var("CLICKHOUSE_URL") else { + eprintln!("skipping collection process E2E because CLICKHOUSE_URL is unset"); + return; + }; + let user = std::env::var("CLICKHOUSE_USER").ok(); + let password = std::env::var("CLICKHOUSE_PASSWORD").ok(); + let client = reqwest::Client::new(); + let table = format!("asap_collection_elements_{}", std::process::id()); + for sql in [ + format!("CREATE TABLE default.{table}(timestamp Int64, samples Array(Float64), nullable_samples Array(Nullable(Float64)), position Nullable(Int64)) ENGINE=Memory"), + format!("INSERT INTO default.{table} VALUES (100,[10,20],[10,20],-1),(200,[3.5],[3.5],9),(300,[],[],1),(400,[7.5],[NULL],1),(500,[1],[1],NULL),(2000,[999],[999],1)"), + ] { + let mut request = client.post(&clickhouse_url).body(sql); + if let Some(user) = &user { request = request.basic_auth(user, password.as_ref()); } + let response = request.send().await.unwrap(); + let status = response.status(); + let body = response.text().await.unwrap(); + assert!(status.is_success(), "native collection fixture: {body}"); + } + let sql = format!("SELECT arrayElement(samples, position) AS result, arrayElement(nullable_samples, position) AS nullable_result FROM {table} WHERE timestamp >= 0 AND timestamp < 2000 ORDER BY result NULLS FIRST"); + let mut workload = mixed_workload(&sql); + workload.tables = HashMap::from([( + table.clone(), + Schema::with_time_index( + vec![ + Column::new("timestamp", DataType::Int64, false), + Column::new( + "samples", + DataType::List { + element: Box::new(Column::new("item", DataType::Float64, false)), + }, + false, + ), + Column::new( + "nullable_samples", + DataType::List { + element: Box::new(Column::new("item", DataType::Float64, true)), + }, + false, + ), + Column::new("position", DataType::Int64, true), + ], + 0, + vec![], + ), + )]); + let (publication, trace) = + control_plane::clickhouse::compile_automatic_clickhouse_workload(&workload) + .await + .unwrap(); + assert!(publication.precompute_plan.materializations.is_empty()); + let entry = publication.query_plan.entries.values().next().unwrap(); + assert!(entry + .nodes + .values() + .any(|node| matches!(node, QueryPlanNode::ExternalExact { .. }))); + assert!(entry.nodes.values().any(|node| { + let QueryPlanNode::Relational { operation, .. } = node else { return false; }; + let ValueOperation::Project { cols, .. } = serde_json::from_value(operation.clone()).unwrap() else { return false; }; + cols.iter().any(|column| matches!(&column.expr, QueryExpr::FunctionCall { name, .. } if name == "asap_element_access")) + }), "Planner-selected DAG must preserve local element evaluation"); + eprintln!( + "collection Planner selection: {}", + serde_json::to_string(&trace).unwrap() + ); + let install = publication.install_request(None, Vec::new()).unwrap(); + let api_port = unused_port(); + let sql_port = unused_port(); + let output = tempfile::tempdir().unwrap(); + let mut bootstrap = tempfile::NamedTempFile::new().unwrap(); + writeln!(bootstrap, "aggregations: []").unwrap(); + let _process = spawn_backend( + &clickhouse_url, + user.as_deref(), + password.as_deref(), + output.path(), + bootstrap.path(), + api_port, + sql_port, + ) + .await; + let response = client + .post(format!("http://127.0.0.1:{api_port}/api/v1/physical-plan")) + .json(&install) + .send() + .await + .unwrap(); + let status = response.status(); + let body = response.text().await.unwrap(); + assert!(status.is_success(), "stage collection publication: {body}"); + let response = client + .post(format!( + "http://127.0.0.1:{api_port}/api/v1/physical-plan/activate" + )) + .json(&serde_json::json!({"plan_id":72,"plan_version":1})) + .send() + .await + .unwrap(); + assert!(response.status().is_success()); + let mut native = client + .post(&clickhouse_url) + .body(format!("{sql} FORMAT JSON")); + if let Some(user) = &user { + native = native.basic_auth(user, password.as_ref()); + } + let expected: serde_json::Value = native.send().await.unwrap().json().await.unwrap(); + let mut actual_request = client + .get(format!("http://127.0.0.1:{sql_port}/")) + .query(&[("query", sql.as_str()), ("default_format", "JSON")]); + if let Some(user) = &user { + actual_request = actual_request.basic_auth(user, password.as_ref()); + } + let response = actual_request.send().await.unwrap(); + assert!(response.status().is_success()); + assert_eq!( + response.headers().get("x-asap-execution").unwrap(), + "exact_fallback" + ); + assert_eq!( + response.headers().get("x-asap-execution-detail").unwrap(), + "external_dag" + ); + let actual: serde_json::Value = response.json().await.unwrap(); + for field in ["meta", "rows"] { + assert_eq!(actual[field], expected[field], "{field}"); + } + let actual_rows = actual["data"].as_array().unwrap(); + let expected_rows = expected["data"].as_array().unwrap(); + assert_eq!(actual_rows.len(), expected_rows.len()); + for (actual, expected) in actual_rows.iter().zip(expected_rows) { + for field in ["result", "nullable_result"] { + if expected[field].is_null() { + assert!(actual[field].is_null()); + } else { + // Both declared columns are Float64; JSON 20 and 20.0 encode + // the same value despite different serde_json Number variants. + assert_eq!( + actual[field].as_f64().unwrap().to_bits(), + expected[field].as_f64().unwrap().to_bits(), + "{field}" + ); + } + } + } + assert_eq!( + actual["data"], + serde_json::json!([{ "result":null, "nullable_result":null },{ "result":0.0, "nullable_result":null },{ "result":0.0, "nullable_result":null },{ "result":7.5, "nullable_result":null },{ "result":20.0, "nullable_result":20.0 }]) + ); + let mut cleanup = client + .post(&clickhouse_url) + .body(format!("DROP TABLE default.{table}")); + if let Some(user) = &user { + cleanup = cleanup.basic_auth(user, password.as_ref()); + } + assert!(cleanup.send().await.unwrap().status().is_success()); +} From f711679831c35f09838f016f8545343847085c8d Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 09:46:04 -0600 Subject: [PATCH 28/29] test(clickhouse): include nested tuple fields in SQL process replay --- Cargo.lock | 10 ++--- control_plane/Cargo.toml | 8 ++-- crates/asap_types/Cargo.toml | 2 +- data_plane/Cargo.toml | 6 +-- .../tests/clickhouse_differential_e2e.rs | 42 +++++++++++++------ 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 13a4be1ff..75f855345 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=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=23270ba33009953b6c1e293ba95c6a430ab8d222#23270ba33009953b6c1e293ba95c6a430ab8d222" 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=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=23270ba33009953b6c1e293ba95c6a430ab8d222#23270ba33009953b6c1e293ba95c6a430ab8d222" dependencies = [ "asap-types", "promql-parser", @@ -393,7 +393,7 @@ dependencies = [ [[package]] name = "asap-frontend-sql" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=23270ba33009953b6c1e293ba95c6a430ab8d222#23270ba33009953b6c1e293ba95c6a430ab8d222" dependencies = [ "asap-sql-function-catalog", "asap-types", @@ -415,12 +415,12 @@ dependencies = [ [[package]] name = "asap-sql-function-catalog" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=23270ba33009953b6c1e293ba95c6a430ab8d222#23270ba33009953b6c1e293ba95c6a430ab8d222" [[package]] name = "asap-types" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=cf1f9acf0e09ce31b8319e674fb2508185a31148#cf1f9acf0e09ce31b8319e674fb2508185a31148" +source = "git+https://github.com/ProjectASAP/ASAPPlanner?rev=23270ba33009953b6c1e293ba95c6a430ab8d222#23270ba33009953b6c1e293ba95c6a430ab8d222" dependencies = [ "serde", "serde_json", diff --git a/control_plane/Cargo.toml b/control_plane/Cargo.toml index fb664da32..22ae1147d 100644 --- a/control_plane/Cargo.toml +++ b/control_plane/Cargo.toml @@ -76,8 +76,8 @@ asap_types.workspace = true # scaffolding, unaware that `data_plane`'s `summary_executor.rs` in *this* # repo is a real one. Vendored locally instead of chased upstream -- see # `data_plane/src/query_engines/asap_query_engine/summary_exec.rs`. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } -asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } +asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } # L1 adoption (design-target-architecture.md Part B): the PromQL front # end itself, replacing control_plane's own query_parser/promql.rs. @@ -85,8 +85,8 @@ asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = # `planner-types`/`asap-aware-mapping` above -- these three MUST move # together (two revs of the same upstream repo's types in one workspace # resolve to distinct Rust types that won't unify). -asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } -asap-frontend-sql = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } +asap-frontend-sql = { git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } [dev-dependencies] tokio = { version = "1", features = ["full", "test-util"] } diff --git a/crates/asap_types/Cargo.toml b/crates/asap_types/Cargo.toml index 16ed0aba9..88ce9cef9 100644 --- a/crates/asap_types/Cargo.toml +++ b/crates/asap_types/Cargo.toml @@ -34,4 +34,4 @@ sha2 = "0.10" # exactly (`control_plane/Cargo.toml`) -- two different revs of the same # git dependency in one workspace resolve to two distinct Rust types that # won't unify. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } diff --git a/data_plane/Cargo.toml b/data_plane/Cargo.toml index 536f24fe7..399067837 100644 --- a/data_plane/Cargo.toml +++ b/data_plane/Cargo.toml @@ -39,8 +39,8 @@ sha2 = "0.10" # reduction: Reduction, .. }`) are `pre_asap` types, in the same crate now # (not a separate `asap-ir` import). Query serving consumes the compiled # QueryPlan; these types are used at physical-plan compilation boundaries. -planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } -asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +planner-types = { package = "asap-types", git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } +asap-frontend-promql = { git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } # Shared external (workspace) serde.workspace = true @@ -133,7 +133,7 @@ fs2 = "0.4" # none of them. [dev-dependencies] -asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "cf1f9acf0e09ce31b8319e674fb2508185a31148" } +asap-aware-mapping = { git = "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/ProjectASAP/ASAPPlanner", rev = "23270ba33009953b6c1e293ba95c6a430ab8d222" } tempfile = "3.20.0" criterion = { version = "0.5", features = ["html_reports"] } tokio-tungstenite = "0.21" diff --git a/data_plane/tests/clickhouse_differential_e2e.rs b/data_plane/tests/clickhouse_differential_e2e.rs index 8023fd229..a470580e6 100644 --- a/data_plane/tests/clickhouse_differential_e2e.rs +++ b/data_plane/tests/clickhouse_differential_e2e.rs @@ -492,7 +492,7 @@ async fn run_mixed_aggregate(aggregate: &str) { } #[tokio::test] -async fn array_sql_executes_local_element_after_typed_exact_leaf() { +async fn collection_sql_executes_local_elements_after_typed_exact_leaf() { use asap_types::query_plan::QueryPlanNode; use planner_types::{ post_asap::ValueOperation, @@ -507,8 +507,8 @@ async fn array_sql_executes_local_element_after_typed_exact_leaf() { let client = reqwest::Client::new(); let table = format!("asap_collection_elements_{}", std::process::id()); for sql in [ - format!("CREATE TABLE default.{table}(timestamp Int64, samples Array(Float64), nullable_samples Array(Nullable(Float64)), position Nullable(Int64)) ENGINE=Memory"), - format!("INSERT INTO default.{table} VALUES (100,[10,20],[10,20],-1),(200,[3.5],[3.5],9),(300,[],[],1),(400,[7.5],[NULL],1),(500,[1],[1],NULL),(2000,[999],[999],1)"), + format!("CREATE TABLE default.{table}(timestamp Int64, samples Array(Float64), nullable_samples Array(Nullable(Float64)), tuples Array(Tuple(ts Int64, value Nullable(Float64))), position Nullable(Int64)) ENGINE=Memory"), + format!("INSERT INTO default.{table} VALUES (100,[10,20],[10,20],[(1,5.5)],-1),(200,[3.5],[3.5],[(1,NULL)],9),(300,[],[],[],1),(400,[7.5],[NULL],[(1,3.5)],1),(500,[1],[1],[(1,9.5)],NULL),(2000,[999],[999],[(1,999)],1)"), ] { let mut request = client.post(&clickhouse_url).body(sql); if let Some(user) = &user { request = request.basic_auth(user, password.as_ref()); } @@ -517,7 +517,7 @@ async fn array_sql_executes_local_element_after_typed_exact_leaf() { let body = response.text().await.unwrap(); assert!(status.is_success(), "native collection fixture: {body}"); } - let sql = format!("SELECT arrayElement(samples, position) AS result, arrayElement(nullable_samples, position) AS nullable_result FROM {table} WHERE timestamp >= 0 AND timestamp < 2000 ORDER BY result NULLS FIRST"); + let sql = format!("SELECT arrayElement(samples, position) AS result, arrayElement(nullable_samples, position) AS nullable_result, tupleElement(arrayElement(tuples, 1), 'value') AS tuple_result FROM {table} WHERE timestamp >= 0 AND timestamp < 2000 ORDER BY result NULLS FIRST"); let mut workload = mixed_workload(&sql); workload.tables = HashMap::from([( table.clone(), @@ -538,6 +538,22 @@ async fn array_sql_executes_local_element_after_typed_exact_leaf() { }, false, ), + Column::new( + "tuples", + DataType::List { + element: Box::new(Column::new( + "item", + DataType::Struct { + fields: vec![ + Column::new("ts", DataType::Int64, false), + Column::new("value", DataType::Float64, true), + ], + }, + false, + )), + }, + false, + ), Column::new("position", DataType::Int64, true), ], 0, @@ -554,11 +570,13 @@ async fn array_sql_executes_local_element_after_typed_exact_leaf() { .nodes .values() .any(|node| matches!(node, QueryPlanNode::ExternalExact { .. }))); - assert!(entry.nodes.values().any(|node| { - let QueryPlanNode::Relational { operation, .. } = node else { return false; }; - let ValueOperation::Project { cols, .. } = serde_json::from_value(operation.clone()).unwrap() else { return false; }; - cols.iter().any(|column| matches!(&column.expr, QueryExpr::FunctionCall { name, .. } if name == "asap_element_access")) - }), "Planner-selected DAG must preserve local element evaluation"); + for function in ["asap_element_access", "asap_struct_field"] { + assert!(entry.nodes.values().any(|node| { + let QueryPlanNode::Relational { operation, .. } = node else { return false; }; + let ValueOperation::Project { cols, .. } = serde_json::from_value(operation.clone()).unwrap() else { return false; }; + cols.iter().any(|column| matches!(&column.expr, QueryExpr::FunctionCall { name, .. } if name == function)) + }), "Planner-selected DAG must preserve local {function} evaluation"); + } eprintln!( "collection Planner selection: {}", serde_json::to_string(&trace).unwrap() @@ -628,11 +646,11 @@ async fn array_sql_executes_local_element_after_typed_exact_leaf() { let expected_rows = expected["data"].as_array().unwrap(); assert_eq!(actual_rows.len(), expected_rows.len()); for (actual, expected) in actual_rows.iter().zip(expected_rows) { - for field in ["result", "nullable_result"] { + for field in ["result", "nullable_result", "tuple_result"] { if expected[field].is_null() { assert!(actual[field].is_null()); } else { - // Both declared columns are Float64; JSON 20 and 20.0 encode + // The declared result columns are Float64; JSON 20 and 20.0 encode // the same value despite different serde_json Number variants. assert_eq!( actual[field].as_f64().unwrap().to_bits(), @@ -644,7 +662,7 @@ async fn array_sql_executes_local_element_after_typed_exact_leaf() { } assert_eq!( actual["data"], - serde_json::json!([{ "result":null, "nullable_result":null },{ "result":0.0, "nullable_result":null },{ "result":0.0, "nullable_result":null },{ "result":7.5, "nullable_result":null },{ "result":20.0, "nullable_result":20.0 }]) + serde_json::json!([{ "result":null, "nullable_result":null, "tuple_result":9.5 },{ "result":0.0, "nullable_result":null, "tuple_result":null },{ "result":0.0, "nullable_result":null, "tuple_result":null },{ "result":7.5, "nullable_result":null, "tuple_result":3.5 },{ "result":20.0, "nullable_result":20.0, "tuple_result":5.5 }]) ); let mut cleanup = client .post(&clickhouse_url) From ed7983df3c1fc84a5dc001e03e188db9dbdc1fa1 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 10:12:43 -0600 Subject: [PATCH 29/29] fix(storage): restore immutable admission after committed recovery --- .../sketch_db/index/maintenance.rs | 116 +++++++++++++++++- .../storage_engines/sketch_db/index/mod.rs | 54 +++++++- .../summary-catalog-sds-architecture.md | 7 +- 3 files changed, 170 insertions(+), 7 deletions(-) diff --git a/data_plane/src/storage_engines/sketch_db/index/maintenance.rs b/data_plane/src/storage_engines/sketch_db/index/maintenance.rs index bc5fa822d..f8a3403d5 100644 --- a/data_plane/src/storage_engines/sketch_db/index/maintenance.rs +++ b/data_plane/src/storage_engines/sketch_db/index/maintenance.rs @@ -163,10 +163,17 @@ impl SketchStore { .or_insert(window.1); return Ok(true); } - Ok(publisher + let found = publisher .lookup_immutable_window(&record, digest, window.0, window.1) .map_err(|error| error.to_string())? - .is_some()) + .is_some(); + if found { + completed + .entry(sid) + .and_modify(|end| *end = (*end).max(window.1)) + .or_insert(window.1); + } + Ok(found) } /// Publish a derived result without passing it through additive hot-state @@ -275,3 +282,108 @@ impl SketchStore { Ok(!published.already_published) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::precompute_engine::operators::SumAccumulator; + use crate::storage_engines::types::PrecomputedOutput; + use asap_types::traits::SerializableToSink; + + #[test] + fn committed_recovery_restores_live_seal_and_rejects_additive_derived_writes() { + // A durable sidecar may survive an I/O error before the caller updates + // its live seal. Recovery must repair admission, not only return a hit. + let snapshot: control_plane::physical::compiler::BackendLocalPlanningSnapshot = + serde_json::from_str(include_str!( + "../../../../../docs/examples/asapquery-planning-snapshot.json" + )) + .unwrap(); + let plan = snapshot.compile().unwrap(); + let mut source_config = plan.precompute_plan.materializations[0].clone(); + source_config.aggregation_type = asap_types::AggregationType::Sum; + source_config.aggregation_sub_type = "sum".into(); + let source_id = source_config.policy_fingerprint().into(); + let mut target = source_config.clone(); + target.derived_input = Some(asap_types::derived_input::DerivedInputIdentity { + inputs: BTreeSet::from([source_id]), + program_sha256: "0".repeat(64), + }); + let catalog = asap_types::summary_catalog::SummaryCatalog::from_materializations( + 1, + 1, + &[source_config, target.clone()], + ) + .unwrap(); + let store = Arc::new(SketchStore::new()); + store.install_summary_catalog(Arc::new(catalog)).unwrap(); + let directory = tempfile::tempdir().unwrap(); + let mut config = persistence::config::SketchStorePersistenceConfig::with_memory_limit( + 1 << 24, + directory.path().to_path_buf(), + ); + config.delete_older_than_ms = None; + config.hot_window_ms = None; + let mut persistence = store.start_persistence(config).unwrap(); + let generation = store.active_catalog_generation().unwrap(); + let mut output = PrecomputedOutput::new(0, 1000, None, target.policy_fingerprint()); + output.catalog_generation = Some(Arc::clone(&generation)); + store + .register_precompute_output(601, &target, &output) + .unwrap(); + let record = store + .metadata_record(&store.instances.read().unwrap()[&601]) + .unwrap(); + let state = SumAccumulator::new(); + let snapshot = persistence::source::EpochSnapshot { + agg_id: 601, + epoch_id: 0, + min_ts: 0, + max_ts: 1000, + approx_bytes: 0, + entries: vec![persistence::source::EpochSnapshotEntry { + start_ts: 0, + end_ts: 1000, + label: None, + sketch_type_name: state.type_name().into(), + encoding_tag: 0, + sketch_bytes: state.serialize_to_bytes(), + }], + }; + persistence + .flusher + .publish_immutable_window(&record, [7; 32], &snapshot) + .unwrap(); + assert!(!store.completed_windows.read().unwrap().contains_key(&601)); + let input = FrozenExactWindows { + sid: 600, + definition: source_id, + generation, + group: BTreeMap::new(), + windows: BTreeMap::new(), + }; + assert!(store + .recover_frozen_maintenance_output(601, &target, &input, [7; 32], (0, 1000)) + .unwrap()); + assert_eq!( + store.completed_windows.read().unwrap().get(&601), + Some(&1000) + ); + assert!(!store.append_precompute( + 601, + BTreeMap::new(), + (2000, 3000), + Box::new(SumAccumulator::new()) + )); + assert!(!store.append_sample( + 601, + BTreeMap::new(), + (2000, 3000), + SketchSampleState { + bytes: vec![], + encoding: SketchEncoding::MsgpackFull, + } + )); + persistence.shutdown(); + } +} 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 710a97f47..20f0cfc24 100644 --- a/data_plane/src/storage_engines/sketch_db/index/mod.rs +++ b/data_plane/src/storage_engines/sketch_db/index/mod.rs @@ -1265,6 +1265,27 @@ impl SketchStore { series_label_values: BTreeMap, window: TimestampRange, sample: SketchSampleState, + ) -> bool { + let instances = self.instances.read().unwrap(); + if instances.get(&sid).is_some_and(|binding| { + matches!( + binding.data_descriptor.source, + asap_types::sds::DataSourceIdentity::Derived { .. } + ) + }) { + return false; + } + self.append_sample_with_binding(sid, series_label_values, window, sample) + } + + // Caller retains the existing metadata guard and has rejected derived + // definitions. Avoid recursively acquiring it when a writer is waiting. + fn append_sample_with_binding( + &self, + sid: u64, + series_label_values: BTreeMap, + window: TimestampRange, + sample: SketchSampleState, ) -> bool { let completed = self.completed_windows.read().unwrap(); if completed.get(&sid).is_some_and(|end| window.1 <= *end) { @@ -1313,6 +1334,27 @@ impl SketchStore { series_label_values: BTreeMap, window: TimestampRange, payload: Box, + ) -> bool { + let instances = self.instances.read().unwrap(); + if instances.get(&sid).is_some_and(|binding| { + matches!( + binding.data_descriptor.source, + asap_types::sds::DataSourceIdentity::Derived { .. } + ) + }) { + return false; + } + self.append_precompute_with_binding(sid, series_label_values, window, payload) + } + + // Caller retains the existing metadata guard and has rejected derived + // definitions. Avoid recursively acquiring it when a writer is waiting. + fn append_precompute_with_binding( + &self, + sid: u64, + series_label_values: BTreeMap, + window: TimestampRange, + payload: Box, ) -> bool { let completed = self.completed_windows.read().unwrap(); if completed.get(&sid).is_some_and(|end| window.1 <= *end) { @@ -2844,7 +2886,13 @@ impl SketchStore { // recreate orphan payload after the tombstone commits. let instances = self.instances.read().ok()?; let binding = instances.get(&sid)?; - if !binding.metadata.is_writable() || binding.metadata.policy_fp != output.policy_fp { + if !binding.metadata.is_writable() + || binding.metadata.policy_fp != output.policy_fp + || matches!( + binding.data_descriptor.source, + asap_types::sds::DataSourceIdentity::Derived { .. } + ) + { return None; } if binding.catalog_generation.is_some() && output.catalog_generation.is_none() { @@ -2883,7 +2931,7 @@ impl SketchStore { let window = (output.start_timestamp, output.end_timestamp); let accepted = match crate::storage_engines::sketch_db::data::agg_kind_for_config(agg_cfg) { - AggKind::Sketch { .. } => self.append_sample( + AggKind::Sketch { .. } => self.append_sample_with_binding( sid, label_values_map, window, @@ -2892,7 +2940,7 @@ impl SketchStore { encoding: SketchEncoding::MsgpackFull, }, ), - AggKind::ExactAgg { .. } => self.append_precompute( + AggKind::ExactAgg { .. } => self.append_precompute_with_binding( sid, label_values_map, window, diff --git a/docs/design_docs/summary-catalog-sds-architecture.md b/docs/design_docs/summary-catalog-sds-architecture.md index f8ac9f702..53ed95e32 100644 --- a/docs/design_docs/summary-catalog-sds-architecture.md +++ b/docs/design_docs/summary-catalog-sds-architecture.md @@ -506,8 +506,11 @@ The engine resumes a matching durable pending part and looks up the stored input digest before computing a potentially randomized sketch. The existing flusher publishes a new result through its part reservation protocol; SummaryStore fences query reads and physical lifetime changes during publication. A concurrent identical completion reuses the durable -result instead of comparing newly randomized bytes. The latest committed window -can be retried after restart without adding another part. +result instead of comparing newly randomized bytes. Both pending recovery and a +committed lookup restore the live completion boundary. Catalog-derived definitions +reject additive sketch/precompute writes even beyond that boundary; only reserved +publication may create their output state. The latest committed window can be +retried after restart without adding another part. This is an explicit maintenance entry point, not automatic workload coverage. The initial consumer supports one source definition and population, complete