Skip to content

finish_range_context reads streaming_config twice, risking value/key config inconsistency on hot-reload #598

Description

@milindsrivastava1997

`finish_range_context` (`asap-query-engine/src/engines/simple_engine/promql.rs`) acquires `self.streaming_config.read()` twice: once for the value aggregation's `window_size_ms` (~line 579-584), once for the key aggregation's, added by #583 (~line 614-621). Each is a separate, momentary lock acquisition — the guard is dropped immediately after `.map(...)`.

`streaming_config: RwLock<Arc>` (`mod.rs:152`) is genuinely hot-swappable via `SimpleEngine::update_streaming_config` (`mod.rs:344-346`), presumably called when the planner recomputes configs. If a swap lands between the two read-lock acquisitions in `finish_range_context`, the value-side and key-side metadata for the same range query could be drawn from two different config generations — inconsistent window sizes, bucket widths, etc.

Before #583 there was only one `.read()` call here, so there was nothing to be inconsistent with. #583 added the second lookup, doubling the exposure window and introducing this specific new risk.

Fix should be simple: acquire the read lock once, look up both configs from the same guard before dropping it.

Found during a code review of #583/#587 (PR #595).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions