-
[OPIK-7636] [BE] perf: dedup the feedback-scores trace_final CTE with argMax instead of FINAL (#7689)
frostbyte_neo 发布于
2026-08-03 10:25:56 +00:00 - [OPIK-7636] [BE] perf: dedup trace-stats trace_final with argMax, gated on searchText
Replace FINAL with GROUP BY + argMax in the trace_final CTE of both
trace-stats templates. traces is a ReplacingMergeTree(last_updated_at)
ordered by (workspace_id, project_id, id), so grouping on that key and
taking each value-based predicate's verdict from the greatest
last_updated_at is exactly what FINAL + predicate computes.The rewrite is exact but not unconditionally cheaper: it trades a
streaming scan that filters row by row for a hash table holding one group
per trace in the key range, built in full before HAVING can discard
anything. It therefore renders only when search_text is present, on top
of the existing join-bearing scope guard. Measured in production, with
searchText it is ~2.8x cheaper in CPU and ~3.4x in peak memory on the
feedback-scores template, which re-evaluates this CTE from three scopes;
without it the aggregation state is pure overhead, up to ~6.3x more CPU
and ~85x more peak memory on a 26M-trace project.Non-key predicates move into HAVING argMax(...) so they are evaluated on
the latest version; leaving them row-level in WHERE would strand a group
on the latest surviving version. Per-version columns are aliased latest_*
and renamed by an enclosing SELECT, since a same-named alias nests argMax
inside argMax and ClickHouse rejects it with ILLEGAL_AGGREGATION.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] fix: pick the latest trace version atomically with a tuple-valued argMax
Per-column argMax let row versions sharing the greatest last_updated_at
contribute different columns to the same output row, synthesising a row
that never existed — where FINAL always yields one whole physical row.
Wrapping the projection in a single argMax(tuple(...), last_updated_at)
and unpacking it in the enclosing SELECT picks one version atomically, so
a timestamp tie degrades to "which of the tied versions", exactly as under
FINAL, rather than "a mixture of them".Verified against production: result-identical to the per-column form
(132,936 groups, same hash) and to FINAL on the full rendered query, at
lower peak memory (226 vs 353 MiB — one aggregate state instead of seven).The tuple alias also shadows no column, so it removes the latest_* renaming
that existed only to keep HAVING from resolving against an aggregate alias
and failing with ILLEGAL_AGGREGATION.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] docs: correct the tuple-argMax memory claim after re-measuring
The tuple form's memory advantage does not generalise. Re-measured on
production across the shapes the gate is built on: 226 vs 353 MiB over 133K
groups and 3.56 vs 4.11 GiB on the no-filter shape, but identical 9.43 GiB
over 26.4M groups, where the aggregate state is dominated by the grouped
values rather than per-aggregate overhead.The gate rationale is unchanged and re-verified against the tuple form: the
no-filter shape is still +51% memory and +24% CPU versus FINAL, and the
26.4M-group cheap-filter shape is still ~85x memory and ~6x CPU.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] docs: record why no argMax tie-breaker is added
ReplacingMergeTree breaks an equal-ver tie by "most recently inserted row
wins", and insertion order is not a column, so no value-based tie-breaker
can reproduce it — adding one would deliberately pick a different row than
FINAL in exactly the tie case, turning an unobserved divergence into a
guaranteed one.Measured instead: over 20 tied versions inserted in scrambled order across
multiple parts, and in the adversarial case where the last-inserted row
sorts lowest by value, FINAL and argMax selected the same row on every run
at 8 threads. Also recorded why ties are near-unreachable: rows sharing a
sort key within one INSERT collapse at part write, separate batches take
distinct now64(6) values, and no duplicate row versions exist across the
four largest production projects.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] docs: scope the argMax equivalence claim to unique latest versions
Stop claiming exact equivalence with FINAL across the board. For any key
whose latest last_updated_at is unique, argMax returns exactly the FINAL
row. For a key with tied latest versions it returns one of the tied rows,
and which one is not contractually the same row FINAL would pick, because
ClickHouse does not specify argMax's tie behaviour. That is an accepted,
bounded difference rather than exact equivalence.Both forms always return a whole row that was really stored, and the
difference can only surface on keys carrying byte-identical
last_updated_at. The measured agreement across tie scenarios is recorded
as observed behaviour, explicitly not as a guarantee.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] docs: condense the canDedupByArgMax javadoc
The javadoc had grown into a benchmark log. Trimmed from 80 to 53 lines,
keeping the rules a maintainer needs — the two gate conditions, predicate
placement, why the projection is one tuple-valued argMax, the scope of the
equivalence, and the is_deleted cutover prerequisite — and dropping every
measurement figure and verification narrative. Measurements live in
OPIK-7636, which the javadoc now points at instead of restating.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] refactor: address review — named tuple, rendered-SQL tests, helper cleanup
Named tuple instead of positional unpacking. Inserting a column into the
middle of the 7-element tuple shifts every later position and positional
access absorbs it silently; a wrong name is a parse error. Verified on
26.3.16.16: reordering the members made latest.1 return duration where
thread_id was expected, while latest.thread_id stayed correct. Result-
identical to the positional form on prod traces (140,317 groups, same hash),
and the ::Tuple(...) only attaches names to a type the expression already
has.Rendered-SQL tests. The guard suite only asserted the boolean; nothing
covered the SQL. Adds 6 cases: the traces/spans template drops FINAL, emits
GROUP BY/HAVING, unpacks by name, and renders balanced parentheses — the
wrapper's "FROM (" and its ")" come from separate template blocks — plus a
case pinning the deliberate asymmetry that the feedback-scores template
groups with no wrapper, so adding a projected column there fails loudly.Javadoc: record that filters is not vetoed even though TraceField.GUARDRAILS
renders the joined gagg alias into it, because FilterUtils always sets
guardrails_filters alongside and that is what vetoes — the two must stay in
sync. Also record that the <if(filters || search_text)> wrapper is
deliberately defensive rather than reachable.Tests: collapse three overloaded traceCount helpers into one and group both
helpers ahead of the tests, removing the overload-resolution footgun between
a search string and a single-element filter list.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] test: drop the rendered-SQL assertions, keep coverage black-box
Reverts the SQL-string tests and the @VisibleForTesting widening of the two
stats templates. Asserting the rendered query text couples the suite to how
the SQL is written rather than what it returns; the integration tests already
cover the same failure modes by executing the query — unbalanced SQL from a
one-sided edit to the wrapper fails to parse, and a missing wrapper or a
mis-ordered projection shows up as wrong stats.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] fix: keep filters index-visible via an id semi-join
Moving value-based predicates into HAVING argMax(...) put them out of reach
of the table's skip indexes. Measured on a 2.5M-trace project, a thread_id
equality filter plus searchText: FINAL prunes to 1 of 10,169 granules and
reads 4.08 MiB / 108 ms CPU; the argMax form read all 10,169 granules,
51.54 GiB / 22,416 ms — 12,900x bytes and 207x CPU. The searchText gate did
not prevent it, because that query has searchText.Filters are now also injected as id IN (SELECT id FROM traces WHERE
). The subquery is index-visible, and since it selects ids where any
version matches it is a superset of the correct answer, which the HAVING
narrows to the latest version.A column-level veto was implemented first and reverted: selectivity depends
on the filter's value, not its column. source IN ('sdk','unknown') prunes
nothing (those two values are 98% of rows) while source = 'playground' prunes
to zero granules, so a column list is wrong in both directions.Regression data, all result-identical:
- prunable shape (thread_id + searchText): 4.11 MiB / 48 ms, versus FINAL's
4.08 MiB / 108 ms — regression eliminated - production shape (source + searchText), 3 runs each: 55.60 GiB / 4.98 GiB /
62,277 ms with the semi-join versus 55.32 GiB / 4.33 GiB / 59,993 ms
without, and 60.86 GiB / 4.04 GiB / 69,221 ms for FINAL — the semi-join
costs ~4% CPU and ~15% memory and keeps the win
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] refactor: narrow the argMax dedup to SELECT_FEEDBACK_SCORES_STATS
The two stats templates behave oppositely under the rewrite, and the reason
is the projection width. SELECT_FEEDBACK_SCORES_STATS projects only
id, project_id — both group keys — so its aggregate state is small and peak
memory improves. SELECT_TRACES_SPANS_STATS projects seven per-version columns,
so it holds a state per column per trace and memory regresses, worsening with
group count.Measured on production, all result-identical:
- feedback_scores + searchText, 132K groups: 10.33 GiB / 79,808 ms CPU with
FINAL versus 2.64 GiB / 25,250 ms — 3.91x memory, 3.16x CPU better - traces_spans + searchText, 2.51M groups: 4.04 GiB / 69,221 ms versus
4.98 GiB / 62,277 ms — 1.11x CPU better for +23% memory - traces_spans + searchText, 5.38M groups: 6.11 GiB / 168,234 ms versus
9.56 GiB / 138,057 ms — 1.22x CPU better for +56% memory
All 13 Code: 241 OOMs were on feedback_scores, where this takes peak memory
10.33 -> 2.64 GiB. traces_spans peaked at 34.7 GiB against a ~47 GiB ceiling,
so +56% there risks creating OOMs in a family that does not have them; its
~1.1-1.2x CPU is not worth that.SELECT_TRACES_SPANS_STATS is therefore reverted to byte-identical with
production, which also removes the wide-projection apparatus it needed: the
wrapper subquery, the named tuple, and the ILLEGAL_AGGREGATION aliasing
hazard. Only this template's gate call sites remain.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7636] [BE] refactor: collapse the guard tests and trim the javadoc
Review follow-ups.
The seven guard test methods were the same assertion parameterised by
(slots, expected), so they are now one @CsvSource case with 18 rows —
same coverage, one method. Kept as a unit test rather than moved black-box
because the gate selects a SQL shape and both shapes return the same rows by
design, so it is not observable from query results; the behaviour the shapes
must share is already covered by the integration tests.Javadoc trimmed from 73 to 48 lines: the two gates, the guardrails coupling,
predicate placement, the semi-join, the scoped equivalence and the is_deleted
prerequisite, with measurements left to OPIK-7636 rather than restated.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Co-authored-by: Claude Opus 5 (1M context) noreply@anthropic.com
下载附件