Skip to content

planner/optimizer: support partial-width Sliding windows merged/subtracted together; re-check Sliding's freshness guard #422

Closed

Description

@milindsrivastava1997

planner/optimizer — candidate_gen.rs::window_candidates only ever generates Sliding candidates with W = range_a exactly (line ~136), queried via Direct (no merge/subtract, since n_windows is always 1 for Sliding).

Missing candidate class. A Sliding accumulator with W < range_a (say W = range_a / k for integer k) refreshes every slide_interval, just like a full-width one — so you can merge/subtract k of these partial-width staggered readings to cover the full range_a, at lower ingest cost than a full-width Sliding window (fewer concurrent accumulators: ⌈W/S⌉ instead of ⌈range_a/S⌉), while still refreshing far more often than an equivalent Tumbling+Merge setup (whose freshness is bounded by its window width, not by the slide interval).

Concrete example: range_a = 10m, t_repeat = 30s.

  • Tumbling, w=30s: cheap ingest (1 concurrent accumulator + 20 retained), but a 20-way merge per query.
  • Sliding, size=10m, slide=30s: zero query cost (Direct), but ⌈600/30⌉ = 20 concurrent accumulators — the most expensive ingest option.
  • Sliding, size=5m, slide=30s, merging 2 staggered 5m readings: ⌈300/30⌉ = 10 concurrent accumulators (half the ingest cost of the previous option) plus 1 retained reading, with only a 2-way merge per query.

That third option is a real point in the cost/freshness tradeoff space that the current grid search never generates. Implementing it needs:

  • window_candidates to also emit Sliding candidates with W = range_a / k for valid k, alongside the existing W = range_a candidates.
  • determine_query_method to allow Merge/Subtract for Sliding with n_windows > 1 (currently guarded by debug_assert_eq!(window_type, WindowType::Tumbling) whenever n_windows > 1, since this case never arises today).
  • cost_model.rs::ingest_cost's mem_retain term, which is hardcoded to 0.0 for Sliding ("already counted in mem_active's concurrent windows") — that assumption breaks once a Sliding candidate needs to retain a prior completed reading to merge against the current one.

Separately, a likely-backwards guard. window_candidates only generates any Sliding candidate at all when range_a <= min_t_repeat_secs (line ~133) — i.e. it gates on the full window width W (since W = range_a today) being within the freshness bound. But the whole point of a sliding window is that fresh answers arrive every slide_interval, not every W seconds — once warmed up, freshness should be bounded by the slide interval, not by W. As written, this guard looks like it would incorrectly reject exactly the case Sliding is meant for: a wide window (W large) refreshed frequently (slide small). Worth re-deriving what the correct guard should be once the above grid-search extension is scoped, since both pieces touch the same freshness reasoning.

Found during code review of PR #407.

Activity

  1. milindsrivastava1997 commented on Jun 23, 2026

    @milindsrivastava1997
    ContributorAuthor

    Follow-up from auditing window_candidates against the stated tumbling/sliding spec — two more deviations in the same function, same root cause as the freshness-guard issue above:

    1. Inner slide-bound is wrong too, not just the outer guard. The slide loop (let mut s = scrape_interval_secs; while s <= range_a { ...; s *= 2 }, around line 135) bounds S by s <= range_a — i.e. slide ≤ window width. The spec says slide should be bounded by min_t_repeat_secs (freshness), not by range_a/W. This is the same confusion as the outer guard (checking freshness against W instead of S), just showing up a second time. Both bounds need correcting together when this gets reworked.

    2. Sliding's window size is never checked to be a multiple of scrape_interval. Since W is hardcoded to range_a exactly, the code never verifies range_a is itself scrape-aligned. If a query's range isn't a multiple of the scrape interval, a Sliding candidate still gets generated anyway, silently violating that part of the spec. This becomes directly relevant once partial-width candidates (W = range_a / k) are added above, since at that point the size-vs-scrape-interval check needs to actually run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions