Repository navigation
feat(rqe-optimizer): family properties, shared fixed-size sketches and DeltaSet key tracker - #160
Merged
Merged
Conversation
…d DeltaSet key tracker S4b of #143, including the cost-model part of #142. - family_properties(variant): mergeable_across_windows, one_fixed_size_sketch_for_all_groups, needs_delta_set_key_tracker; unknown variants panic. - hydra-kll joins Quantile families (undeployable). Its deployments carry the exact-delta-set row as key_tracker and are candidates only when card(G) <= the row's measured subpopulations. - Cost phases sum over sketch and tracker; a shared fixed-size sketch is one instance, not card(G). - Dominance pruning compares the cost model's full costs. - AutoSketch skips shared fixed-size families (#159). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… cost vectors computed once in pruning - priced_parts asserts a deployment carries a key tracker iff its family needs one, so a hydra Deployment built elsewhere can't be underpriced. - The key tracker row is used only when it is deployable or undeployable families are allowed. - Dominance pruning computes each candidate's cost vector once instead of per pair. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milindsrivastava1997
left a comment
Contributor
Author
There was a problem hiding this comment.
Review notes. The inline comments cover lines in the diff. Three more, on lines outside it:
rqe-optimizer/src/lib.rsaccuracy_at: Hydra rows have an emptymerge_accuracy, so a query merging n windows reads the single-windowerr. A fixed-size sketch folding n windows of every group holds n times the items in the same space, so for a quantile RAQE with L = 10x,accuracy_ok_forcan pass while the true rank error exceeds the SLA. Either measure Hydra at several merge counts or refuse n > 1 for Hydra until those rows exist.scripts/export_rqe_optimizer_costs.sh(~L152): this comment is out of date. It says the optimizer prices every row as card(G) x per-instance cost and that HydraKLL rows "must not become candidates until #142 lands". This PR is #142, so the comment should be updated here.docs/rqe_sketch_deployment_v1.md: the side-by-side table leaves out query memory and latency. Latency ((n-1)·c_mrg + G·c_qryfor Hydra vsG·(c_qry + (n-1)·c_mrg)per group) is the main reason Hydra survives pruning, so it belongs in the table.
All 50 rqe-optimizer tests pass at 23ec5f7.
This was referenced Oct 7, 2026
… tracker row - is_eligible takes facts and holds the group-count gate and the tracker-presence check, so the MILP, enumeration and pruning agree; facts is threaded through eligible_deployments_for and enumerate. - The key tracker row is used whenever its family is a candidate (no separate deployability check); more than one exact-delta-set row panics. - The pruning test zeroes the tracker's insert and memory, so only latency keeps Hydra. - Export script comment and design doc updated (latency and query memory rows; #165, #166). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Replies to the three review-body notes (42e3216):
🤖 Generated with Claude Code |
This was referenced Oct 7, 2026
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 #143 (S4b), including the cost-model part of #142. S7 was already done in #145.
Cost formulas by family shape
G = card(G),a_D = x/yopen windows,n = L/xwindows merged per query,C = (L − x)/y + 1closed windows,T= query interval.min the rowc_mrgin the rowc_ins/c_qryλ·a_D·c_insλ·a_D·c_insλ·a_D·c_insG·m·a_DG·m·a_Dm·a_DG·(n−1)·c_mrg/T(n−1)·c_mrg/TG·mmG·c_qry/TG·c_qry/TG·8BG·8BG·m·Cm·CG·(c_qry + (n−1)·c_mrg)(n−1)·c_mrg + G·c_qryA deployment that needs a key tracker also pays for a DeltaSet (
exact-delta-set,m_dsper key). It is priced like an exact accumulator on the sameG,xandy, in every phase:λ·a_D·c_ins_ds, memoryG·m_ds·a_DG·(n−1)·c_mrg_ds/T(union of the windows' key sets), memoryG·m_dsG·c_qry_ds/T(enumerate keys to probe)G·m_ds·CG·(c_qry_ds + (n−1)·c_mrg_ds)Design decisions (please approve)
These were settled in a design session. Reviewers, please approve them along with the code.
family_properties(variant)inlib.rs, next toDEPLOYABLE_FAMILIES, not in the cost JSONsubtractable?mergeable?mergeable_across_windows,truefor every family; a non-mergeable family is eligible only whenL == xcard(G)for ingest memory, merge, storage and latency; query stayscard(G) · c_qry(one probe per group)factsand compare the cost model's real costscard(G)is no longer a common multiplier within a groupcard(G) ≤ query_accuracy["subpopulations"]; more measured points in S5 (comment on #143)Deployment.key_trackercarries theexact-delta-setrow, priced as an exact accumulator in all phases (ingest, merge, query, storage, latency); no tracker row means no Hydra candidateLneeds the union of keys across the merged windows, so merge and query costs are realfamilies()only, planned only with--allow-undeployable-familiesmergeable_across_windows,one_fixed_size_sketch_for_all_groups,needs_delta_set_key_trackerfamily_propertiespanics, and a test covers everyfamilies()entrypriced_partsasserts a key tracker is present exactly when the family needs oneDeltaSetAggregator; the check would silently drop Hydra if it became deployable without the tracker being listedGvalue alone (one subkey per sample), so load =card(G). The current rows fan out to every label subset; re-measuring without fan-out is #165is_eligible(r, d, facts)holds the group-count gate and the tracker-presence check;factsis threaded througheligible_deployments_forandenumerate::*exact-delta-setrowInterface changes later sessions depend on
FamilyPropertiesandfamily_properties(variant)(lib.rs). Fields:mergeable_across_windows,one_fixed_size_sketch_for_all_groups,needs_delta_set_key_tracker. An unknown variant panics instead of getting a default; a test checks that everyfamilies()entry andKEY_TRACKER_FAMILYhas one.Deployment::properties()is a shorthand.L == x.falsehere, because the export already prices them per group.subtractableis left out until Subtract (S9) needs it.Deployment.key_tracker: Option<AtomicCostEntry>: theexact-delta-setrow for families that need a tracker,Noneotherwise. S8 reads this field to flag the tracker in the plan. If the cost table has no tracker row, families that need one are not candidates.KEY_TRACKER_FAMILY = "exact-delta-set". It is in no capability'sfamilies(), so it is never a candidate on its own.Capability::Quantile.families()but notDEPLOYABLE_FAMILIES, so it is planned only with--allow-undeployable-families.is_eligible(r, d, facts)(wasis_eligible(r, d)), andfactsadded toeligible_deployments_for,enumerate::{unservable, brute_force, for_each_mapping, for_each_mapping_while}. Eligibility now also requires a key tracker exactly when the family needs one.is_eligible): a family withone_fixed_size_sketch_for_all_groupsis eligible only whencard(G) ≤ query_accuracy["subpopulations"]of its row. Each row has one measured point, so larger groupings are never planned. Rows measured at more group counts (S5 follow-up, see the comment on Tracking: make rqe-optimizer the solver for ASAPQuery's planner #143) loosen the gate with no code change.instance_memory_bytestakes theconfigto size (the sketch or its tracker) as its first argument. DDSketch's value-range sizing (rqe-optimizer: size DDSketch memory from the metric's value range #152) applies to the sketch only.ingest,query_latency_ms,merge,storage_bytes) instead of per-group copies. Each candidate's cost vector is computed once and then compared pairwise.card(G)is no longer a common multiplier once shared and per-group sketches compete in the same group.exact-delta-setrow panics during candidate generation.one_fixed_size_sketch_for_all_groups, even with--allow-undeployable-families, because its ranking compares per-instance memory. Fixing that is rqe-optimizer: AutoSketch baseline should rank shared fixed-size sketches (HydraKLL) by deployment cost #159.Not in this PR
hydra-kllorexact-delta-setrows yet: the export hasn't been re-run since feat(bench): exact-delta-set row and HydraKLL in the rqe-optimizer cost export #149. Tests use hand-built rows.small_problemdoesn't print the key tracker (S8).Test plan
cargo test --workspace, clippy (-D warnings) and fmt via pre-commit.🤖 Generated with Claude Code