refactor: PromQL-only — remove SQL/ElasticDSL front-ends and internal DB tier - #302
Merged
Merged
Conversation
…QL-only) The control plane only ever shipped a working PromQL parser; the SQL front-end and the ElasticDsl stub are removed, leaving PromQL as the sole query language. The physical planner's internal 'Database' tier (DbQuery / Placement::Database / SqlTimeBucket / RawSamples) is removed; multi-intent / HAVING aggregates now route to an exact HashAggregate at the query engine. data_plane comments no longer advertise unsupported SQL/ClickHouse/Elasticsearch protocols (no such adapters existed; only prometheus_http.rs). cargo check --workspace and cargo test -p control_plane pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 20, 2026
…omQL-only) #302 removed the SQL/ElasticDSL front-ends but left `query_string_sql_populates_workload` (asserts `SELECT ... GROUP BY` parses into a workload) plus two "PromQL or SQL" doc comments in pipeline.rs. With SQL parsing gone the test fails. Removed the stale test and de-SQL'd the QuerySpec docs. Surfaced when rebasing the #301 counter-fn fix onto post-#302 main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 20, 2026
…oses #301, #300) (#304) * fix(query): honor counter-function semantics in ExactAgg dispatch (closes #301, #300) Post-#299 the agent streams per-window DELTAS for counters, but the asap engine collapsed `sum` / `sum_over_time` / `increase` / `rate` — four semantically-distinct PromQL counter idioms — onto the same two reducer paths, so all instant counter sums returned the same wrong number and `rate` was systematically under-reported. Four layered bugs: Layer 1 (engine dispatch): only `rate` vs everything-else branched; sum / sum_over_time / increase / instant-sum all hit one path. Layer 2 (lookback): instant sum used a 5-min default == the [5m] range, so on a <5min producer every idiom captured the same horizon. Layer 3 (instant projection): the instant branch took `samples.last()` — the MOST RECENT window's delta — instead of the cumulative-since-storage value PromQL `sum(counter)` requires. Layer 4 (rate divisor): `evaluate_exact_agg_rate` divided by the NOMINAL range (300 for [5m]) regardless of how much data actually covered the window, halving the rate when the producer ran < range. Fix: - control_plane: extend `OuterFn` from {Plain, Rate} to the full counter-fn taxonomy {Plain, Rate, Increase, SumOverTime}, populated by the analyzer's `trace_from_promql` walker (inner-counter-idiom wins for composed shapes like `sum by (..) (rate(..))`; precedence enforced by a new `set_counter_fn` helper). Carried on `ASAPTierCandidate.outer_fn`. - engine dispatch (instant + range surfaces): branch on the typed counter-fn — `Rate` → rate reducer; `Increase` → accumulate windows over the [t-r,t] clip into one cumulative number; `Plain` instant sum → accumulate ALL windows over the full storage horizon (t0=0) → cumulative-since-start; `SumOverTime` over a counter → capability-miss → archive (asap stores deltas and cannot reconstruct the Σ-of-cumulative-samples sum_over_time wants — issue #301 decision (a), subsumes #300). - reducer: `evaluate_exact_agg` gains an `accumulate_windows` flag that collapses each group's per-window deltas into ONE cumulative sample (Layer 3); the matrix/range surface keeps `accumulate_windows=false`. `evaluate_exact_agg_rate` now divides by `min(range_seconds, actual_coverage_span_seconds)` via a new `SketchStore::exact_agg_coverage_bounds(sid, t0, t1)` that reports the true `(min_window_start, max_window_end)` span (Layer 4). Tests: control_plane 783, data_plane 755 green. New unit + integration coverage pins each counter-fn semantic (instant-sum accumulates all windows not last; increase accumulates without divisor; sum_over_time capability-misses; rate divisor uses actual coverage). Multinode validation (4 zones, 10000 series @ 100 Hz; asap node2:9091 vs VictoriaMetrics baseline node2:8428), steady state: rate : asap 997,413 b0 999,987 rel-err 0.26% (was 64%) topk(rate): asap 997,413 (tracks rate; unchanged semantic) sum_over_time: asap capability-miss → archive (no longer fabricates) The four idioms now return distinct values (sum != increase != rate), confirming the dispatch no longer collapses them. Working queries (`quantile_over_time(0.99, ...)`, `max by (zone) (quantile_over_time)`) unaffected. Instant `sum` is cumulative-since-storage (runtime-dependent; residual gap is producer-runtime / flush-lag, not a dispatch bug). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove obsolete SQL query-string test + doc refs (post-#302 PromQL-only) #302 removed the SQL/ElasticDSL front-ends but left `query_string_sql_populates_workload` (asserts `SELECT ... GROUP BY` parses into a workload) plus two "PromQL or SQL" doc comments in pipeline.rs. With SQL parsing gone the test fails. Removed the stale test and de-SQL'd the QuerySpec docs. Surfaced when rebasing the #301 counter-fn fix onto post-#302 main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes the SQL and ElasticDSL query front-ends and the internal "Database" execution tier from the control plane + query backend, leaving PromQL as the only supported query language.
Why
The control plane only ever shipped a working PromQL parser. SQL was a parser slated for retirement;
ElasticDslwas a never-implemented stub. There are no ClickHouse/Elasticsearch query adapters indata_plane(onlyprometheus_http.rs). This aligns the code with what is actually supported.Changes
control_plane
query_parser/sql.rs;parse_query_expris PromQL-only; drop thesqlparserdep and the SQL tests.QueryLanguage = { PromQL }(removedSql,ElasticDsl).PhysicalOp::DbQuery,Placement::Database,PhysicalWindow::SqlTimeBucket,ExchangeFormat::RawSamples, and thewindow_fusionDatabase arm. Multi-intent / HAVING aggregates now route to an exactHashAggregateatQueryEngine.data_plane
Verification
cargo check --workspacepasses.cargo test -p control_planepasses (only pre-existing unused-import warnings remain).🤖 Generated with Claude Code