Skip to content

fix(store + engine): closest-pane window queries + precompute_window annotation - #71

Merged
zzylol merged 2 commits into
mainfrom
fix/store-range-query-overlap-filter
May 1, 2026
Merged

zzylol merged 2 commits into
mainfrom
fix/store-range-query-overlap-filter

Conversation

@zzylol

@zzylol zzylol commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Two related changes — a store-side filter fix and an engine/adapter change — both addressing the original symptom (PromQL queries returning empty even though the store has data) and the user follow-up ("answer with a single closest window and annotate which one").

PromQL queries against sketch-aggregated metrics returned empty even though the precompute store demonstrably had data — worker logs Worker emitting 1 sketch outputs for group (1, ), runtime-info earliest_timestamp_per_aggregation_id populates, yet the engine reports No precomputed outputs found.

Part 1 — Overlap filter (commit a7ca91d)

MutableEpoch::range_query_into and SealedEpoch::range_query_into in simple_map_store/common.rs (and the on-disk parts variant in per_key.rs:query_disk_parts) used a "fully contained" filter:

if tr.0 < start || tr.0 > end || tr.1 > end {
    continue;  // pane must satisfy: start ≤ tr.0 ≤ tr.1 ≤ end
}

For tumbling windows of size W with a query range of size R, this matches at most floor(R / W) panes — and only when both query endpoints land exactly on the pane grid. PromQL queries don't align: quantile_over_time(...[1m]) with prometheus_scrape_interval=30 and a pane size of 30s, the query range [query_time - 60s, query_time] is offset from the 30s grid by the wall-clock fractional portion of query_time, so neither of the two panes that should match is fully contained — both have tr.0 < start (they straddle the lower boundary) or tr.1 > end (they straddle the upper boundary). Result: empty.

Fix: standard half-open interval overlap test. A window [tr.0, tr.1) overlaps the query range [start, end) when tr.1 > start && tr.0 < end. Skip iff neither (i.e. tr.1 <= start || tr.0 >= end).

Same change in three places that all share the same dead semantics:

  • common.rs::MutableEpoch::range_query_into (columnar, used by current/in-flight epoch).
  • common.rs::SealedEpoch::range_query_into (sorted-entries variant, used by older sealed epochs).
  • per_key.rs::query_disk_parts (on-disk parts iteration — matches the in-memory contract).

The SealedEpoch variant kept its partition_point binary-search upper bound but was rewritten to bound by tr.0 < end instead of tr.0 < start — entries where tr.0 < start can now overlap (when tr.1 > start).

Part 2 — Closest-pane + annotation (commit 86aafbc)

After Part 1, the engine could merge 2-3 overlapping panes. That works for sum/count but is slightly imprecise for sketch summaries — the merged sketch covers more time than the query asked for — and the caller has no way to see which time range was actually consulted.

Per follow-up review feedback, for window queries the engine now picks a single closest pane and annotates the response with the actual [start_ms, end_ms) it used.

Pick the closest pane

In execute_and_merge_store_queries, after the store returns overlapping panes, compute the global "closest" pane (max tr.1, tie-break on max tr.0 — the latest pane that overlaps the request range). Then:

  • Tumbling case: keep only that pane per series, run through the existing merge path (no-op for a single bucket but preserves accumulator-side cleanup).
  • Sliding case: untouched — was already exact-match per key.

Thread window_used through QueryResult

InstantVector and RangeVector get a new window_used: Option<(u64, u64)> field, paired with QueryResult::with_window_used (chainable like with_accuracy).

execute_query_pipeline returns (elements, window_used). execute_context attaches the window onto the QueryResult. The schema-timeline dispatch path drops the per-segment window because a combined-result spans multiple agg_ids/windows; only the single-agg path surfaces it.

Annotate the Prometheus HTTP response

PrometheusResponse::with_precompute_window((start, end)) pushes a human-readable line onto the existing infos array:

precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)

Mirrors the with_accuracy pattern — no new top-level field on the wire, just an item in the existing infos. Grafana 11+ already renders these inline.

Verification (live end-to-end)

b3-delta agent (60s window, 30s tumbling panes), querying after 3+ window flushes have populated the store:

offset=-90s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-75s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-60s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-45s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-30s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)

Same value and same precompute_window across the offsets — engine consistently picked the same closest pane ([17:08:00, 17:08:30)) and reported it. Pre-fix queries returned result: []. Pre-Part-2 the value would have been right but infos carried only the accuracy line; caller had to guess which range produced the value.

A query at now-30s or now-0s correctly empty when the most recent pane is younger than the query end — the agent hasn't flushed it yet (timing, not the filter, expected for [1m] queries with a 60s agent window + 30s pane that close 60-90s after the data they cover).

Test plan

  • PromQL quantile_over_time(0.5, http_requests_total_latency_ms_quantile[1m]) returns non-empty at appropriate offsets.
  • All four runtime range checks at now-120s, now-90s, now-60s return real values; now-30s, now-0s correctly empty due to data not yet emitted.
  • infos array contains a precompute_window: [start, end) ms (width N ms) line for every non-empty result.
  • Same query at adjacent offsets returns the same precompute_window (engine deterministically picks the same closest pane).

🤖 Generated with Claude Code

zzylol and others added 2 commits May 1, 2026 12:48
…tained

PromQL queries against sketch-aggregated metrics returned empty
even though the precompute store demonstrably had data — worker
logs `Worker emitting 1 sketch outputs for group (1, )`,
runtime-info `earliest_timestamp_per_aggregation_id` populates,
yet the engine reports `No precomputed outputs found`.

## Root cause

`MutableEpoch::range_query_into` and `SealedEpoch::range_query_into`
in `simple_map_store/common.rs` (and the on-disk parts variant in
`per_key.rs:query_disk_parts`) used a "fully contained" filter:

```rust
if tr.0 < start || tr.0 > end || tr.1 > end {
    continue;  // pane must satisfy: start ≤ tr.0 ≤ tr.1 ≤ end
}
```

For tumbling windows of size W with a query range of size R, this
matches at most floor(R / W) panes — and only when both query
endpoints land exactly on the pane grid. PromQL queries don't
align: `quantile_over_time(...[1m])` with `prometheus_scrape_interval=30`
and a pane size of 30s, the query range
`[query_time - 60s, query_time]` is offset from the 30s grid by
the wall-clock fractional portion of `query_time`, so neither
of the two panes that should match is fully contained — both
have `tr.0 < start` (they straddle the lower boundary) or
`tr.1 > end` (they straddle the upper boundary). Result: empty.

This is not specific to PromQL — any query whose endpoints
don't align to the window grid hits the same wall.

## Fix

Replace fully-contained with the standard half-open interval
overlap test: a window `[tr.0, tr.1)` overlaps the query range
`[start, end)` when `tr.1 > start && tr.0 < end`. Skip iff
neither (i.e. `tr.1 <= start || tr.0 >= end`).

Same change in three places that all share the same dead
semantics:
- `common.rs::MutableEpoch::range_query_into` (columnar, used by
  current/in-flight epoch).
- `common.rs::SealedEpoch::range_query_into` (sorted-entries
  variant, used by older sealed epochs).
- `per_key.rs::query_disk_parts` (on-disk parts iteration —
  matches the in-memory contract).

Note the `SealedEpoch` variant retained its
`partition_point(tr.0 < start)` upper-bound binary search but
was rewritten to bound by `tr.0 < end` instead, since entries
where `tr.0 < start` *can* now overlap (when `tr.1 > start`).

## Trade-off acknowledged in the comments

A pane that crosses the query boundary contributes its sketch
state — including data points slightly outside `[start, end)` —
to the merged result. For sketch-based aggregations this is the
right trade vs. silently returning empty: the alternative is to
either align query timestamps to the grid (changing PromQL
semantics: a query at 16:43:54 reports data ending at 16:43:30)
or to require callers to pre-align their ranges. Both are worse
ergonomics than slightly-imprecise sketch values at the edges.

## Verification

End-to-end with b3-delta agent (60s agent window, 30s tumbling
panes), querying after 3+ window flushes have populated the store:

```
$ curl 'http://localhost:19091/api/v1/query?query=quantile_over_time(0.5, http_requests_total_latency_ms_quantile[1m])&time=$(now-90s)'
{"data":{"result":[{"metric":{"node":""},
                    "value":[1777653931.0, "9.488447485932145"]}],
         "resultType":"vector"},
 "accuracy":{"epsilon":0.01,"kind":"relative_quantile"}}
```

Pre-fix at the same offset: `result: []` with backend log
`No precomputed outputs found for metric: ..., aggregation_id: 1`.

A query at `now-30s` or `now-0s` still returns empty when the
most recent pane is younger than the query end — the agent
hasn't flushed it yet (this is timing, not the filter, and is
fine for `[1m]` queries that land 60-90s after a window closes).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #71's overlap-filter change made `range_query_into` return
every pane that intersected the query range. The merge path
then merged 2-3 panes per series, which works for sum/count but
gives a slightly wrong answer for sketch summaries — the merged
sketch covers more time than the query asked for, and the user
has no way to see which time range was actually consulted.

Per user request: for a window query, pick the *closest single
window* and annotate the response with which one it was.

## Changes

### Pick the closest pane

In `execute_and_merge_store_queries`, after the store returns
all overlapping panes, compute the global "closest" pane (max
`tr.1`, tie-break on max `tr.0` — i.e. the latest pane that
overlaps the request range). Then:

- Tumbling case: keep only that pane per series, run through the
  existing merge path (which is a no-op for a single bucket but
  preserves accumulator-side cleanup).
- Sliding case: untouched — was already exact-match per key.

The chosen `(start_ms, end_ms)` is bubbled out as a third
element of the result tuple.

### Thread `window_used` through QueryResult

`InstantVector` and `RangeVector` get a new `window_used:
Option<(u64, u64)>` field, paired with `QueryResult::with_window_used`
(chainable like `with_accuracy`).

`execute_query_pipeline` returns `(elements, window_used)`.
`execute_context` attaches the window onto the `QueryResult`.
The schema-timeline dispatch path drops the per-segment window
because a combined-result spans multiple agg_ids/windows; only
the single-agg path surfaces it.

### Annotate the Prometheus HTTP response

`PrometheusResponse::with_precompute_window((start, end))`
pushes a human-readable line onto the existing `infos` array:

```
precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
```

Mirrors the `with_accuracy` pattern — no new top-level field on
the wire, just an item in the existing `infos`. Grafana 11+
already renders these inline.

## Verification

End-to-end with b3-delta agent (60s window, 30s tumbling pane):

```
$ for offset in -90 -75 -60 -45 -30; do curl …time=$(now$offset)…; done
offset=-90s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-75s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
…
offset=-30s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
```

Same `value` and same `precompute_window` across the offsets —
the engine consistently picked the same closest pane
([17:08:00, 17:08:30)) and reported it. Pre-fix, `infos` only
carried the `accuracy` line; the caller had to guess which time
range produced the value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol

zzylol commented May 1, 2026

Copy link
Copy Markdown
Contributor Author

Update — closest-pane + response annotation (per @user feedback)

Pushed an additional commit: 86aafbc fix(engine): pick closest pane for window queries + annotate response.

The previous overlap fix in this PR returned all overlapping panes and merged them, which works numerically for sum/count but is slightly imprecise for sketch summaries (the merged sketch spans more time than the query asked for) and gave the caller no visibility into the actual time range consulted.

This update changes that:

  1. Single closest pane. In execute_and_merge_store_queries, after the store returns all overlapping panes, compute the latest pane (max tr.1, tie-break on max tr.0) and keep only that pane per series before running through the merge path. Sliding-window queries are unchanged (they were already exact-match).
  2. window_used on QueryResult. New Option<(u64, u64)> field on InstantVector / RangeVector paired with chainable QueryResult::with_window_used. Mirrors the with_accuracy pattern.
  3. Annotate the response. PrometheusResponse::with_precompute_window((start, end)) pushes one line onto the existing infos array:
precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)

No new top-level field on the wire; Grafana 11+ already renders infos inline.

Verification (live e2e)

Sweeping query offsets from -90s to -30s:

offset=-90s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-75s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-60s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-45s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)
offset=-30s → value=19.493849507395904
              precompute_window: [1777655280000, 1777655310000) ms (width 30000 ms)

Same value and same precompute_window — engine consistently picked the same closest pane ([17:08:00, 17:08:30)) and reported it. Pre-update, infos only carried the accuracy line; the caller had to guess which range produced the value.

@zzylol zzylol changed the title fix(store): range_query_into uses overlap filter instead of fully-contained fix(store + engine): closest-pane window queries + precompute_window annotation May 1, 2026
@zzylol
zzylol merged commit 1899040 into main May 1, 2026
@zzylol
zzylol deleted the fix/store-range-query-overlap-filter branch May 1, 2026 17:16
zzylol added a commit that referenced this pull request May 1, 2026
Capture this session's merged backend work — #70 (tonic OTLP
gRPC max_decoding_message_size bumped to 64 MiB) and #71 (store
range_query_into overlap filter + engine closest-pane selection
+ Prometheus-adapter precompute_window annotation) — at the top
of TODO.md so the runtime-warm-tier-actually-works claim is
testable from the doc.

Cited the matching collector-side PRs (ASAPCollector#210 +
#211) in the companion-changes note so future readers can see
both halves of the wire fix.

Added one new entry under "Known reconciliation gap":
`IngestState.sketch_snapshots` is RAM-only, so backend restarts
break delta ingest until the agent restarts too. Same item is
mirrored in the collector's PROGRESS.md follow-up list — fix on
either side closes the gap.

`_Last updated_` set to 2026-05-01.

Docs only; no code changes.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 2, 2026
* docs(todo): sync after 2026-05-01 query-path-closes-the-loop work (#72)

Capture this session's merged backend work — #70 (tonic OTLP
gRPC max_decoding_message_size bumped to 64 MiB) and #71 (store
range_query_into overlap filter + engine closest-pane selection
+ Prometheus-adapter precompute_window annotation) — at the top
of TODO.md so the runtime-warm-tier-actually-works claim is
testable from the doc.

Cited the matching collector-side PRs (ASAPCollector#210 +
#211) in the companion-changes note so future readers can see
both halves of the wire fix.

Added one new entry under "Known reconciliation gap":
`IngestState.sketch_snapshots` is RAM-only, so backend restarts
break delta ingest until the agent restarts too. Same item is
mirrored in the collector's PROGRESS.md follow-up list — fix on
either side closes the gap.

`_Last updated_` set to 2026-05-01.

Docs only; no code changes.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: retire sketch-core mirror (#73)

* refactor: retire sketch-core mirror

* refactor: switch consumer imports to asap_sketchlib::sketches::*

Update PR #73 against the reorganized asap_sketchlib (PR #36): the
runtime sketches no longer live under a dedicated `asap::` module — they
were merged into the existing `src/sketches/` layout (single home per
sketch concept, ASAP-runtime types appended to the file that already
holds the high-throughput in-process variant).

Mechanical path swaps in asap-query-engine:
- `asap_sketchlib::asap::dd_sketch::*`           → `::sketches::ddsketch::*`
- `asap_sketchlib::asap::count_min::*`           → `::sketches::countmin::*`
- `asap_sketchlib::asap::count_sketch::*`        → `::sketches::count::*`
- `asap_sketchlib::asap::hll_sketch::*`          → `::sketches::hll::*`
- `asap_sketchlib::asap::kll::*`                 → `::sketches::kll::*`
- `asap_sketchlib::asap::count_min_with_heap::*` → `::sketches::cms_heap::*`
- `asap_sketchlib::asap::hydra_kll::*`           → `::sketches::hydra_kll::*`
- `asap_sketchlib::asap::set_aggregator::*`      → `::sketches::set_aggregator::*`
- `asap_sketchlib::asap::delta_set_aggregator::*`→ `::sketches::delta_set_aggregator::*`
- `asap_sketchlib::asap::config::*`              → `::asap_runtime::*`

Naming-conflict renames carried through to the consumers:
- `HllDelta` → `HllSketchDelta` (octo_delta::HllDelta still wins the short name)
- `HeapItem` → `CmsHeapItem`   (common::input::HeapItem still wins the short name)

main.rs aliases `asap_sketchlib::asap_runtime as config` so the existing
clap derive references (`config::DEFAULT_CMS_IMPL`, `config::configure(...)`)
still work without touching the rest of the bin.

Tests:
- `cargo build --workspace`                                → clean
- `cargo test -p query_engine_rust --lib precompute_operators` → 141 passed, 0 failed

Depends on ProjectASAP/asap_sketchlib#36 (force-pushed `e473ccc`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: align CountSketchDelta consumer with sketchlib-go wire format

Track the additive `hh_keys` field on `asap_sketchlib::CountSketchDelta`
so the proto delta path constructs the type with all fields filled in.

Sends an empty `hh_keys` for now: the vendored Rust proto bindings in
`asap_otel_proto::sketchlib::v1` haven't been regenerated against the
latest `.proto` (which carries `hh_keys` on the Go side). The TopK
rebuild on the proto-delta path will fire once those bindings sync;
the sketchlib-go-aligned semantics are already in place underneath.

Bumps the asap_sketchlib git dep to `refactor/wire-format-align-go`
(see asap_sketchlib PR #37).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant