Skip to content

Commit 841b853

Browse files
g-talbotclaude
andcommitted
fix: include window_duration_secs in CompactionScope
CompactionScope only used window_start_secs, so splits with the same start but different durations (e.g. after a window config change) would be grouped together. The merge engine requires all inputs to agree on both window_start and window_duration, so merging across durations would fail validation. Added test_different_window_duration which caught the bug before the fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e4e49d8 commit 841b853

1 file changed

Lines changed: 26 additions & 0 deletions

File tree

  • quickwit/quickwit-parquet-engine/src/merge/policy

quickwit/quickwit-parquet-engine/src/merge/policy/scope.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ pub struct CompactionScope {
4545
pub sort_fields: String,
4646
/// Window start in epoch seconds.
4747
pub window_start_secs: i64,
48+
/// Window duration in seconds. The merge engine requires all inputs to
49+
/// agree on both start and duration, so a config change that alters the
50+
/// window size must not cause old and new splits to be merged.
51+
pub window_duration_secs: i64,
4852
}
4953

5054
impl CompactionScope {
@@ -59,6 +63,7 @@ impl CompactionScope {
5963
partition_id: split.partition_id,
6064
sort_fields: split.sort_fields.clone(),
6165
window_start_secs: window.start,
66+
window_duration_secs: window.end - window.start,
6267
})
6368
}
6469
}
@@ -170,6 +175,24 @@ mod tests {
170175
assert!(result.is_empty());
171176
}
172177

178+
#[test]
179+
fn test_different_window_duration() {
180+
// Bug: only window_start was in the scope key, so splits with the
181+
// same start but different durations (e.g. after a config change)
182+
// would be grouped together. The merge engine requires all inputs
183+
// to agree on both window_start and window_duration.
184+
let sort = "metric_name|host|timestamp/V2";
185+
let splits = vec![
186+
test_split("idx:001", sort, Some(0), 900), // 0..900
187+
test_split("idx:001", sort, Some(0), 1800), // 0..1800
188+
];
189+
let result = group_by_compaction_scope(splits);
190+
assert!(
191+
result.is_empty(),
192+
"different window durations must not be grouped together"
193+
);
194+
}
195+
173196
#[test]
174197
fn test_different_window_start() {
175198
let splits = vec![
@@ -227,12 +250,14 @@ mod tests {
227250
partition_id: 0,
228251
sort_fields: "metric_name|host|timestamp/V2".to_string(),
229252
window_start_secs: 1000,
253+
window_duration_secs: 3600,
230254
};
231255
let scope_b = CompactionScope {
232256
index_uid: "idx:001".to_string(),
233257
partition_id: 0,
234258
sort_fields: "metric_name|host|timestamp/V2".to_string(),
235259
window_start_secs: 4600,
260+
window_duration_secs: 3600,
236261
};
237262

238263
assert_eq!(result[&scope_a].len(), 3);
@@ -286,5 +311,6 @@ mod tests {
286311
assert_eq!(scope.partition_id, 7);
287312
assert_eq!(scope.sort_fields, "metric_name|host|timestamp/V2");
288313
assert_eq!(scope.window_start_secs, 7200);
314+
assert_eq!(scope.window_duration_secs, 3600);
289315
}
290316
}

0 commit comments

Comments
 (0)