Skip to content

asap engine: ExactAgg(Sum) dispatch collapses sum/sum_over_time/increase/rate to same code path (function semantics lost) #301

Description

@zzylol

Symptom

The asap engine returns the same value for four semantically-different PromQL queries on a Counter metric, demonstrating that the function-name semantics (sum vs sum_over_time vs increase) are lost at capability matching:

Query (against http_requests_total) asap baseline (VictoriaMetrics raw) Baseline meaning
sum by (zone) (http_requests_total) (instant) 60.1M 92.7M Σ of current cumulative-counter values per series, per zone
sum by (zone) (increase(...[5m])) 60.1M 93.0M Σ of (last - first) per series in 5m window
sum by (zone) (sum_over_time(...[5m])) 60.1M 4,519M Σ of all sample values in 5m window
sum(http_requests_total) (no by) 60.1M 93.1M as above, no grouping
sum by (zone) (rate(...[5m])) 363K 999K events/sec over last 5min
topk(5, sum by (zone) (rate(...))) 363K 999K same
count(http_requests_total) 10,000 10,000 ✅ EXACT — series cardinality

The 60.1M for sum/sum_over_time/increase/instant-sum confirms they all hit the same engine path. The 363K for rate = 60.1M / range_seconds, confirming rate uses the range divisor but the numerator is the same all-of-storage delta-sum.

Root cause

Post #298 / #299 the agent now sends per-window deltas (not cumulative counter values), so the cubic blowup is gone. But evaluate_exact_agg in data_plane/src/storage_engines/sketch_db/query/sketch_reducer.rs:592 dispatches ALL Capability::ExactAgg(Sum) queries through the same code path:

  1. Read all per-window deltas across [t0_ms, t1_ms] for the matching sids
  2. Sum them per group
  3. Return one value per group

This is fine for sum_over_time(delta_metric[r]) IF [t0,t1] = [t-r, t] (PromQL-correct range scoping). But:

  • For instant sum(counter) PromQL spec says: return the CURRENT counter value per series, summed. The reducer should look at the latest window's value, treating earlier windows as accumulated counter state. Currently returns Σ-of-deltas-in-storage-range.
  • For sum_over_time(counter[r]): PromQL spec says: Σ of all SAMPLE values in [r] window. For a cumulative counter that's roughly Σ(0,1,2,...,N) ≈ N²/2 — a quadratic. Asap returns just N (sum of deltas = total events). Baseline returns ~50× more (4.5B vs 60.1M ratio here).
  • For increase(counter[r]): should return (last - first) in [r]. For a delta-metric stored series, that's just Σ-of-deltas-in-r. Currently OK by coincidence, but the code doesn't know it.
  • For rate(counter[r]): events/sec in [r]. = Σ-of-deltas-in-r / r. Asap gets this CLOSE because it does divide by range_seconds (via evaluate_exact_agg_rate path post-feat(query): compose rate-over-Sum into ExactAgg(Sum) candidate (multinode topk dispatch) #292), but the numerator is over the full storage range, not just [r].

Fix shape

The analyzer needs to type-track the inner counter-function semantic distinctly. Today Capability::ExactAgg(Sum) is too coarse. Suggested taxonomy:

// Replace or augment Capability::ExactAgg(Sum) with:
pub enum CounterOp {
    InstantSum,           // sum(counter) — return current
    SumOverTime(Duration), // sum_over_time(counter[r]) — Σ samples in r
    Increase(Duration),    // increase(counter[r]) — (last - first) in r
    Rate(Duration),        // rate(counter[r]) — increase / r
}

Then the reducer in sketch_reducer.rs branches on CounterOp:

  • InstantSum: read the LATEST window's accumulated delta from the start of storage (cumulative-up-to-now). For a fresh stack this is "total events since stack start". Comparable to baseline's current-counter-value.
  • SumOverTime(r): requires re-deriving cumulative-at-each-scrape from stored deltas, then summing. Quadratic on storage horizon — likely not worth supporting; CapabilityMiss → archive.
  • Increase(r): clip to [t-r, t], return Σ-of-deltas-in-clip.
  • Rate(r): same as Increase but divide by r.

Or: limit the reducer's range to [t-r, t] correctly for all three, which already gets Increase and Rate right; only InstantSum and SumOverTime need additional handling.

Scope

This is a follow-up to #298 — the fix in #299 unblocked the cubic blowup; this is the next layer (function-specific semantics on per-window-delta storage). Tagged enhancement — affects accuracy of counter queries against asap tier but doesn't break the system.

Verification

Once fixed, the demo's accuracy table should show:

  • sum by (zone) (http_requests_total) close to baseline 93M (currently 60M)
  • sum by (zone) (rate(http_requests_total[5m])) close to baseline 1M/s (currently 363K/s)
  • topk(5, sum by (zone) (rate(...))) same as rate (no change in semantic, derived from rate)
  • sum by (zone) (sum_over_time(...[5m])) close to baseline 4.5B (currently 60M) — OR CapabilityMiss → archive if too expensive
  • sum by (zone) (increase(...[5m])) close to baseline 93M (currently 60M)

Reference

  • Capture dir with these measurements: /tmp/resource-capture-v2-193938/
  • Earlier issue documenting Sum cubic blowup: #298 (fixed in #299)
  • Related: #300 was filed for sum_over_time specifically; this issue subsumes it with the full diagnosis.

Activity

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

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions