Skip to content

Binary-arithmetic PromQL queries re-parse every query_config on every arm lookup (O(#configs) parses, no cache) #493

Description

@zaoxing

Component: asap-query-engine (engines/simple_engine/promql.rs)
Perf. impact: Mid to High

Summary

SimpleEngine::find_query_config_promql_structural matches a (sub-)query to a precomputed QueryConfig by re-parsing and re-rendering every configured query string on every call, inside the .find() closure. It is reached from the two binary-arithmetic arm builders, so every leaf arm of a binary expression pays O(#query_configs) full PromQL parses (promql_parser::parser::parse) plus an AST→String render. Cost grows linearly with the number of planned queries.

Location & evidence (main @ afdde03)

asap-query-engine/src/engines/simple_engine/promql.rs:158

pub fn find_query_config_promql_structural(&self, arm_ast: &promql_parser::parser::Expr) -> Option<QueryConfig> {
    let arm_canonical = format!("{}", arm_ast);
    self.inference_config.read().unwrap().query_configs.iter()
        .find(|config| {
            let config_canonical = promql_parser::parser::parse(&config.query)  // ⬅ parse + render per config, per call
                .map(|ast| format!("{}", ast)).unwrap_or_default();
            config_canonical == arm_canonical
        })
        .cloned()
}
  • No cache: InferenceConfig.query_configs: Vec<QueryConfig>; QueryConfig { query: String, aggregations: Vec<AggregationReference> } — no stored parsed AST / canonical string.
  • Call sites (only 2, both binary-arm builders):
    • promql.rs:368build_arm_logical_plan (instant binary arm)
    • promql.rs:475build_arm_range_context (range binary arm)
  • Not on the primary path: single/aggregate queries resolve via find_query_config (mod.rs:319), a literal config.query == query scan with no parsing.
  • Per binary query: O(#leaf_arms × #configs) PromQL parses; the .find() short-circuits on match, so it's O(#configs) when the arm doesn't match any config.
  • Under lock: the parse loop runs while holding the inference_config read lock.

Impact

  • Binary-arithmetic query latency/CPU scales linearly with the planned-workload size (#query_configs).
  • Wasted repeated work: the same config strings are parsed on every such request.
  • Parser work held under the config read lock contends with config hot-reloads.

Suggested fix

Precompute each config's canonical form once when InferenceConfig is loaded/swapped (e.g. a HashMap<String /*canonical AST*/, QueryConfig> built under the RwLock write), and match format!("{}", arm_ast) against it — O(1) average, no per-call parsing. Preserve current handling of configs whose query fails to parse (currently mapped to the empty canonical string).

Notes

  • Separately, the primary path's find_query_config uses a literal string compare, so a planned query that differs only by whitespace / MetricsQL normalization won't match the structural form used by binary arms — a matching-consistency gap, not a perf issue. Unifying both on the cached canonical map would fix both at once.

Metadata

Metadata

Assignees

No one assigned

    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