Skip to content

fix(precompute): restore weighted sketch updates - #482

Merged
zzylol merged 2 commits into
mainfrom
revert/frequency-unit-weight
Sep 3, 2026
Merged

zzylol merged 2 commits into
mainfrom
revert/frequency-unit-weight

Conversation

@zzylol

@zzylol zzylol commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Before

PR #481 treated every raw Prometheus sample sent to bare CMS and CountSketch as one event. That changes metric-value aggregation into scrape counting: samples with values 2, 3, and 5 estimate 3 instead of 10.

ASAPQuery #665 did not make unit weighting unconditional. It introduced an explicit count_events choice between 1.0 and the datapoint value. The current backend physical plan has no equivalent typed count-vs-value contract, so inferring count mode from the sketch kind is incorrect.

After

  • Revert the unconditional unit-weight update from fix(precompute): count frequency sketch events #481.
  • Bare CMS and CountSketch again add each raw Prometheus sample value.
  • Document that event counting requires an explicit typed physical-plan contract.
  • Add direct regression tests proving values 2 + 3 + 5 estimate 10 for both sketches.
  • Preserve the existing explicit Value/Count modes for heap sketches.

Verification

  • cargo fmt -p data_plane -- --check
  • cargo test -p data_plane bare_cms_adds_sample_values
  • cargo test -p data_plane bare_count_sketch_adds_sample_values

Both targeted tests pass. Existing compiler warnings are unchanged.

@zzylol
zzylol merged commit 0273f46 into main Sep 3, 2026
1 check passed
@zzylol
zzylol deleted the revert/frequency-unit-weight branch September 3, 2026 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant