Repository navigation
Support sliding window candidates in the query engine (execution) #557
Description
Sub-issue of #554.
Scope: actually serving queries correctly against sliding-window aggregations — the execution-side work, separate from generating valid candidates (#554's other sub-issue, #556).
Concretely:
- Relax
capability_matching.rs'swindow_compatible()Sliding rule beyond exact-match (data_range_ms == window_size_ms) to allowdata_range_msa multiple ofwindow_size_ms, mirroring Tumbling's existing rule. - Implement merge (mergeable sketch types) / subtract (subtractable types) across multiple stored sliding-window buckets in
simple_engine's query-serving path (create_store_query_plan/execute_and_merge_store_queriesinsimple_engine/mod.rs, which today unconditionally does a single exact-match lookup for Sliding and explicitly skips merging). - Update
planner/cleanup.rs'sWindowType::Slidingbranch to be window-count-aware instead of hardcodingread_count_threshold = 1. - Resolve the epoch/slide-interval alignment question for exact-match lookups (
align_end_timestamp_promqlonly aligns to the scrape interval, not the aggregation'sslide_interval_ms). - Owns the end-to-end correctness test (plan → run → query a sliding-window aggregation → assert correct results) — Manual windowing override (tumbling/sliding) in config.yaml #555 explicitly defers this test here.
Needs its own design session — not scoped further here. See #554 and the design docs linked there for context.
Metadata
Metadata
Assignees
Labels
No labels
Activity
docs(design): sliding-window query engine execution design (#557)
fix(query-engine): sliding windows validate step against slide interval
milindsrivastava1997 commentedon Aug 23, 2026
When sliding-window merging lands here, worth removing `do_merge` entirely rather than keeping it around.
Context from PR #582 review: `do_merge` is computed once, early, from the original single-instant window comparison, and never recomputed when a range query's fetch window gets widened later (`finish_range_context`). `merge_precomputed_outputs`'s `do_merge=false` branch used to hard-assert exactly one precompute per key; #582 had to loosen that to a `warn!` + merge-anyway because ordinary widened range queries were legitimately tripping the assert. That's a stale-flag bug, not a real invariant violation — once sliding windows can also be merged, `do_merge` stops being a meaningful gate and the merge path should just always run (or the flag should be recomputed against the actual fetch window, if some queries still need to skip merging).
milindsrivastava1997 commentedon Aug 26, 2026
Finding from scoping #608: PromQL selector duration (the
[Ns]text) is currently ignored for Sliding aggregations — the aggregation's own configuredwindow_size_msis always authoritative instead.For Sliding aggregations,
create_store_query_plan(mod.rs:432-436) computes the instant window as[end - window_size_ms, end]— using the aggregation's configuredwindow_size_ms, not whatever duration you wrote in the selector. Andfinish_range_context's lookback (widen_query_window,promql.rs:554) derives from that same instant window's width — so it'swindow_size_mstoo, transitively.So e.g. with
window_size_ms=5000,slide_interval_ms=1000, a query written as[10s]: the system doesn't actually use 10 seconds. It silently substitutes the aggregation's real configured window — 5 seconds — regardless of what's typed in brackets.sum_over_time(cpu_load[10s])andsum_over_time(cpu_load[5s])produce identical results against this aggregation today.Practical consequence: a "merge N non-overlapping sub-window buckets to answer a wider-than-window_size_ms lookback" scenario can't actually arise in this system as built — there's no code path that ever asks a Sliding aggregation for a lookback wider than its own
window_size_ms. Confirmed this keeps #608's fix scoped to a single-bucket lookup rather than a general multi-bucket merge.Separately — silently ignoring a selector duration that doesn't match the aggregation's configured window is arguably its own correctness question (should this validate/error instead of silently substituting?), unrelated to #608 — flagging here since #557 is the broader sliding-window-execution issue, not filing a new issue for it unless you want one.
milindsrivastava1997 commentedon Aug 26, 2026
While scoping #620 (variable naming), found a gap in this issue's own premise.
The gap: this issue's write-up (and #554's) says the Sliding exact-match rule is "enforced live, on every query, via
find_compatible_aggregation()." That's only true when a query reaches capability matching. There's a second path that skips it entirely:build_promql_execution_context_tail(simple_engine/promql.rs:1178) first triesfind_query_config()— a pre-boundquery_configsentry that maps a literal query string straight to an aggregation ID. If it hits,get_aggregation_id_info()(simple_engine/mod.rs:995) resolves that binding directly andfind_compatible_aggregation()— the function that owns thedata_range_ms == window_size_mscheck — never runs.Concrete effect today: for any query pre-bound via
query_configs,create_store_query_plan's Sliding branch (mod.rs:432-440) usesend_timestamp - window_size_msfor the fetch window no matter what range the query asked for.sum_over_time(cpu_load[10s])andsum_over_time(cpu_load[5s])return identical results against a 5s-configured Sliding aggregation — not an error, not a rejection, just silently the wrong window.Why this matters for #557's fix: the merge/subtract work here needs to reach queries through both entry points — the
find_compatible_aggregation()path this issue already targets, and thefind_query_config()fast path, which currently has no equivalent check at all. Worth keeping in mind when relaxingwindow_compatible()/ updatingcreate_store_query_plan, so the fix doesn't leave the pre-configured path still silently wrong.