Skip to content

fix(topk): preserve typed count updates and migrate acceptance to registered QueryPlans - #511

Merged
zzylol merged 9 commits into
mainfrom
fix/topk-materialization-e2e
Sep 8, 2026
Merged

fix(topk): preserve typed count updates and migrate acceptance to registered QueryPlans#511
zzylol merged 9 commits into
mainfrom
fix/topk-materialization-e2e

Conversation

@zzylol

@zzylol zzylol commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Why

TopK binding must retain its typed count updates, and its acceptance tests must exercise the integrated architecture: register a temporal query, select a Planner DAG, install the physical QueryPlan, then execute that binding.

Before this PR

The legacy emitter dropped the selected count mode. Two heap E2Es installed only StreamingConfig and requested unregistered instantaneous topk(3, top_endpoint_qps); both failed with No result for query.

After this PR

The evidence-aware legacy adapter retains weight_mode=count and fails closed without membership evidence. The two serving tests are migrated to:

topk(3, count_over_time(top_endpoint_qps[5s]))

For six series with counts alpha=100, beta=50, gamma=200, delta=75, epsilon=10 and zeta=150, both CMS-with-heap and CountSketch-with-heap return gamma=200, zeta=150 and alpha=100. Every raw sample has value 17, so a value-weighted implementation cannot accidentally pass.

How and scope

  • Use the pinned Planner's forced-family cost model and explicit fixture membership evidence to cover each heap implementation.
  • Compile and install the complete physical artifact in the production backend; assert the selected materialization family and count mode.
  • Ingest raw Remote Write samples; check all three item identities, exact counts, timestamps, two range steps and retry deduplication.
  • Verify unregistered instantaneous TopK follows configured exact fallback, without serving-time replanning.
  • Replace the two obsolete heap-serving tests and remove their unused CMS envelope helper. Other legacy OTLP fixtures are unchanged.

Based on #505, which already contains the shared materialization identity and canonical temporal TopK changes originally developed here. This acceptance migration is backend-local, not distributed OTLP/sketch-envelope parity. Existing item labels retain canonical series identities; native label reconstruction is not claimed.

Fixture certificates are deterministic test inputs, not a production evidence estimator. This PR does not close Planner #340's entire legacy acceptance scope.

Verification

Current commit 1a85748:

  • cargo test -p data_plane --test asapquery_compatibility_process_e2e: 4 passed, including both newly migrated heap tests.
  • cargo test -p data_plane --test e2e_controller_plans_and_backend_serves --no-run: passed.
  • Formatting of the changed process test and git diff --check: passed.
  • Before migration, both original heap E2Es were rerun and reproduced their No result failures.

Earlier in this PR: 656 control-plane library tests and the distributed production-process test passed for the emitter/base integration. The complete legacy E2E suite is not claimed green.

Self-review, not independent approval. No merge performed.

@zzylol zzylol changed the title fix(topk): separate weighted materializations and verify results fix(topk): bind evidenced workloads and verify materializations Sep 7, 2026
@zzylol zzylol changed the title fix(topk): bind evidenced workloads and verify materializations fix(topk): retain evidence and count semantics in legacy workload bindings Sep 8, 2026
@zzylol zzylol changed the title fix(topk): retain evidence and count semantics in legacy workload bindings fix(topk): preserve typed count updates and migrate acceptance to registered QueryPlans Sep 8, 2026
@zzylol
zzylol changed the base branch from feat/501-promql-compatibility to main September 8, 2026 20:28
@zzylol
zzylol merged commit fe54701 into main Sep 8, 2026
1 check passed
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