Repository navigation
fix(plan-selection): prove Count-Min weights over a gauge declared non-negative - #815
Merged
Merged
Conversation
zzylol
force-pushed
the
fix/cms-weights-declared-non-negative
branch
from
October 21, 2026 17:59
9f3cfe0 to
f363e51
Compare
This was referenced Oct 21, 2026
zzylol
force-pushed
the
fix/cms-weights-declared-non-negative
branch
from
October 21, 2026 19:49
f363e51 to
9d929ef
Compare
…tive are proven confirmed_weights answered an open SamplesOfMetric requirement from the declared metric type alone, so raw samples of a gauge declared values_non_negative stayed unproven and every Count-Min and CmsWithHeap candidate over them was rejected as count_min_weights_not_proven_non_negative. WeightDomain::confirmed now takes both facts: samples are proven by a declared counter (CounterSamples) or else a declared non-negative metric (the new DeclaredNonNegativeSamples). A counter derivative stays counter-only, since a reset-corrected rate of a gauge can be negative.
zzylol
force-pushed
the
fix/cms-weights-declared-non-negative
branch
from
October 21, 2026 20:00
9d929ef to
acda60b
Compare
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.
Why
The ASAPQuery workloads' top-k-sum queries (
q06-topk-sum,q07-topk-sum, e.g.topk by (region, service) (4, sum_over_time(data[1m]))) readdata, a gauge the converter declares non-negative (values_non_negative, #780). Every Count-Min and CmsWithHeap candidate over it was still rejected ascount_min_weights_not_proven_non_negative, "not declared a counter". The sign gate in the accuracy model already reads the declaration, but the weight check that guards the executor did not.What
WeightDomain::SamplesOfMetricrequirement is now proven when the metric is declared a counter or declared non-negative.NonNegativeWeightProof::DeclaredNonNegativeSamplesrecords the second case in the selected plan the executor runs.CounterDerivativeOfMetric(rate, increase, irate) stays counter-only, because a reset-corrected rate of a gauge can be negative.How
confirmed_weightspasses bothdeclared_counterand the existingdeclared_non_negative(the same rule the DDSketch sign composition and thevalues_non_negative[m]sign gate use) toWeightDomain::confirmed. A counter keeps theCounterSamplesproof. A metric that is only declared non-negative getsDeclaredNonNegativeSamples. The executor accepts anyNonNegativeproof for a keyed Count-Min build, so it needs no change.Key code interfaces added / modified
crates/types/src/ir/schema/sketch.rscrates/plan-selection/src/lib.rsUsage
This is
count_min_over_samples_of_a_gauge_declared_non_negative_is_admitted, run at the linked commit.Before this PR
Count-Min and CmsWithHeap over a non-negative gauge were always invalid, whatever the accuracy settings, because the samples were proven non-negative only for a counter.
After this PR
They pass the weight check and are judged on accuracy and cost like any other candidate. A rate or increase of that gauge is still rejected.
Limitation: through
plan_installable, a heap top-k plan is still refused by the executor compile check (ExecutorCannotCompile), because the executor can't run the fused heap's output yet. #831 fixes that. With both PRs, ASAPQuery's q06 (topk by (region, service) (4, sum_over_time(data[1m]))over the converter's non-negative gauge,--no-raw --failure-probability 0.3 --check --ignore-accuracy-guarantees) selects the CmsWithHeap plan at 2.243654e-8 per second instead of the exact one at 2.383717e-8. It passes the check: recall 0.9875 and max relative error at most 0.029 in each of 3 evaluations. The full table is in #831.Verification
count_min_over_samples_of_a_gauge_declared_non_negative_is_admitted:topk by (job) (10, sum_over_time(m[1m]))over a gauge declared non-negative. Every CmsWithHeap candidate passes the weight check, and the confirmed plan carriesDeclaredNonNegativeSamples.topk(10, sum by (job) (increase(m[5m])))over the same gauge is still rejected for its weights.confirmed_weights_rewrite_only_the_summary_updatewith the declared-non-negative gauge.confirmed_weightsnot reading the declaration, both tests fail (P2 rejected asCountMinWeightsNotProvenNonNegative; domainUnknownOrSigned).cargo test --workspace --locked --no-fail-faston the head (rebased on main at 334d549): 1,849 passed, 0 failed, 36 ignored.