Repository navigation
space-saving: drop total from the state (new ASAPv1 kinds 0x18 0x01, 0x1e 0x01, 0x1f 0x01) - #258
Merged
Merged
Conversation
…0x1e 0x01, 0x1f 0x01) SpaceSaving no longer keeps the weight it recorded: no readout reads it and after a merge it is not derivable from the counts. total() and the decode rule that total cover the counted weight are removed. Space-Saving, CountMinSketchWithSpaceSaving and CountSketchWithSpaceSaving encode under new kinds without total; the retired 0x18 0x00 / 0x1e 0x00 / 0x1f 0x00 still decode, total dropped, and are never encoded. Golden fixtures for the new kinds; docs, API pages, tests.md and CHANGELOG updated. Part of #250 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol
marked this pull request as ready for review
October 29, 2026 00:05
zzylol
force-pushed
the
space-saving/drop-total
branch
from
October 29, 2026 00:05
0abfa05 to
624240b
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.
Part of #250. Golden fixtures: ProjectASAP/sketchlib-golden-bytes#7 (merge first; the submodule pointer then moves to the merged commit).
Why
A Space-Saving summary answers with its
(key, count, error)triples and its ceilingdiscarded_max.SpaceSavingalso kepttotal, the weight it had recorded, and the ASAPv1 payloads of Space-Saving (§3.5), CountMinSketchWithSpaceSaving (§3.24) and CountSketchWithSpaceSaving (§3.25) carried it. No Space-Saving readout reads it:estimate,error,upper_bound,min_count,is_guaranteedandtop_kread the triples and the ceiling. Its only uses were thetotal()accessor and a decode rule thattotalcover the counted weight. It is also not derivable from the state: after a merge the counts include themin_counteach side adds for keys the other did not hold, and a trim drops counted weight.The design decision for #250: sum and count are a separate exact aggregate, not sketch state. This PR is the Space-Saving part.
What
SpaceSaving's state is the counters, the buckets anddiscarded_max. Thetotalfield andtotal()are removed.sum(count - error) <= total(§3.5 rule 10) is removed.kind_idmismatch error names the retired id too, as the DDSketch decoder does.0x18 0x01(Space-Saving),0x1e 0x01(CountMinSketchWithSpaceSaving) and0x1f 0x01(CountSketchWithSpaceSaving), each payload withouttotal. The retired0x18 0x00/0x1e 0x00/0x1f 0x00still decode, withtotalread and dropped, and are never encoded.space-saving-nototal(f6fcfe7, off golden-bytesmain).message_pack_format.md;docs/tests.md;tests/PBT_COVERAGE.md; CHANGELOG.Deleted public API (orphaned by this change):
SpaceSaving::total()summary_wire::encode(the payload'stotal) and tests onlyNo estimate or bound changes: none of them read
total. The capability table carries no Space-SavingTotalrow (main after #254).How
space_saving.rs: the field, its updates inupdate_weighted,clearandmerge, and the rebuild check are removed.SpaceSavingStateandfrom_partslosetotal.space_saving/wire.rs:SpaceSavingPayloadis[keys, counts, errors, discarded_max].SpaceSavingPayloadWithTotalreads the retired five-field payload;read_payloadconverts it by droppingtotal.from_asapv1accepts0x18 0x01or0x18 0x00and reads each with its own shape.countminsketch_space_saving/summary_wire.rs: the same for the shared CMS / CS payload (SummaryTopKPayload,SummaryTopKPayloadWithTotal).SummaryKindgainsretired_kind_id_with_total, so each of the two kinds decodes its own retired id.Key code interfaces added / modified
src/sketches/space_saving.rssrc/sketches/space_saving/wire.rsCountMinSketchWithSpaceSaving/CountSketchWithSpaceSaving(.../wire.rs)src/asapv1/kind.rs(crate-private; the wire ids)Wire format (
docs/asapv1_wire_format.md)0x18 0x01[keys, counts, errors, discarded_max]0x18 0x00[keys, counts, errors, total, discarded_max]totaldropped0x1e 0x01/0x1f 0x01[counts, keys, summary_counts, errors, discarded_max]0x1e 0x00/0x1f 0x00[counts, keys, summary_counts, errors, total, discarded_max]totaldroppedUsage
A collector pane tracks heavy-hitter status codes and keeps the request count in its own exact counter. This was compiled and run as an integration test on this branch (at
0abfa05):Before this PR
SpaceSavingkepttotal, exposedtotal(), carried it on the wire (0x18 0x00, and inside0x1e 0x00/0x1f 0x00) and rejected payloads whosetotalwas under the counted weight.After this PR
The summary is its triples and ceiling (
0x18 0x01,0x1e 0x01,0x1f 0x01). Every estimate and bound is unchanged, and older bytes still decode.Verification
Only the tests covering the changed code, plus format and lint, on
0abfa05(rebased onmain21b6357):cargo test --lib -- asapv1 wire_key envelope space_saving: 117 passed.--features experimental --lib -- space_saving asapv1: 109 passed.cargo test --test asapv1_golden: 71 passed (default features), 72 passed (experimental).cargo test --test pbt -- space_saving: 19 passed.cargo test --release --test e2e -- heavy space_saving wire: 117 passed.cargo test --test conformance_kit: 13 passed.cargo test --test capability: 3 passed (default andexperimental).cargo fmt --check: clean.cargo clippy --all-targetsand--all-targets --features experimental: no warnings.space_saving_decodes_the_retired_kind_with_total,the_retired_kind_with_total_decodes_and_drops_it: retired bytes decode for anytotal(including ones under the counted weight, which the old rule rejected) and re-encode under the new id; each id rejects the other's payload shape.space_saving_retired_fixtures_decode_to_their_nototal_twins, and the CMS / CS golden tests: each retired fixture re-encodes as its*_nototal_*twin.total_covers_the_recorded_weight_saturatingand thetotal of 0 under the 9crafted cases (the rule is gone). The e2e testa_merge_keeps_the_total_and_never_reads_lowis renameda_merge_never_reads_low, since it no longer checks a total.pbt/e2esuites, and benchmarks. CI is not run on this draft.Limitations and follow-ups
space-saving-nototalbranch first, then this submodule pointer moves to the merged commit.sketches/SpaceSavingserializes with gob only); itswire/asapv1/kind.goregistry gets the new ids (18 01,1e 01,1f 01; the0x00ids retired) in a separate sketchlib-go PR, stacked on its DDSketch registry branch.SpaceSaving::total()keeps its own count.🤖 Generated with Claude Code