Skip to content

feat(planner): add manual windowing override - #643

Merged
milindsrivastava1997 merged 18 commits into
mainfrom
issue-555-windowing-override
Aug 28, 2026
Merged

feat(planner): add manual windowing override#643
milindsrivastava1997 merged 18 commits into
mainfrom
issue-555-windowing-override

Conversation

@milindsrivastava1997

Copy link
Copy Markdown
Contributor

Summary

  • add a global windowing override for PromQL and SQL planner inputs
  • support explicit tumbling/sliding candidates with per-query divisor validation
  • fail fast with aggregated validation errors and preserve query-log defaults
  • thread the option through experiment config generation and document it

Testing

  • 41 PromQL integration tests
  • 35 SQL integration tests
  • repository pre-commit hooks

Execution-side behavior remains out of scope; this PR only controls planner candidate generation.

Closes #555

@milindsrivastava1997

Copy link
Copy Markdown
Contributor Author

One real bug, three consistency/design gaps, one minor inefficiency.

Real bug (asap-planner-rs/src/planner/window.rs:93): apply_windowing_override checks window_size_ms % slide_divisor == 0 but never checks data_range_ms % window_size_ms == 0, which cleanup.rs's Sliding branch requires downstream. A realistic config (rate(x[5m]) at repetition_delay_ms: 45000) passes the override check but fails in cleanup.rs with a differently-worded error that doesn't match the "window_size_ms (" prefix the generators use to aggregate errors — so it aborts on the first bad query instead of collecting all of them, breaking the PR's own "report all invalid queries" feature. Both new test fixtures happen to avoid this case entirely.

Two related design issues in the same aggregation mechanism: the string-prefix match is duplicated across promql/generator.rs:109 and sql/generator.rs:102 with no shared constant, and binary-arm queries (promql/generator.rs:184) bypass the aggregation Vec entirely via ?.

Plus: SQLController::from_file_with_discovery (sql/controller.rs:40) skips windowing.validate() that every other constructor calls, and windowing.validate() runs redundantly up to 4x per plan generation (minor, not a correctness issue).

@milindsrivastava1997
milindsrivastava1997 marked this pull request as ready for review August 27, 2026 17:37
@milindsrivastava1997
milindsrivastava1997 merged commit c8e5ea3 into main Aug 28, 2026
17 checks passed
@milindsrivastava1997
milindsrivastava1997 deleted the issue-555-windowing-override branch August 28, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manual windowing override (tumbling/sliding) in config.yaml

1 participant