-
[OPIK-7611] [BE] Cut the fixed cost of the optimization list query (#7672)
frostbyte_neo 发布于
2026-07-31 16:00:54 +00:00 - [OPIK-7611] [BE] Cut the fixed cost of the optimization list query
OptimizationDAO.FIND is the most expensive query shape in the ClickHouse workload,
and its cost is fixed per call rather than proportional to data: rows read varies from
0 to 770k with no change in CPU, and the largest-data bucket is the cheapest. Because
ClickHouse inlines CTEs, each branch materialises independently and all stay alive in
the final six-way join, so the pipeline costs ~2.2 CPU-s and ~3.9 GiB whether or not
there is anything to aggregate.Consolidate best_candidate and baseline_candidate into a single candidate_rollup over
candidate_metrics. Both grouped by the same key over the same rows, so the second pass
was pure duplication: measured 26-34% less CPU and 22-31% less memory.Give best_duration and best_cost a deterministic tie-break. argMax over weighted_score
picked arbitrarily among tied candidates, and 93.6% of optimizations have a tied max
score with differing durations - the unchanged query returns different values for
451 of 1000 optimizations on nothing more than a change of max_threads. Tie-break on
earliest_created_at, which no optimization ties on and which matches how baseline
already orders. baseline is untouched. This changes these two user-visible values for
optimizations whose scores tie, replacing arbitrary results with stable ones.Serve optimizations with no experiments from a projection that skips the aggregate
pipeline. Every aggregate derives from experiments_final, so with no experiments they
collapse to empty-input values. A pre-check bounded at 13 CPU-ms across every real
production scope decides, and it omits the narrowing filters so a negative answer is
conservative. Note total_optimization_cost is a non-nullable zero, because sum() over
an empty group returns 0 rather than NULL.The projection keeps all 26 columns with identical names, types and order, verified by
diffing DESCRIBE against the unchanged template.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Fix the experiment probe binding and address review
The probe reused bindQueryParams, which binds name, filters and dataset_deleted.
Its query declares none of those placeholders, and binding a parameter the rendered
query does not contain fails the statement - so every optimization search carrying
one of those criteria failed. That is what broke Backend Integration Groups 1, 9 and
16 and the Python SDK E2E suites.Split the two binders along the boundary the probe needs: bindScopeTemplateParams and
bindScopeQueryParams cover the criteria that select optimizations by identity, and the
full binders now delegate to them, so the probe and the main queries share one
implementation instead of duplicating it. Verified mechanically that every bound
parameter appears in the rendered template and vice versa.Also from review: add the required log_comment so the probe is attributable in
query_log, rename to hasExperimentsForDirectFilters to stop the name implying the
narrowing criteria are applied, and skip the probe when the count is zero. The
short-circuit is safe because the full path builds (page, 0, total, [], []) which at
total = 0 is exactly OptimizationPage.empty(page, List.of()).Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Bind entity_type only for the template that declares it
Same defect as the probe, second instance: FIND declares :entity_type in its
feedback-score CTEs, the fast path omits those CTEs, and bindQueryParams was called
with isFindQuery = true regardless of which template was chosen. Binding a parameter
the rendered query does not contain fails the statement, so every search that fell to
the fast path failed - which is why Integration Group 16's studio tests, whose
optimizations have no experiments, kept failing.The flag already existed for exactly this purpose; drive it from the template choice.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Cover the score tie-break and attribute the fast-path query
Add an integration case where two candidates tie on the objective score. Under a tie
the best candidate is the earliest one, which is also what the baseline resolves to,
so best and baseline cost and duration must coincide. The two candidates are given
sharply different costs and trace durations, so picking the later candidate would
break that equality rather than passing silently. The previous implementation had no
defined tie-break and returned different values for identical data depending only on
the query plan, so nothing pinned this down before.Also thread log_comment through the fast-path query, which was executing unattributed,
and render it inside the workspace context so the comment can carry the workspace.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Use the class's standard assertion form
The new cases mixed .as() descriptions and usingComparator into the BigDecimal
assertions. The surrounding tests assert isNotNull first, then compare through
StatsUtils.bigDecimalComparator and expect isZero, and carry their reasoning in
comments. Match that so the new cases read like their neighbours.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Assert the tie-break against concrete values, and share the comparison
The tie-break case compared best cost and duration against baseline, which still passes
if both rollups pick the later candidate - the very regression it was meant to catch.
Baseline determinism was being used as a fixed reference while it is itself part of what
is under test. The fixture creates one trace and one span per dataset item, so per-trace
cost reduces to exactly the span cost; assert the earliest candidate's concrete cost and
that it is not the later candidate's.Extract the repeated isNotNull-then-bigDecimalComparator pair into
StatsUtils.assertBigDecimalEquals, next to the comparator it wraps, and use it at the
seven call sites across the two optimization test classes.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Correct two test assertions written against assumed values
The fast-path case asserted feedbackScores was empty, but getScoresAggregation maps an
empty score map to null rather than an empty list, so the assertion failed on a value
the code has always returned. Assert null.The tie-break case asserted the earliest candidate's concrete per-trace cost, which
depends on how that aggregate is derived from the fixture rather than on the tie-break
itself, and the surrounding test only ever asserts these costs are present. Require
instead that neither rollup reports the later candidate's cost and that the two agree.
That still catches either rollup picking the later candidate - the regression the
comparison to baseline alone would have missed - without resting on the derivation.Document on the fast-path query that the check runs as its own statement, so an
experiment inserted between the two reads leaves the aggregates empty for one response;
reads here are already eventually consistent through replica lag.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
- [OPIK-7611] [BE] Pick candidate costs the comparator can actually tell apart
The tie-break case used costs of 0.01 and 0.99 to identify which candidate the rollups
picked. bigDecimalComparator ends by comparing only toBigInteger(), deliberately, since
the quantile figures it also serves are not accurate to the fraction - so both values
truncate to 0 and compare equal, and the assertion that best must not report the later
candidate's cost could never fail. That is what timed out, not the tie-break.Use 1 and 9 so the two candidates differ in their integer parts, and record why in a
comment so the values are not "simplified" back later.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Co-authored-by: Claude Opus 5 (1M context) noreply@anthropic.com
下载附件