Skip to content

Precompute engine can't ingest planner-generated CountMinSketch configs, and silently mis-builds DeltaSetAggregator as Sum #477

Description

@zaoxing

Summary

When running query_engine_rust (precompute engine) against configs generated by asap-planner, any workload that produces approximate aggregations (sum / count / avg, including avg_over_time and ... by (label)) fails at ingest time. Two distinct defects in asap-query-engine/src/precompute_engine/accumulator_factory.rs are involved, both rooted in the factory not faithfully handling the aggregation types/parameters the planner emits — and a _ => default to Sum catch-all that turned them into warnings / wrong results instead of loud failures.

Environment

  • Component: asap-query-engine (precompute engine), src/precompute_engine/accumulator_factory.rs
  • Producer: asap-planner (--streaming_engine precompute)
  • Version: v0.5.x (defect 1 is a regression from refactor(query-engine): make sketch config params required in accumulator factory #397 “make sketch config params required in accumulator factory”)
  • Mode: standalone query_engine_rust --config-file <engine_config.yaml> with ingest.type: http_remote_write, Prometheus backend

Steps to reproduce

# 1. Plan a workload with approximate aggregations, e.g.:
#      avg(metric{...})
#      sum by(label)(metric{...})
#      avg_over_time(metric{...}[5m])
asap-planner --input_config workload.yaml --output_dir plan \
  --streaming_engine precompute --prometheus_scrape_interval 15
#    -> plan/streaming_config.yaml now contains CountMinSketch + DeltaSetAggregator entries

# 2. Run the engine on the generated configs
query_engine_rust --config-file engine_config.yaml
#    (streaming_config: plan/streaming_config.yaml)

# 3. remote_write samples for the planned metrics (e.g. fake_remote_writer --from-config)

A CountMinSketch aggregation, exactly as the planner emits it:

- aggregationId: 21
  aggregationType: CountMinSketch
  aggregationSubType: sum
  parameters:
    depth: 3        # planner emits depth/width
    width: 1024
  labels: { grouping: [], aggregated: [label_3], rollup: [] }
  metric: fake_metric_16
  windowSize: 15
  windowType: tumbling

Observed (actual)

WARN  ... worker: error processing group (21, ): CMS config missing required parameter: row_num
WARN  ... Unknown aggregation_type 'DeltaSetAggregator', defaulting to SingleSubpopulation Sum

Expected

The engine builds the correct sketch for every aggregation the planner emits; CountMinSketch and its companion DeltaSetAggregator ingest cleanly and serve approximate sum/count/avg queries.


Defect 1 — CountMinSketch parameter-name mismatch (hard error)

cms_params() (used by CountMinSketch and HydraKLL) only reads row_num/col_num, but the planner (asap-planner-rs/src/planner/sketch.rs) emits depth/width for CountMinSketch. So every plain-CMS aggregation throws CMS config missing required parameter: row_num on its first sample and never ingests.

Impact: all CountMinSketch aggregations fail to ingest → approximate sum/count (and avg, avg_over_time) queries are not served.

Defect 2 — DeltaSetAggregator / SetAggregator not wired (silent wrong result)

create_accumulator_updater() had no arm for DeltaSetAggregator or SetAggregator; they fell into the _ => default to Sum catch-all, which logged a warning and built a SumAccumulator.

The planner emits a DeltaSetAggregator companion with every CountMinSketch to record the set of subpopulation keys the CMS was fed (a CMS is a point-query structure with no key list of its own; the read path needs this set to enumerate per-key results). Built as a Sum, the key set is lost.

Impact: approximate keyed sum/count queries return wrong/empty results. Only a warning is logged, so it’s easy to miss.

Underlying issue

The _ => default to Sum catch-all in create_accumulator_updater() masked unhandled aggregation types as warnings + wrong results rather than failing loudly. An exhaustive match would have surfaced both defects as compile errors.


Proposed fix

  • cms_params(): accept depth/width first, then row_num/col_num (mirror cms_heap_params()).
  • Wire DeltaSetAggregator → new DeltaSetAggregatorUpdater and SetAggregator → new SetAggregatorUpdater (keyed updaters that add_key the aggregated-label key; value ignored). Add both to config_is_keyed().
  • Remove the _ => default to Sum catch-all so the match is exhaustive over AggregationType (future unhandled types become compile errors, not silent degradation).
  • Tests: CMS depth/width parses; row_num/col_num still works; DeltaSetAggregator builds the right accumulator and records distinct keys; config_is_keyed()is_keyed() consistency for the new types.

Affected files

  • asap-query-engine/src/precompute_engine/accumulator_factory.rs (fix)
  • asap-planner-rs/src/planner/sketch.rs (producer side, for the parameter-naming context)

Metadata

Metadata

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions