Skip to content

edge: CountSketch dim-validation + nil-guards, drop counters, warm-lateness, cache pruning, hot-path scratch (review follow-ups) - #464

Merged
zzylol merged 1 commit into
mainfrom
fix/edge-review-followups
May 27, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/edge-review-followups

Conversation

@zzylol

@zzylol zzylol commented May 27, 2026

Copy link
Copy Markdown
Contributor

Code-review follow-ups to the asap_edge edge processor + precompute runtime. Part of a 3-repo review-fix set (with sketchlib-go fix/wire-format-review-followups + ASAPQuery-backend fix/backend-review-followups).

Fixes

  • P0 CountSketch nil-panic: boot-time 64-bit row-hash budget validation (rows*ceil(log2(cols))<=64, pow2 cols); factory no longer discards the constructor error (logs + skips instead of a nil-backed sketch); wrapper nil-guards.
  • P0 silent series drop: DroppedSerialize/encode-drop counters instead of invisible drops.
  • P1 warm lateness: new WarmAllowedLateness knob (defaults to WindowDuration) decouples warm-window admission from the 2s cold reorder-grace.
  • P1 unbounded SnapshotCache (agent memory pin): RetainKeys prunes vanished series each window.
  • P1 hot-path allocs: per-shard scratch buffers in observe() (residual string allocs flagged for a future zero-alloc API).
  • P1 envelope double-count guard; delta-threshold comment; dead code removed. Tests added.

No exported signatures changed. Excludes unrelated working-tree edits (asap-gorilla-go/intchunk).

🤖 Generated with Claude Code

…unters, warm-lateness, SnapshotCache pruning, hot-path scratch

Code-review follow-ups to the asap_edge edge processor + precompute runtime:

- P0 CountSketch nil-panic: config_validate.go pre-validates the 64-bit row-hash
  budget (rows*ceil(log2(cols))<=64, pow2 cols) for FamilyCountSketch and rejects
  at boot; warm_sketch.go stops discarding the wrapper constructor error (logs +
  skips wiring instead of building a nil-backed sketch); countsketch.go wrapper
  nil-guards UpdateString/Snapshot/Reset.
- P0 silent series drop: precompute.go finishRotate + the processor flush now
  increment a DroppedSerialize / encode-drop counter (telemetry.go) instead of
  dropping invisibly.
- P1 warm lateness: new Config.WarmAllowedLateness knob (defaults to
  WindowDuration) decouples warm-window admission from the cold reorder-grace (2s),
  so processing-delayed-but-in-window samples are no longer dropped.
- P1 unbounded SnapshotCache (agent memory pin): SnapshotCache.RetainKeys prunes
  outbound/inbound entries for series absent from the just-closed window.
- P1 hot-path allocs: per-shard scratch buffers (kv slice, attr-key bytes,
  Observation) reused in observe() (residual SeriesKeyFor/AttributesKey string
  allocs flagged for a future zero-alloc precompute API).
- P1 envelope double-count guard (forwarder path); delta-threshold comment fixed;
  dead Shutdown code removed. Tests added throughout.

No exported signatures changed (all additions are additive).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol marked this pull request as ready for review May 27, 2026 18:47
@zzylol

zzylol commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Integrated end-to-end validation — 6/6 query families pass warm

All five parallel review-fix agents' changes were integrated (disjoint files, no conflicts) and validated together on a fresh localhost deploy of all three rebuilt images:

Family Query Result
SUM sum by (zone)(http_requests_total) ✅ asap_query
KLL quantile_over_time(0.99, http_requests_total_latency_ms[30s])
KLL quantile_over_time(0.99, request_size_bytes[30s])
HLL count(unique_users_per_min)
CMS count_over_time(endpoint_request_freq[30s])
TopK topk(5, top_endpoint_qps)
  • Integrated cargo check clean; data_plane + control_plane lib tests pass (815 + control_plane, 0 failures); asapedgeprocessor + sketchlib-go Go suites pass (pre-existing CocoSketch / TestKLL_Reset / XTEST_DIR-gated / TestHLLWrapper_DeltaNeverLargerThanFull failures confirmed unrelated — fail identically on clean main).
  • One integration regression was caught + fixed during validation: a controller with_heap default change had dropped the heap emit for the (unenumerated) top-k CountSketch — fixed by keying with_heap off the metric's item_label, restoring topk(top_endpoint_qps).

Known non-blocking item: the new rate(cms_metric) → warm-FrequencyEstimate fallback fires in unit tests but, for the tier:both demo metric, the cold archive answers rate() exactly first; the demo's CMS query (count_over_time) resolves warm. Flagged for follow-up.

🤖 Generated with Claude Code

@zzylol
zzylol merged commit 6dd5b36 into main May 27, 2026
@zzylol
zzylol deleted the fix/edge-review-followups branch May 27, 2026 18:59
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