Skip to content

Commit 685655e

Browse files
zzylolclaude
andcommitted
Fix sliding window aggregation: feed samples into all overlapping windows
Previously each sample was assigned to only one window via window_start_for(), which is incorrect for sliding windows where window_size > slide_interval. Added window_starts_containing() that returns all window starts whose range covers the timestamp, and use it in the worker aggregation loop. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b749ac3 commit 685655e

2 files changed

Lines changed: 58 additions & 12 deletions

File tree

asap-query-engine/src/precompute_engine/window_manager.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,20 @@ impl WindowManager {
7272
closed
7373
}
7474

75+
/// Return all window starts whose window `[start, start + window_size_ms)`
76+
/// contains the given timestamp. For tumbling windows this returns exactly
77+
/// one start; for sliding windows it returns `ceil(window_size / slide)`
78+
/// starts.
79+
pub fn window_starts_containing(&self, timestamp_ms: i64) -> Vec<i64> {
80+
let mut starts = Vec::new();
81+
let mut start = self.window_start_for(timestamp_ms);
82+
while start + self.window_size_ms > timestamp_ms {
83+
starts.push(start);
84+
start -= self.slide_interval_ms;
85+
}
86+
starts
87+
}
88+
7589
/// Return the window `[start, end)` boundaries for a given window start.
7690
pub fn window_bounds(&self, window_start: i64) -> (i64, i64) {
7791
(window_start, window_start + self.window_size_ms)
@@ -150,4 +164,34 @@ mod tests {
150164
let closed = wm.closed_windows(15_000, 35_000);
151165
assert_eq!(closed, vec![0]);
152166
}
167+
168+
#[test]
169+
fn test_window_starts_containing_tumbling() {
170+
// 60s tumbling windows — each sample belongs to exactly one window
171+
let wm = WindowManager::new(60, 0);
172+
let mut starts = wm.window_starts_containing(15_000);
173+
starts.sort();
174+
assert_eq!(starts, vec![0]);
175+
176+
let mut starts = wm.window_starts_containing(60_000);
177+
starts.sort();
178+
assert_eq!(starts, vec![60_000]);
179+
}
180+
181+
#[test]
182+
fn test_window_starts_containing_sliding() {
183+
// 30s window, 10s slide — each sample belongs to 3 windows
184+
let wm = WindowManager::new(30, 10);
185+
186+
// t=15_000 belongs to [0, 30_000), [10_000, 40_000)
187+
// and [-10_000, 20_000) which starts negative — still returned
188+
let mut starts = wm.window_starts_containing(15_000);
189+
starts.sort();
190+
assert_eq!(starts, vec![-10_000, 0, 10_000]);
191+
192+
// t=30_000 belongs to [10_000, 40_000), [20_000, 50_000), [30_000, 60_000)
193+
let mut starts = wm.window_starts_containing(30_000);
194+
starts.sort();
195+
assert_eq!(starts, vec![10_000, 20_000, 30_000]);
196+
}
153197
}

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -221,18 +221,20 @@ impl Worker {
221221
continue; // already dropped
222222
}
223223

224-
let window_start = agg_state.window_manager.window_start_for(ts);
225-
226-
let updater = agg_state
227-
.active_windows
228-
.entry(window_start)
229-
.or_insert_with(|| create_accumulator_updater(&agg_state.config));
230-
231-
if updater.is_keyed() {
232-
let key = extract_key_from_series(series_key, &agg_state.config);
233-
updater.update_keyed(&key, val, ts);
234-
} else {
235-
updater.update_single(val, ts);
224+
let window_starts = agg_state.window_manager.window_starts_containing(ts);
225+
226+
for window_start in window_starts {
227+
let updater = agg_state
228+
.active_windows
229+
.entry(window_start)
230+
.or_insert_with(|| create_accumulator_updater(&agg_state.config));
231+
232+
if updater.is_keyed() {
233+
let key = extract_key_from_series(series_key, &agg_state.config);
234+
updater.update_keyed(&key, val, ts);
235+
} else {
236+
updater.update_single(val, ts);
237+
}
236238
}
237239
}
238240

0 commit comments

Comments
 (0)