Skip to content

Commit f9e14fa

Browse files
added logic to remove stale states from precompute engine after reconfiguration
1 parent 05ab615 commit f9e14fa

2 files changed

Lines changed: 102 additions & 2 deletions

File tree

‎.design_docs/issue-242-design-decisions.md‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,30 @@ This means: if a metric was being precomputed and the planner decides it still s
105105

106106
---
107107

108-
## Decision 8: Stale precomputed data after config replace
108+
## Decision 8: Stale precomputed data and in-flight state after config replace
109109

110-
**Question:** When `streaming_config` is replaced, the `SimpleMapStore` may contain precomputed sketch data for aggregations that are no longer in the new config. Should this data be purged?
110+
**Question:** When `streaming_config` is replaced, how is in-flight precompute data handled, and what happens to stale state in both the store and the workers?
111+
112+
### In-flight data in precompute engine workers
113+
114+
`UpdateAggConfigs` replaces each worker's `agg_configs` map but **does not touch `group_states`**. Existing `GroupState` entries (keyed by `(agg_id, group_key)`) for old aggregation IDs are left in the worker's HashMap.
115+
116+
Consequences:
117+
- **Data is not silently dropped.** The flush loop iterates over *all* `group_states`, so old groups still close their open windows and emit accumulated sketch data to the `SimpleMapStore` — under the old aggregation IDs.
118+
- **No new samples reach old group states.** The ingest router only routes samples to agg IDs present in the new `agg_configs`, so old groups stop receiving input but still flush whatever they have accumulated.
119+
- **Memory is never reclaimed.** Old `GroupState` entries are never evicted from the worker's HashMap. For the one-shot first-plan scenario this is a fixed-size overhead. For repeated reconfigurations this is a latent memory leak — stale group states accumulate in worker memory until process restart.
120+
121+
The data committed to the store under old agg IDs becomes **orphaned**: the query engine now uses new inference/streaming configs pointing to new agg IDs, so those store entries are never queried and eventually expire via the cleanup policy.
122+
123+
**Implementation:** On `UpdateAggConfigs`, workers now prune `group_states` entries whose `agg_id` is absent from the new config, after a final forced flush. Specifically:
124+
1. Identify removed agg IDs (old `agg_configs` keys not present in `new_configs`).
125+
2. For each removed group state, run the window-close logic with `effective_wm = i64::MAX` — since no new samples will arrive, all open windows are treated as due regardless of watermark.
126+
3. Emit accumulated data from those windows to the store, then remove the entry from `group_states` and update `group_count`.
127+
4. Swap in `new_configs`.
128+
129+
The final flush happens **before** `agg_configs` is swapped so that `GroupState.config` (an `Arc<AggregationConfig>`) is still valid during window-bound calculations. The emitted data is still written under old agg IDs (and will not be queried), but this avoids silently discarding partially-accumulated windows and correctly frees the worker memory.
130+
131+
### Stale data already committed to `SimpleMapStore`
111132

112133
**Decision:** Let it expire naturally via the existing cleanup policy. No active purge.
113134

‎asap-query-engine/src/precompute_engine/worker.rs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,85 @@ impl Worker {
170170
break;
171171
}
172172
WorkerMessage::UpdateAggConfigs(new_configs) => {
173+
// Flush and evict group states for agg IDs that are being removed.
174+
// Must happen before swapping agg_configs so GroupState.config is
175+
// still valid during the final window close.
176+
let removed_ids: Vec<u64> = self
177+
.agg_configs
178+
.keys()
179+
.filter(|id| !new_configs.contains_key(id))
180+
.copied()
181+
.collect();
182+
183+
if !removed_ids.is_empty() {
184+
let mut emit_batch: Vec<(PrecomputedOutput, Box<dyn AggregateCore>)> =
185+
Vec::new();
186+
187+
for agg_id in &removed_ids {
188+
// Drain all group states for this agg_id.
189+
let removed_keys: Vec<_> = self
190+
.group_states
191+
.keys()
192+
.filter(|(id, _)| id == agg_id)
193+
.cloned()
194+
.collect();
195+
196+
for key in removed_keys {
197+
let Some(state) = self.group_states.remove(&key) else {
198+
continue;
199+
};
200+
if state.previous_watermark_ms == i64::MIN {
201+
continue; // No samples received — nothing to emit.
202+
}
203+
// Force-close all open windows by advancing the watermark
204+
// to i64::MAX. No new samples will arrive for this group.
205+
let (group_key_str, mut active_panes) =
206+
(key.1.clone(), state.active_panes);
207+
let closed = state
208+
.window_manager
209+
.closed_windows(state.previous_watermark_ms, i64::MAX);
210+
211+
for window_start in &closed {
212+
let (_, window_end) =
213+
state.window_manager.window_bounds(*window_start);
214+
let pane_starts =
215+
state.window_manager.panes_for_window(*window_start);
216+
if let Some(accumulator) =
217+
merge_panes_for_window(&mut active_panes, &pane_starts)
218+
{
219+
let group_key_lv =
220+
build_group_key_label_values(&group_key_str);
221+
let output = PrecomputedOutput::new(
222+
*window_start as u64,
223+
window_end as u64,
224+
Some(group_key_lv),
225+
*agg_id,
226+
);
227+
emit_batch.push((output, accumulator));
228+
}
229+
}
230+
}
231+
}
232+
233+
if !emit_batch.is_empty() {
234+
if let Err(e) = self.output_sink.emit_batch(emit_batch) {
235+
warn!(
236+
"Worker {}: error flushing removed agg_ids {:?}: {}",
237+
self.id, removed_ids, e
238+
);
239+
}
240+
}
241+
242+
self.group_count
243+
.store(self.group_states.len(), Ordering::Relaxed);
244+
info!(
245+
"Worker {}: evicted {} removed agg_id(s) {:?}",
246+
self.id,
247+
removed_ids.len(),
248+
removed_ids,
249+
);
250+
}
251+
173252
let added = new_configs.len().saturating_sub(self.agg_configs.len());
174253
self.agg_configs = new_configs;
175254
info!(

0 commit comments

Comments
 (0)