Skip to content

MutableEpoch::exact_query requires a write lock, serializing concurrent queries #607

Description

@milindsrivastava1997

MutableEpoch::exact_query (asap-query-engine/src/stores/simple_map_store/common.rs:238) takes &mut self because it lazily builds window_to_ids: Option<HashMap<TimestampRange, Vec<u32>>> on first access after any write, invalidating it on every insert. Callers (per_key.rs:620, global.rs:568, and legacy equivalents) must therefore take a write lock on the whole per-aggregation store shard for any exact-window query — even though the query itself doesn't mutate stored data, only an internal index.

By contrast:

  • query_precomputed_output (the range/scan call) only takes a read lock (per_key.rs:477, .read()).
  • SealedEpoch::exact_query (common.rs:350) already takes &self and does a binary search — sealed (immutable) epochs have no lock problem at all.

So the write-lock requirement is scoped to exactly one struct: the current, still-being-written MutableEpoch's lazy index.

Effect: any code path that calls query_precomputed_output_exact serializes against all other readers/writers of that aggregation's current epoch for the query's duration — a real concurrency cost, not just latency. This matters more once #608 routes more traffic through the exact-query call (currently only Sliding-window instant queries use it).

Proposed fix

Give MutableEpoch::exact_query a &self signature via interior locking scoped to just the index, e.g.:

  • An atomic "dirty" flag (replacing the plain window_to_ids = None field write on insert — should stay ~zero-cost, an atomic store instead of a bare field write).
  • The index itself behind its own small RwLock<Option<HashMap<TimestampRange, Vec<u32>>>> (or similar), rebuilt under that inner lock only when dirty.

This lets per_key.rs/global.rs (and their legacy equivalents) always take the outer per-shard lock as .read(), for both scan and exact queries. Insert's cost should remain effectively unchanged (still O(1) amortized, no HashMap work on the hot path).

Scope

  • asap-query-engine/src/stores/simple_map_store/common.rs (MutableEpoch)
  • asap-query-engine/src/stores/simple_map_store/per_key.rs and global.rs (lock acquisition at the call sites)
  • Legacy equivalents under simple_map_store/legacy/ — check whether they're still live or dead code before touching.

Testing

Needs both a correctness test (index still returns correct results across concurrent insert+query, dirty flag correctly forces rebuild) and, if feasible, a concurrency test demonstrating that two exact queries (or an exact query + a scan) no longer block each other.

Related: #581 (this removes a blocker for routing more of #581's fetch-mode unification through the exact-query call), #608.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

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