Repository navigation
fix: max, min, count and avg over a range function are one series per group on query_range - #734
Merged
Merged
Conversation
github-actions
Bot
added
size/XL
szibis
added a commit
that referenced
this pull request
Oct 12, 2026
…ion-unreduced, with its visual fix proof page
github-actions
Bot
added
size/XL
Contributor
⚪ Performance A/B: no change beyond noise131 shape×range — 131 ⚪ same · results vs Loki: 101 same, 0 new difference(s), 30 pre-existing Every shape is within noise of the base build (±30% and 100 ms). 131 unchanged shape×range (within noise)
Cold (first-run) timings — what a user opening a panel sees
↺ Loki cold taken from the first set that ran the same query; Loki's cache answered this one. Why these shapes
base |
Contributor
✅ Visual smoke: passed with warningsBase Fix proofCaptures of the exact cases this pull request adds or fixes (registry case, base | PR | Loki). The verdict compares the PR with Loki where the base did not match. The gate fails a case unless one capture is fixed or improved, or, beyond the Loki data window, the base failed where Loki and the PR both answered with data (the label does not excuse it; only
explore-max-by-count-over-time-sliding-15m: improved
explore-max-by-count-over-time-sliding-1h: improved
explore-max-by-count-over-time-sliding-6h: undecided
Regression check
Gate: a base-vs-PR data difference passes when the PR is closer to Loki, and fails when the PR diverges from Loki or Loki cannot decide it (at a range Loki does not hold, a difference passes when the same page proved it closer to Loki at a shorter range; otherwise label the pull request explore-max-by-count-over-time-sliding-6h: no Loki data at this range; the same differences match Loki at a shorter range (1): query ('max by (service_name) (count
Core set montages (5): base | PR | Lokidd-fields-1h
dd-landing-1h
dd-service-logs-1h
explore-logs-plain-1h
explore-metric-plain-1h
All montages of this run: |
Contributor
PR Quality ReportCompared against base branch Coverage and tests
Compatibility
Performance smokeLower CPU cost (
State
|
… group on query_range A window that is not the step is evaluated by the proxy's range evaluator from the first stats pipe of the translation alone, which holds the series identity and not the outer operator, so max(count_over_time(...[5m])) returned one series per stream where Loki returns one. The query's vector aggregations are now applied to the series the evaluator returns, per step, the way Loki's VectorAggEvaluator folds them (by keeps the listed labels, none keeps none; count is the number of series, avg their mean, min and max the extreme), and max_query_series is checked on that answer. A nested aggregation (sum(max(rate(...)))) also evaluated a count of lines where the query asked for a rate, because only the outermost operator was stripped to find the range function.
…to Loki's folded series
…'s vector aggregation
…ion-unreduced, with its visual fix proof page
…a nested without is answered as before count by (route) over | logfmt route on a sliding window folded the series into one without a label, because the window evaluator drops the labels a parser extracts unless the proxy folds a without. The fold of the vector aggregations now asks for them the same way. The nested aggregations are read down to the range function except under a without that is not the one the proxy folds (the outermost): that shape is answered as it was, so the without fold keeps its scope. The goldens and open-shape pins that recorded the per-stream series are updated to the folded answers.
…, record the sliding figures The changelog entry and the registry rows say what the fold covers: chains of sum, count, min, max and avg without a without, and a grouping on a parsed label. A without nested under another aggregation stays with semantics/without-nested-under-aggregation; the query_range rate case of semantics/without-over-inner-aggregation-collapses is fixed, and the sliding figure of semantics/unwrap-extraction-list-series-identity is recorded.
…ports after the rebase onto main
szibis
force-pushed
the
fix/non-sum-vector-aggregation-unreduced
branch
from
October 23, 2026 09:20
a47440d to
63e6190
Compare
github-actions
Bot
added
size/XL
szibis
marked this pull request as ready for review
October 23, 2026 09:21
…fferential leaves no goroutines behind The differential built one proxy per scenario (6,336) and closed only the stub backend. Every proxy kept two caches running and the scenario's own cache, two goroutines each: 38,016 goroutines stayed alive and the internal/proxy test binary peaked at 2.8 GB against 1.8 GB on main. With the proxy shut down and the three caches closed, the test alone peaks at 91 MB and the package at main's level.
github-actions
Bot
added
size/XL
szibis
added a commit
that referenced
this pull request
Oct 26, 2026
The README led with measurements from 2026-05/06 (a Drilldown bench with Loki at 12 GiB, proxy v1.50.0 throughput tables) that read as the current position, and docs/compatibility-coverage.md still showed 15/35 implemented endpoints (current registry: 23/35). - README: new "Quality and performance vs Loki (current)" section with the registry scores (23/35 routed, 19/35 compared against Loki, 12/35 proven), the ranked-difference counts (88 rows: 69 open, 12 documented, 5 owner-kept, 2 upstream), the cold first-request count (54 pairs slower than Loki), the repeated and refreshed window numbers of #862, the load test, and the shapes that are still slower or fail where Loki answers. The 2026-06-05 one-glance comparison and the v1.50.0 throughput section are labelled historical with their conditions. - docs/performance.md: "Current position versus Loki" with the sources and method for each number; the 2026-06-05 metadata table is dated. - docs/benchmarks.md, docs/honest-tldr.md: historical banners with the conditions; methodology and reproduction steps are kept. - docs/KNOWN_ISSUES.md: numeric tenant routing (#846), kept stats buckets (#862), a "where the proxy is slower" paragraph, and the max/min/count/avg fold (#734) and delete 404 (#855) under the closed gaps. - docs/real-window-compatibility-gaps.md: marked as a historical record and linked to the current summary. - docs/compatibility-coverage.md: regenerated by conformance/scripts/coverage_map.py.
szibis
added a commit
that referenced
this pull request
Oct 27, 2026
… the timeout perf-smoke on #746 was cancelled at its 30-minute limit. Of the 1658 s the shapes ran, W8 (sum without (pod) (count_over_time({env="production"}[1m]))) took 1019 s: Loki timed out at 90 s on all 4 runs of the first pass and on all 7 runs of the confirm pass the PR's speedup on W8 (0.58 s against 0.80 s) started. Loki timed out on W8 in the perf-smoke runs of #739 and #734 as well (they had no confirm pass on W8). Under a `without`, Loki keeps the structured metadata of the OTel-instrumented streams (trace_id, span_id) in the series, one series per line; in the daily run its 1h answer filled 80.9 MB of results cache against the 41 series the proxy answers. W8 now uses the selector the nested-aggregation set uses ({env="production", instrumentation!="otel", app!="otel-collector"}), as W1-W4 moved to `| logfmt amount` for the same reason. It still measures the stream-label `sum without` fold. The other 172 selected shapes ran at about 5 s each, so the selection of 20 sets should finish in about 18 to 20 minutes with the stack and seed.
szibis
added a commit
that referenced
this pull request
Oct 27, 2026
…ant queries and tumbling windows, like Loki (#746) * fix: a vector aggregation over another one keeps every level's own by Loki folds the innermost level first and each level keeps exactly its own grouping labels (pkg/logql/evaluator.go VectorAggEvaluator.Next). The native routes (an instant query, a tumbling window) dropped the outer by of a nested aggregation and answered one series without labels. Each level is now a stats pipe with its own by; a level above the first keeps only labels the previous one kept, and an innermost without is folded over the series rows level by level in the proxy, under the without marker main's fold uses (the outermost level's operator, series rows, the limit on the folded answer). On a sliding window the proxy evaluates the first stats pipe, the series of the innermost level, and folds every level over them: the fold of the vector aggregations also takes a nest of sums and a static without level, the evaluator names the series by the innermost level's labels (they took the outermost level's by position), and a nest spelled sum(...) by (l) reads its range function from the text as written. The shapes the open without rows pinned (max(sum without ...), sum by (ns) (sum without ...), sum without (sum by ...)) now answer Loki's series and values; their pinned answers are updated, and the two rows are fixed. * test: nested vector aggregations against Loki on an instant query and a tumbling window * docs(registry): state the VictoriaLogs versions the nested aggregation chain was checked on * chore(conformance): visual fix proof page for nested-aggregation-outer-by-native-routes * chore(conformance): record #746 as the fix of nested-aggregation-outer-by-native-routes * test(bench): A/B set for nested vector aggregations Adds the nested-aggregation set to bench/ab/shapes.json: 15 shapes of a vector aggregation over another one, on the generator's data. N1-N8 are range queries over tumbling windows (range == step via $__auto): two-level nests of sum, max, min, avg and count, a three-level chain, a `| json` inner level with an explicit extraction, a `| logfmt` inner level, and a `without` over a level that names its labels. N9-N13 are the same forms as instant queries, N13 with an innermost `without`. N14-N15 are sliding windows (range 5m over the step), one of them a nest of sums. An innermost `without` over a tumbling window has no range shape: its series rows are folded in the proxy exactly as the single-level `avg without (pod)` already is on main (about 1 s at 6h and a 10 s step on both builds), so a shape there would only repeat that cost against a base that answered the wrong thing. The selector leaves out the OTel-instrumented streams and the otel-collector: Loki carries their structured metadata (trace_id, span_id) in the series, so an inner level grouped by pod counts per line there, a series identity difference that predates the nest fix. Checked on a 90 minute seed against Loki first (all 200, non-empty): origin/main differs from Loki on N1-N13 and N15 (one series, or the wrong series count); this branch is the same as Loki on all 15. The registry item's `area` now names the source files with their paths (internal/translator/nested_vector_chain.go, internal/proxy/nested_vector_fold.go, internal/proxy/vector_agg_fold.go), so selection.py ties a change to them to the set; generated performance evidence regenerated. * perf: VictoriaLogs folds a tumbling window's `without`, and the proxy's fold decodes once A nest of vector aggregations with an innermost `without` kept a row per stream and bucket and folded every level in the proxy: `avg without (pod) (count_over_time({env="production"}[10s]))` over 6h at a 10 s step took 0.8 to 1.2 s on main, and the same `without` under `min by (app)` 1.44 s on this branch. - A `<op> without (...)` over a tumbling window (range equal to the step), alone or as the innermost level of a nest, is cut in VictoriaLogs when the excluded labels are plain names (letters and digits; not level, detected_level, service_name or a label the service name is derived from; no custom field mapping renames them): the labels of a series are the pairs of its `_stream` text, so the pairs are cut out of it between the stats pipe that counts a series and the one that combines the series now sharing a stream, and the levels above read their labels as columns of the pipes (translator.PushDownWithout). A parser, a sliding window, an instant query and every other name keep the proxy's fold. Measured on a seeded stack, 6h, 10 s step: the single-level form 0.8 to 1.2 s on main, 25 to 70 ms warm and 175 ms cold here; the nest 14 ms against Loki's 45 ms (178 ms cold, Loki 1.26 s). - The proxy's fold of series rows (a sliding window, an instant query, a parser, the names above) decodes the answer once, folds every level over the series and encodes it once (typed_fold.go), where it parsed and wrote the JSON again at every level: 47 ms and 89k allocations against 164 ms and 3.1M on 200 series of 2160 points. The sliding `sum by (app) (max by (app, pod) (count_over_time(...[5m])))` is 28% (6h) and 66% (1h) faster. - A query with one aggregation no longer parses for a nest: the benchmarks of the control queries show the allocations of main (translate: 38 -> 43 per query before, 38 now; handler: +17 to +34 before, identical now). Differential: TestWithoutPushdownMatchesFoldAndLoki (320 random label sets, a label on some streams only, excluded names the cut takes and does not take, values with quotes, commas, backslashes and pair-like text) holds both routes to a reference of Loki's evaluator and to each other; TestWithoutPushdownDirectedStreams covers streams that differ only by the excluded label, first, in the middle and last of the pairs, for every operator and both functions; TestTypedFoldMatchesGenericFold holds the typed fold to the generic one. TestCompat_NestedVectorAggregation gains single-level `without` forms; it and the controls pass on VictoriaLogs v1.49.0, v1.50.0, v1.51.1, v1.52.0 and v1.53.0 (979 subtests on the floor and the newest, 391 on the others). TestWithoutFoldRowBudgetIsNotASeriesLimit no longer expects the row-limit error for a tumbling `sum without` of count_over_time: VictoriaLogs folds it and the two series it answers fit, as Loki answers it. * perf: the typed fold reads the answer with a scanner, not a document tree Under 32 concurrent clients the proxy's peak RSS with the typed fold was 3.53, 3.78, 3.93 and 3.73 GB in four runs against 3.17, 2.88, 3.16 and 3.20 GB for main (same stack, same queries, ABBA). The heap profile taken at each of those peaks held 1.5 GB in fastjson.(*cache).getValue under decodeTypedBody: a document tree of a sliding nest's answer (millions of samples, one allocated value per number, string and array), alive while the fold ran, on top of the evaluator's own matrices that main also holds. decodeTypedBody now scans the bytes into the series directly (16 bytes a sample) and falls back to the generic fold for anything it does not read. On 200 series of 2160 points: 36 ms, 28 MB and 7.3k allocations against 47 ms, 67 MB and 89k for the tree and 143 ms, 99 MB and 3.1M for the generic fold. Re-run, same method, eight slots ABBA: peak RSS 2.87, 3.06, 2.93 and 2.81 GB for the branch against 2.93, 2.88, 3.32 and 2.91 GB for main, and 2672 requests against 2501 in 150 s per slot. TestTypedScannerReadsWhatTheDocumentTreeReads holds the scanner to encoding/json's tree over 800 generated bodies (indented or compact, extra keys and nested stats, escapes in labels, string and number values, null and short samples, result before resultType). * fix: the typed fold reads sample timestamps without package unsafe gosec v2.29.0 (Security / static) flagged G103 at typed_fold.go:220, where the scanner handed strconv.ParseFloat an unsafe.String over the body bytes to avoid a string conversion per number. main has no use of unsafe and gosec is clean on it. The scanner now reads a whole-second timestamp (up to 15 digits, which a float64 holds exactly) straight from the bytes, and passes anything else to strconv.ParseFloat through a plain string conversion that does not escape. BenchmarkFoldWithoutMatrix/typed, GOMAXPROCS=1, 20 iterations x3: 35.4-36.9 ms with unsafe.String, 42.6-43.6 ms with only the string conversion, 29.6-33.5 ms with the digit fast path; 28.0 MB and 7310 allocations in all three. TestScannerNumberMatchesParseFloat holds the scanner to strconv.ParseFloat, bit for bit, on integers at and past the 15-digit edge, fractions, signs, exponents and malformed numbers. * test(bench): the nest fix proof pages measure the nests, not OTel series identity explore-nested-aggregation-outer-by. On visual-smoke for 0f0ece6 the PR answered the 15 service_name series Loki answers (1365 and 5415 non-zero points on both at 15m and 1h; main answered one series {}), and the first differing series was checkout-svc: 2 against Loki's 1 at 15m, 5 against 1 at 1h. checkout-svc is one of the generator's OTel-instrumented services, whose lines carry trace_id and span_id as structured metadata. Loki puts structured metadata into a sample's labels (pkg/logql/log/metrics_extraction.go:104,109), so count_over_time there is one series per line, each counting 1, and the avg by (service_name, level) over them is 1; VictoriaLogs counts the stream. That is a series identity difference the single-level query has on main too, and the bench/ab nested-aggregation set leaves those streams out for the same reason. The page now uses the set's selector: {env="production", instrumentation!="otel", app!="otel-collector"}. main still collapses the nest on it (its translation ends in an ungrouped `stats min(...)` with either selector) and the PR keeps `stats by (service_name)`. explore-without-nested-under-aggregation. `sum without (service_name)` merges no series on the generator's data (every stream's app equals its service_name), so main's answer, the max over the unmerged streams, was already Loki's: base and PR were identical. The page now asks max(sum without (pod) (count_over_time({app="payment-service"}[5m]))): pods change every generator batch (10 s apart), so a 5m window sums several pods per level, and main reads the max over single pods. Through the handlers on emulated VictoriaLogs, main answers 5 where Loki's evaluation of the same rows is 20 (instant) and the PR 20; on query_range main answers two series {ns=x},{ns=y} where Loki answers one {} = 8,16,20. A tumbling $__auto window would not show it at 15m and 1h: their steps hold one batch each. The payment-service streams carry no structured metadata, which also leaves out the per-line OTel series that kept Loki from settling the old query at 1h. * test(bench): W8 leaves out the OTel streams so Loki answers it within the timeout perf-smoke on #746 was cancelled at its 30-minute limit. Of the 1658 s the shapes ran, W8 (sum without (pod) (count_over_time({env="production"}[1m]))) took 1019 s: Loki timed out at 90 s on all 4 runs of the first pass and on all 7 runs of the confirm pass the PR's speedup on W8 (0.58 s against 0.80 s) started. Loki timed out on W8 in the perf-smoke runs of #739 and #734 as well (they had no confirm pass on W8). Under a `without`, Loki keeps the structured metadata of the OTel-instrumented streams (trace_id, span_id) in the series, one series per line; in the daily run its 1h answer filled 80.9 MB of results cache against the 41 series the proxy answers. W8 now uses the selector the nested-aggregation set uses ({env="production", instrumentation!="otel", app!="otel-collector"}), as W1-W4 moved to `| logfmt amount` for the same reason. It still measures the stream-label `sum without` fold. The other 172 selected shapes ran at about 5 s each, so the selection of 20 sets should finish in about 18 to 20 minutes with the stack and seed. * test: a max over a sum without on a sliding count matches Loki The visual fix proof page of semantics/without-nested-under-aggregation now asks max(sum without (pod) (count_over_time({app="payment-service"}[5m]))). TestWithoutNestedAggregationMatchesLoki held max over a without only for the unwrap rate; it now also holds this shape, range and instant, to Loki's evaluation of the emulator's rows (one series {} = 8,16,20; instant 20). On origin/main the same request answers {ns=x}=2,3,5 {ns=y}=2,4,5 and, at an instant, 5: the max over single pods. --------- Co-authored-by: szibis <szibis@users.noreply.github.com>
szibis
added a commit
that referenced
this pull request
Oct 27, 2026
The README led with measurements from 2026-05/06 (a Drilldown bench with Loki at 12 GiB, proxy v1.50.0 throughput tables) that read as the current position, and docs/compatibility-coverage.md still showed 15/35 implemented endpoints (current registry: 23/35). - README: new "Quality and performance vs Loki (current)" section with the registry scores (23/35 routed, 19/35 compared against Loki, 12/35 proven), the ranked-difference counts (88 rows: 69 open, 12 documented, 5 owner-kept, 2 upstream), the cold first-request count (54 pairs slower than Loki), the repeated and refreshed window numbers of #862, the load test, and the shapes that are still slower or fail where Loki answers. The 2026-06-05 one-glance comparison and the v1.50.0 throughput section are labelled historical with their conditions. - docs/performance.md: "Current position versus Loki" with the sources and method for each number; the 2026-06-05 metadata table is dated. - docs/benchmarks.md, docs/honest-tldr.md: historical banners with the conditions; methodology and reproduction steps are kept. - docs/KNOWN_ISSUES.md: numeric tenant routing (#846), kept stats buckets (#862), a "where the proxy is slower" paragraph, and the max/min/count/avg fold (#734) and delete 404 (#855) under the closed gaps. - docs/real-window-compatibility-gaps.md: marked as a historical record and linked to the current summary. - docs/compatibility-coverage.md: regenerated by conformance/scripts/coverage_map.py.
szibis
added a commit
that referenced
this pull request
Oct 27, 2026
The README led with measurements from 2026-05/06 (a Drilldown bench with Loki at 12 GiB, proxy v1.50.0 throughput tables) that read as the current position, and docs/compatibility-coverage.md still showed 15/35 implemented endpoints (current registry: 23/35). - README: new "Quality and performance vs Loki (current)" section with the registry scores (23/35 routed, 19/35 compared against Loki, 12/35 proven), the ranked-difference counts (88 rows: 69 open, 12 documented, 5 owner-kept, 2 upstream), the cold first-request count (54 pairs slower than Loki), the repeated and refreshed window numbers of #862, the load test, and the shapes that are still slower or fail where Loki answers. The 2026-06-05 one-glance comparison and the v1.50.0 throughput section are labelled historical with their conditions. - docs/performance.md: "Current position versus Loki" with the sources and method for each number; the 2026-06-05 metadata table is dated. - docs/benchmarks.md, docs/honest-tldr.md: historical banners with the conditions; methodology and reproduction steps are kept. - docs/KNOWN_ISSUES.md: numeric tenant routing (#846), kept stats buckets (#862), a "where the proxy is slower" paragraph, and the max/min/count/avg fold (#734) and delete 404 (#855) under the closed gaps. - docs/real-window-compatibility-gaps.md: marked as a historical record and linked to the current summary. - docs/compatibility-coverage.md: regenerated by conformance/scripts/coverage_map.py.
szibis
added a commit
that referenced
this pull request
Oct 27, 2026
…#872) * docs: state the current quality and performance position against Loki The README led with measurements from 2026-05/06 (a Drilldown bench with Loki at 12 GiB, proxy v1.50.0 throughput tables) that read as the current position, and docs/compatibility-coverage.md still showed 15/35 implemented endpoints (current registry: 23/35). - README: new "Quality and performance vs Loki (current)" section with the registry scores (23/35 routed, 19/35 compared against Loki, 12/35 proven), the ranked-difference counts (88 rows: 69 open, 12 documented, 5 owner-kept, 2 upstream), the cold first-request count (54 pairs slower than Loki), the repeated and refreshed window numbers of #862, the load test, and the shapes that are still slower or fail where Loki answers. The 2026-06-05 one-glance comparison and the v1.50.0 throughput section are labelled historical with their conditions. - docs/performance.md: "Current position versus Loki" with the sources and method for each number; the 2026-06-05 metadata table is dated. - docs/benchmarks.md, docs/honest-tldr.md: historical banners with the conditions; methodology and reproduction steps are kept. - docs/KNOWN_ISSUES.md: numeric tenant routing (#846), kept stats buckets (#862), a "where the proxy is slower" paragraph, and the max/min/count/avg fold (#734) and delete 404 (#855) under the closed gaps. - docs/real-window-compatibility-gaps.md: marked as a historical record and linked to the current summary. - docs/compatibility-coverage.md: regenerated by conformance/scripts/coverage_map.py. * docs: mark unverified Loki cold times and drop unsourced claims Review of the previous commit: the 7d unwrap rows come from the 2026-10-08 run, which recorded no Loki cache counters, so their Loki cold time is unverified; say so beside the number. Remove registry ids that point at fixed correctness cases from the slow-shape bullets, a microbenchmark clause with no source, and a sentence about Loki's failures that the page's own conclusion contradicts. Correct the #846 logging description and the true-cold counts (0 of 221 slower than the previous build), and date the cold run by its review, not its merge. * docs: the current-position sections name release 2.35.0, which ships #862 * feat: summarize the performance evidence versus Loki by workload perf_evidence.py now writes a Summary versus Loki section into the performance report, computed with the script's own rules over the latest saved run of each shape and range: cold first request faster, on par and slower counts over the comparable pairs (both sides 200, Loki's cache did not answer the cold run); warm and refresh counts for runs that gave each set its own windows (none of the saved runs did, and the report says so); results versus Loki; a table of medians by workload; and the metadata calls with the pairs Loki's cache answered counted apart. The rest of the report is unchanged. * docs: README states the proxy against Loki in numbers, without history The current section is now organised as better, on par and worse in numbers (counts and medians generated by the performance report's summary), then the read API quality counts. Dates, pull request numbers and before and after history stay in the performance guide. The latency table is replaced by the generated per-workload medians, the label browser table by the generated metadata calls, and the translation overhead is re-measured; the loki-bench throughput table is labelled historical. * docs: drop dates and the release name from the README position sections * docs: regenerate the published coverage map without the write endpoints * docs: refresh the position against Loki with the 2026-10-27 daily run Re-take every figure from the regenerated reports: first-request counts over 455 comparable pairs (213 faster, 164 on par, 78 slower), the workload and metadata tables, 87 ranked differences, and the 165 rows above 2x Loki (55 first request, 110 repeat). Add the bounded-staleness label lists and the nested aggregation fix, and state that rows above 2x Loki are defects. * docs: README scope line uses the owner's wording for write endpoints * docs: the README scope line says no other write endpoint is planned --------- Co-authored-by: szibis <szibis@users.noreply.github.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
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.
Summary
Gap
semantics/non-sum-vector-aggregation-unreduced. On query_range,max,min,countandavgover a range function returned one series per stream instead of one per group when the window is not the step.max(count_over_time({service_name="api-gateway"}[5m]))returned 2,160 series over an hour at step 60 s, where Loki 3.7.7 returns 1.max by (baz), a label no stream has, returned 10,656 against 1. Nested chains were also wrong:sum(max(rate(...)))counted lines instead of computing a rate.Cause.
parseStatsCompatSpec), so it returned per-stream series and dropped the outer operator.parseOriginalRangeMetricSpecstripped only the outermost aggregation.Fix.
VectorAggEvaluatordoes (pkg/logql/evaluator.go:400-529). This isfoldVectorAggregationsin the newinternal/proxy/vector_agg_fold.go, built onapplyWithoutGrouping/withoutMergeForOp. It:sum,count,min,maxandavgthat contain nowithout;max_query_serieson the folded answer, as Loki does.count by (route)over| logfmt routefolds on that label. The window evaluator drops parser-extracted labels unless asked for them, and the fold now asks for them.parseOriginalRangeMetricSpecnow walks nested aggregations down to the range function. It stops at a nestedwithoutthat isn't the outermost one, so those shapes answer as before (semantics/without-nested-under-aggregation).evaluator.go:525-535). This also fixes the existingwithoutpath.Rebase onto main (2f52014)
[Unreleased]. The released sections are identical to main.range_metric_compat.go: main addedwithQuantileShapeand movedwithoutUnwrapGateintoproxyManualRangeMetricInstant. Both are kept; the PR's change is still the swap fromfoldUngroupedSumtofoldVectorAggregationsat the range and instant call sites.extraction_list_outside_without_test.go: main's goldens are kept, Sent text included. This PR changes only the answers of three sliding goldens to the folded series:count by (ns)overcount_over_timewith| logfmt route;count by (ns)overratewith| logfmt route;count by (route)overcount_over_timewith| logfmt route.bench/visual/spec.json: main's new pages are kept, and this PR'sexplore-max-by-count-over-time-slidingfix-proof page is added after them.wiring.json,errors.json(line numbers),gaps.md,parity-gaps.mdandroadmap.md.Registry
Changed row
conformance/registry/cases/semantics/non-sum-vector-aggregation-unreduced.yaml:fixed, fixed by fix: max, min, count and avg over a range function are one series per group on query_range #734;Also updated:
semantics/without-over-inner-aggregation-collapses: the query_rangeratecase is now fixed.semantics/unwrap-extraction-list-series-identity: the sliding figure is recorded.parity-gaps.mdmoves the row from rank 2 (open) to the fixed list.Tests
TestVectorAggregationOverRangeFunctionDifferentialruns the real handler against a VictoriaLogs emulator and compares it with a Loki-semantics reference. It covers:count_over_timeandrate;by,by (), a missing label, andwithout;Before the fix, 290 range subtests failed.
Also new:
TestVectorAggregationGroupedByParsedLabelSlidingWindow(max|min|count|avg by (route)over| logfmt routeon a sliding window)TestVectorAggregationGroupedLabelsSlidingWindowTestVectorAggregationUnderBinaryScalarTestVectorAggregationOverRateCounterFoldsTestParseOriginalRangeMetricSpecNestedAggregationsTestFoldByGroupingEdgeCases, including NaNTestApplyWithoutGroupingNaNMinMaxChanged:
TestInstantUnwrapOtherAggregationsKeepPerStreamValuesasserted the old, non-Loki answer. It is nowTestInstantUnwrapOtherAggregationsFoldPerStreamValuesand asserts Loki's.e2e vs Loki:
TestCompat_NonSumVectorAggregationRange(test/e2e-compat, group semantics).Proofs on the head (6042bc3)
go build ./...andgo vet ./internal/proxy/ ./internal/translator/: ok.-race(VectorAgg|FoldByGrouping|ApplyWithoutGrouping|ParseOriginalRangeMetricSpec|InstantUnwrap|ExtractionList|WithoutOpenShape): 1,020 passed. After the leak fix below,VectorAgg|FoldByGrouping: 866 passed.GOMEMLIMIT=3GiB GOMAXPROCS=4 go test -short -count=1 -p 1 ./cmd/... ./internal/...): 13/13 packages ok.TestVectorAggregationOverRangeFunctionDifferentialbuilt a proxy for each of its 6,336 scenarios and closed only the stub backend. 38,016 cache goroutines stayed alive.internal/proxytest binary peaked at 1.93 and 1.90 GB on this PR, against 1.89 and 1.91 GB on main (two runs each, same machine).Shutdownleaves the two caches the proxy creates running. That is fixed separately in fix: shutdown closes the caches the proxy creates, with lifecycle and fuzz leak guards #857.python3 scripts/ci/check_conformance.py: conformance gate ok; ratchet coverage 54%, unchanged.python3 scripts/ci/check_changelog_pr.py --base origin/main --head HEAD: ok.e2e against Loki on the supported VictoriaLogs range
TestCompat_NonSumVectorAggregationRange(test/e2e-compat) ran on an isolatedbench/abstack. The stack used Loki 3.7.7 with the e2e configuration, and VictoriaLogs at each version (verified viavm_app_version). The test ran against main's binary and this PR's, both with the flags of the e2eloki-vl-proxyservice. It refuses to compare when Loki answers no sample.CI's
vl-pinnedjob covers v1.52.0.Data-to-data against Loki (differential run,
bench/parity)Setup.
parity. Loki 3.7.7 and VictoriaLogs v1.52.0 were seeded with the same 1.5 h of generator data: 286,227 lines, equal in every 10-minute slice before and after each run.Subset 1: every corpus request whose outer operator is
count,min,maxoravg. That is 556 queries, each sent with and withoutcategorize-labels, so 1,112 requests.{namespace="prod"}is 2,507 series on Loki and 514 on the proxy.countis lower, andmax/min/avgover merged series differ. That is the recorded deviationmetric-identity/structured-metadata-splits-series, plus the parsed-label identity rows.withoutlabel sets, parse-error texts, and a nested outerbyon the instant path.Subset 2: nested chains (
sum(max(...))and deeper), 14 requests.sum(max(rate({namespace="prod"}[5m])))andsum(max(bytes_over_time({namespace="prod"}[3m]))). Each now matches Loki on the endpoint that differed on main.label_formatparsed-label case;byon the instant path.Pending before ready
Each item below gets its result added here:
explore-max-by-count-over-time-sliding);Not changed here (separate rows)
stddevandstdvar(semantics/stddev-stdvar-grouped-values).quantile_over_timeunder count/min/max/avg: the translation groups rows before taking the quantile.group(...): Loki 3.7.7 has no such operator (handled in fix: reject group(), label_join() and other names Loki does not have, and accept keywords in any case, like Loki #748).byon an instant query or a tumbling window.detected_levellabel that native stats answers lack (handled in fix: a metric's series carry detected_level, unknown included, on every route, like Loki #750).withoutnested under another aggregation (semantics/without-nested-under-aggregation).