Skip to content

control_plane: honor sketch_family_override for HLL/CMS/CountSketch in agent emit - #357

Merged
zzylol merged 3 commits into
mainfrom
fix/controller-emit-sketch-family
May 29, 2026
Merged

zzylol merged 3 commits into
mainfrom
fix/controller-emit-sketch-family

Conversation

@zzylol

@zzylol zzylol commented May 29, 2026

Copy link
Copy Markdown
Contributor

Problem

bind_workload_typed derived the statistic class purely from the query's AggType (Quantile/Cardinality/Frequency). When a workload entry pinned a non-quantile sketch_family_override (HLL / CountMinSketch / CountSketch) but its query classified as a different/incompatible class — e.g. count(...), count_over_time(...), topk(...) landing on Quantile — is_valid_pair(override, statistic) rejected the override and the binder fell back to the catalog default, DDSketch.

Result: the controller emitted family: ddsketch to the fused asap_edge agent for every HLL/CMS/CountSketch metric. Those families never ran at the edge, so their warm queries (count, count_over_time, topk) could never resolve on the warm tier.

Observed live (multi-sketch controller workload): a workload declaring DDSketch/KLL/HLL/CMS/CountSketch on 5 metrics emitted to the agent as DDSketch/KLL/ddsketch/ddsketch/ddsketch — only DDSketch and KLL (both Quantile-class) survived.

Fix

An explicit override is authoritative for the family and the statistic class it answers. When the query-derived statistic is incompatible with the override, re-derive the statistic from the override:

  • DDSketch / KLL → Quantile
  • HLL → Cardinality
  • CountMinSketch → Frequency
  • CountSketch → TopK

so the override drives both. DDSketch/KLL quantile paths are unchanged (the override is already valid for the derived statistic, so the re-derivation is a no-op).

Verification

  • New regression unit test override_pins_nonquantile_family_over_misclassified_query (drives a Quantile-classified query with each override → asserts the bound family matches the override, not DDSketch).
  • Live (controller-driven multinode, supervised agent): the agent's effective fused config now carries the correct families + params:
gct_ms_ddsketch → family: ddsketch
gct_ms_kll      → family: kll
gct_ms_hll      → family: hll              item_label: service
gct_ms_cms      → family: countminsketch   item_label: service
gct_ms_topk     → family: countsketch      item_label: host   emit_heap: true

(previously hll/cms/topk all emitted as ddsketch).

🤖 Generated with Claude Code

zzylol and others added 3 commits May 29, 2026 08:32
…n the agent emit

`bind_workload_typed` derived the statistic class purely from the query's
AggType (Quantile/Cardinality/Frequency). When a workload entry pinned a
non-quantile `sketch_family_override` (HLL / CountMinSketch / CountSketch)
but its query classified as a different/incompatible class (e.g.
`count(...)`, `count_over_time(...)`, `topk(...)` landing on Quantile),
`is_valid_pair(override, statistic)` rejected the override and the binder
fell back to the catalog default — DDSketch. The controller therefore
emitted `family: ddsketch` to the fused asap_edge agent for every
HLL/CMS/CountSketch metric, so those families never ran at the edge and
their warm queries couldn't resolve.

Fix: an explicit override is authoritative for the family AND the statistic
it answers. When the query-derived statistic is incompatible with the
override, re-derive the statistic from the override (HLL→Cardinality,
CountMinSketch→Frequency, CountSketch→TopK, DDSketch/KLL→Quantile) so the
override drives both. DDSketch/KLL quantile paths are unchanged.

Verified live (controller-driven multinode, multi-sketch workload): the
agent's effective config now carries family: hll / countminsketch /
countsketch (+ item_label / emit_heap) for the respective metrics instead
of all-ddsketch. Adds a regression unit test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… cols (fix topk explosion + crash)

Two fixes that make the heap-bearing CountSketch (warm topk) actually
deployable end-to-end:

1. Exclude item_label from aggregate_by (emit_edge_yaml_asap_edge).
   A `topk(10, sum by (host) (m))` workload with grouping_labels:[zone]
   folds `host` into grouping_labels, so the emitter wrote
   aggregate_by:[host, zone]. But `host` is the item_label — the
   heavy-hitter dimension fed to the top-k heap, NOT a series grouping
   key. Leaving it in keyed the edge series PER host: ~5000 per-host
   sids + heaps instead of one heap per zone (cardinality explosion that
   also collapsed agent throughput ~100x). Now the emitted aggregate_by
   drops the item_label → 4 per-zone CountSketchWithHeap sids. Verified
   live: gct_ms_topk registers {['zone']: 4} (was 4598).

2. Round CountSketch cols up to the next power of two
   (BindCountSketchOnTopK). The ε-derived width `ceil(e/eps)` (e.g. 55
   for eps=0.05) is not a power of two, but the agent's
   asapedgeprocessor config_validate REJECTS non-pow2 countsketch cols
   (sketchlib bit-slices the hash with a pow2 column mask) → the
   collector crashed on config apply ("cols=55 must be a power of two"),
   so the agent never came up. Rounding up only tightens the bound
   (ε ≤ e/w). Verified live: cols=64, agent starts clean.

Adds regression tests: fused_emit_excludes_item_label_from_aggregate_by.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…m dim in group_by_keys

A topk query `topk(k, sum by (item) (m))` lowers to a FrequencyTopk
candidate whose group_by_keys includes the inner `by (item)` dim. But
for a heap-bearing CountSketch the item is the heap's ranked dimension
(the sid's item_label, projected OUT of the series key); the sid is
grouped by its own grouping_labels (e.g. zone). Requiring the item in
the sid's group_by_keys never matches (item not subset of {zone}) so the
topk query fell through to archive and returned empty. Clear group_by_keys
for FrequencyTopk candidates so the metric's heap-bearing sids match by
metric+capability; the reducer reads each heap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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