Skip to content

Commit 95f79aa

Browse files
feat(store): added store interface for batched exact-window queries (#627)
* wip(query-engine): batched exact-window store query (#609) Checkpoint before merging main (legacy stores gated behind feature flag in #624). Not yet wired into scan_windows_via_exact; no tests yet. * wip(query-engine): wire batched exact-window query into scan_windows_via_exact (#609) Checkpoint before rebasing onto main (legacy stores removed in #625). * fix(query-engine): restore lock_profiling on batch exact-query, dedupe per_key/global Addresses review feedback on #609's batch method: - per_key.rs and global.rs had dropped the lock_profiling wait/hold-time instrumentation that the single-window path has, on exactly the path now most affected by longer lock hold times. - The per-window epoch-resolution loop (current_epoch/sealed_epochs lookup, read-count bookkeeping) was copy-pasted between the two backends. Extracted into common::resolve_exact_windows, shared by both.
1 parent 28d1bb6 commit 95f79aa

8 files changed

Lines changed: 500 additions & 28 deletions

File tree

asap-query-engine/src/engines/simple_engine/mod.rs

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -554,34 +554,32 @@ impl SimpleEngine {
554554
let window_size_ms = config.window_size_ms;
555555
let step_ms = Self::bucket_step_ms(config);
556556

557-
let mut merged: TimestampedBucketsMap = HashMap::new();
558557
if window_size_ms == 0 || step_ms == 0 || params.start_timestamp > params.end_timestamp {
559-
return Ok(merged);
558+
return Ok(HashMap::new());
560559
}
561560

561+
let mut windows: Vec<crate::stores::TimestampRange> = Vec::new();
562562
let mut window_start = params.start_timestamp.div_ceil(step_ms) * step_ms;
563563
while window_start + window_size_ms <= params.end_timestamp {
564-
let window_end = window_start + window_size_ms;
565-
let partial = self
566-
.store
567-
.query_precomputed_output_exact(
568-
&params.metric,
569-
params.aggregation_id,
570-
window_start,
571-
window_end,
572-
)
573-
.map_err(|e| {
574-
format!(
575-
"Error querying store for metric {}, agg {}, window [{}, {}]: {}",
576-
params.metric, params.aggregation_id, window_start, window_end, e
577-
)
578-
})?;
579-
for (key, buckets) in partial {
580-
merged.entry(key).or_default().extend(buckets);
581-
}
564+
windows.push((window_start, window_start + window_size_ms));
582565
window_start += step_ms;
583566
}
584-
Ok(merged)
567+
568+
// #609: one batched store call for the whole grid instead of one
569+
// query_precomputed_output_exact call per window.
570+
self.store
571+
.query_precomputed_output_exact_batch(&params.metric, params.aggregation_id, &windows)
572+
.map_err(|e| {
573+
format!(
574+
"Error querying store for metric {}, agg {}, {} windows in [{}, {}]: {}",
575+
params.metric,
576+
params.aggregation_id,
577+
windows.len(),
578+
params.start_timestamp,
579+
params.end_timestamp,
580+
e
581+
)
582+
})
585583
}
586584

587585
/// Executes a single store query based on parameters
@@ -2710,6 +2708,14 @@ mod merge_accumulators_regression_tests_596 {
27102708
) -> Result<TimestampedBucketsMap, Box<dyn std::error::Error + Send + Sync>> {
27112709
panic!("NoOpStore should not be called by merge_accumulators tests");
27122710
}
2711+
fn query_precomputed_output_exact_batch(
2712+
&self,
2713+
_: &str,
2714+
_: u64,
2715+
_: &[crate::stores::TimestampRange],
2716+
) -> Result<TimestampedBucketsMap, Box<dyn std::error::Error + Send + Sync>> {
2717+
panic!("NoOpStore should not be called by merge_accumulators tests");
2718+
}
27132719
fn get_earliest_timestamp_per_aggregation_id(
27142720
&self,
27152721
) -> Result<HashMap<u64, u64>, Box<dyn std::error::Error + Send + Sync>> {

asap-query-engine/src/stores/simple_map_store/common.rs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use crate::data_model::{AggregateCore, KeyByLabelValues};
2-
use std::collections::{HashMap, HashSet};
2+
pub use crate::stores::TimestampRange;
3+
use crate::stores::TimestampedBucketsMap;
4+
use std::collections::{BTreeMap, HashMap, HashSet};
35
use std::sync::{Arc, OnceLock};
6+
use tracing::debug;
47

58
pub type MetricID = u32;
69
pub type EpochID = u64;
7-
pub type TimestampRange = (u64, u64);
810
pub type MetricBucketMap = HashMap<MetricID, Vec<(TimestampRange, Arc<dyn AggregateCore>)>>;
911

1012
/// Sorts one key's buckets into chronological (ascending start) order.
@@ -418,3 +420,50 @@ impl SealedEpoch {
418420
windows
419421
}
420422
}
423+
424+
/// Resolves every window in `windows` against `current_epoch`/`sealed_epochs`, merging the
425+
/// results into one map. Shared by `SimpleMapStorePerKey` and `SimpleMapStoreGlobal`'s
426+
/// `query_precomputed_output_exact_batch` (#609) — the only difference between the two
427+
/// backends is how the outer per-aggregation lock is acquired and how `read_counts` is keyed,
428+
/// both handled by the caller. Returns `(results, matched_windows, total_entries)`;
429+
/// `matched_windows` is what the caller bumps read counts for.
430+
pub fn resolve_exact_windows(
431+
current_epoch: &MutableEpoch,
432+
sealed_epochs: &BTreeMap<EpochID, SealedEpoch>,
433+
intern: &InternTable,
434+
windows: &[TimestampRange],
435+
metric: &str,
436+
aggregation_id: u64,
437+
) -> (TimestampedBucketsMap, Vec<TimestampRange>, usize) {
438+
let mut results: TimestampedBucketsMap = HashMap::new();
439+
let mut matched_windows: Vec<TimestampRange> = Vec::new();
440+
let mut total_entries = 0;
441+
442+
for &window in windows {
443+
if window.0 > window.1 {
444+
debug!(
445+
"Invalid exact query range for metric {} agg_id {}: start {} > end {}",
446+
metric, aggregation_id, window.0, window.1
447+
);
448+
continue;
449+
}
450+
451+
let entries_opt = current_epoch.exact_query(window).or_else(|| {
452+
sealed_epochs
453+
.values()
454+
.rev()
455+
.find_map(|epoch| epoch.exact_query(window))
456+
});
457+
458+
if let Some(entries) = entries_opt {
459+
for (metric_id, agg) in entries {
460+
let label = intern.resolve(metric_id).clone();
461+
results.entry(label).or_default().push((window, agg));
462+
total_entries += 1;
463+
}
464+
matched_windows.push(window);
465+
}
466+
}
467+
468+
(results, matched_windows, total_entries)
469+
}

asap-query-engine/src/stores/simple_map_store/global.rs

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use crate::data_model::{
22
AggregateCore, AggregationType, CleanupPolicy, PrecomputedOutput, StreamingConfig,
33
};
44
use crate::stores::simple_map_store::common::{
5-
sort_buckets_chronologically, EpochID, InternTable, MetricBucketMap, MutableEpoch, SealedEpoch,
6-
TimestampRange,
5+
resolve_exact_windows, sort_buckets_chronologically, EpochID, InternTable, MetricBucketMap,
6+
MutableEpoch, SealedEpoch, TimestampRange,
77
};
88
use crate::stores::{Store, StoreResult, TimestampedBucketsMap};
99
use std::collections::{BTreeMap, HashMap, HashSet};
@@ -685,6 +685,96 @@ impl Store for SimpleMapStoreGlobal {
685685
Ok(results)
686686
}
687687

688+
/// Batched exact-window lookup (#609): acquires the process-wide lock once for the
689+
/// whole `windows` slice instead of once per window. Otherwise identical semantics to
690+
/// calling `query_precomputed_output_exact` once per window and merging the results
691+
/// (a window with no exact match simply contributes nothing).
692+
fn query_precomputed_output_exact_batch(
693+
&self,
694+
metric: &str,
695+
aggregation_id: u64,
696+
windows: &[TimestampRange],
697+
) -> Result<TimestampedBucketsMap, Box<dyn std::error::Error + Send + Sync>> {
698+
if windows.is_empty() {
699+
return Ok(HashMap::new());
700+
}
701+
702+
let query_start_time = Instant::now();
703+
let store_key = aggregation_id;
704+
705+
// Measure lock acquisition time
706+
#[cfg(feature = "lock_profiling")]
707+
let lock_wait_start = Instant::now();
708+
709+
let mut data = self.lock.lock().unwrap();
710+
711+
#[cfg(feature = "lock_profiling")]
712+
{
713+
let lock_wait_duration = lock_wait_start.elapsed();
714+
info!(
715+
"🔒 Batched exact query lock wait time: {:.2}ms (metric: {}, agg_id: {}, windows: {})",
716+
lock_wait_duration.as_secs_f64() * 1000.0,
717+
metric,
718+
aggregation_id,
719+
windows.len()
720+
);
721+
}
722+
723+
#[cfg(feature = "lock_profiling")]
724+
let lock_hold_start = Instant::now();
725+
726+
let per_key = match data.stores.get(&store_key) {
727+
Some(pk) => pk,
728+
None => {
729+
debug!(
730+
"Metric {} not found in store for batched exact query",
731+
metric
732+
);
733+
return Ok(HashMap::new());
734+
}
735+
};
736+
737+
let (results, found_windows, total_entries) = resolve_exact_windows(
738+
&per_key.current_epoch,
739+
&per_key.sealed_epochs,
740+
&per_key.intern,
741+
windows,
742+
metric,
743+
aggregation_id,
744+
);
745+
746+
// Update read counts (outer Mutex held — no inner Mutex needed)
747+
if !found_windows.is_empty() {
748+
let rc_map = data.read_counts.entry(store_key).or_default();
749+
for window in &found_windows {
750+
*rc_map.entry(*window).or_insert(0) += 1;
751+
}
752+
}
753+
754+
#[cfg(feature = "lock_profiling")]
755+
{
756+
let lock_hold_duration = lock_hold_start.elapsed();
757+
info!(
758+
"🔓 Batched exact query lock hold time: {:.2}ms (metric: {}, agg_id: {}, matched: {})",
759+
lock_hold_duration.as_secs_f64() * 1000.0,
760+
metric,
761+
aggregation_id,
762+
found_windows.len()
763+
);
764+
}
765+
766+
let query_duration = query_start_time.elapsed();
767+
debug!(
768+
"Batched exact timestamp query took: {:.2}ms ({} windows requested, {} matched, {} entries)",
769+
query_duration.as_secs_f64() * 1000.0,
770+
windows.len(),
771+
found_windows.len(),
772+
total_entries
773+
);
774+
775+
Ok(results)
776+
}
777+
688778
fn get_earliest_timestamp_per_aggregation_id(
689779
&self,
690780
) -> Result<HashMap<u64, u64>, Box<dyn std::error::Error + Send + Sync>> {

asap-query-engine/src/stores/simple_map_store/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,22 @@ impl Store for SimpleMapStore {
130130
}
131131
}
132132

133+
fn query_precomputed_output_exact_batch(
134+
&self,
135+
metric: &str,
136+
aggregation_id: u64,
137+
windows: &[crate::stores::TimestampRange],
138+
) -> Result<TimestampedBucketsMap, Box<dyn std::error::Error + Send + Sync>> {
139+
match self {
140+
SimpleMapStore::Global(store) => {
141+
store.query_precomputed_output_exact_batch(metric, aggregation_id, windows)
142+
}
143+
SimpleMapStore::PerKey(store) => {
144+
store.query_precomputed_output_exact_batch(metric, aggregation_id, windows)
145+
}
146+
}
147+
}
148+
133149
fn get_earliest_timestamp_per_aggregation_id(
134150
&self,
135151
) -> Result<HashMap<u64, u64>, Box<dyn std::error::Error + Send + Sync>> {

asap-query-engine/src/stores/simple_map_store/per_key.rs

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use crate::data_model::{
22
AggregateCore, AggregationType, CleanupPolicy, PrecomputedOutput, StreamingConfig,
33
};
44
use crate::stores::simple_map_store::common::{
5-
sort_buckets_chronologically, EpochID, InternTable, MetricBucketMap, MetricID, MutableEpoch,
6-
SealedEpoch, TimestampRange,
5+
resolve_exact_windows, sort_buckets_chronologically, EpochID, InternTable, MetricBucketMap,
6+
MetricID, MutableEpoch, SealedEpoch, TimestampRange,
77
};
88
use crate::stores::{Store, StoreResult, TimestampedBucketsMap};
99
use dashmap::DashMap;
@@ -756,6 +756,119 @@ impl Store for SimpleMapStorePerKey {
756756
Ok(results)
757757
}
758758

759+
/// Batched exact-window lookup (#609): acquires the shard's read lock once for the
760+
/// whole `windows` slice instead of once per window, resolving each window against
761+
/// `current_epoch` / `sealed_epochs` in a single pass. Otherwise identical semantics
762+
/// to calling `query_precomputed_output_exact` once per window and merging the
763+
/// results (a window with no exact match simply contributes nothing).
764+
fn query_precomputed_output_exact_batch(
765+
&self,
766+
metric: &str,
767+
aggregation_id: u64,
768+
windows: &[TimestampRange],
769+
) -> Result<TimestampedBucketsMap, Box<dyn std::error::Error + Send + Sync>> {
770+
if windows.is_empty() {
771+
return Ok(HashMap::new());
772+
}
773+
774+
let query_start_time = Instant::now();
775+
let store_key = aggregation_id;
776+
777+
#[cfg(feature = "lock_profiling")]
778+
let lock_wait_start = Instant::now();
779+
780+
let store_data_lock = match self.store.get(&store_key) {
781+
Some(lock) => lock,
782+
None => {
783+
debug!(
784+
"Metric {} not found in store for batched exact query",
785+
metric
786+
);
787+
return Ok(HashMap::new());
788+
}
789+
};
790+
791+
#[cfg(feature = "lock_profiling")]
792+
{
793+
let lock_wait_duration = lock_wait_start.elapsed();
794+
info!(
795+
"🔒 Batched exact query DashMap get time: {:.2}ms (metric: {}, agg_id: {}, windows: {})",
796+
lock_wait_duration.as_secs_f64() * 1000.0,
797+
metric,
798+
aggregation_id,
799+
windows.len()
800+
);
801+
}
802+
803+
#[cfg(feature = "lock_profiling")]
804+
let rwlock_wait_start = Instant::now();
805+
806+
// Same rationale as query_precomputed_output_exact: exact_query takes &self, so a
807+
// read lock covers the whole batch (issue #607).
808+
let data = store_data_lock.read().map_err(|e| {
809+
format!(
810+
"Failed to acquire read lock for batched exact query aggregation_id {}: {}",
811+
store_key, e
812+
)
813+
})?;
814+
815+
#[cfg(feature = "lock_profiling")]
816+
{
817+
let rwlock_wait_duration = rwlock_wait_start.elapsed();
818+
info!(
819+
"🔒 Batched exact query RwLock wait time: {:.2}ms (metric: {}, agg_id: {}, windows: {})",
820+
rwlock_wait_duration.as_secs_f64() * 1000.0,
821+
metric,
822+
aggregation_id,
823+
windows.len()
824+
);
825+
}
826+
827+
#[cfg(feature = "lock_profiling")]
828+
let lock_hold_start = Instant::now();
829+
830+
let (results, found_windows, total_entries) = resolve_exact_windows(
831+
&data.current_epoch,
832+
&data.sealed_epochs,
833+
&data.intern,
834+
windows,
835+
metric,
836+
aggregation_id,
837+
);
838+
839+
// Batch the read-count update too: one inner-Mutex acquisition for every window
840+
// that hit, instead of one per window.
841+
if !found_windows.is_empty() {
842+
let mut read_counts = data.read_counts.lock().unwrap();
843+
for window in &found_windows {
844+
*read_counts.entry(*window).or_insert(0) += 1;
845+
}
846+
}
847+
848+
#[cfg(feature = "lock_profiling")]
849+
{
850+
let lock_hold_duration = lock_hold_start.elapsed();
851+
info!(
852+
"🔓 Batched exact query lock hold time: {:.2}ms (metric: {}, agg_id: {}, matched: {})",
853+
lock_hold_duration.as_secs_f64() * 1000.0,
854+
metric,
855+
aggregation_id,
856+
found_windows.len()
857+
);
858+
}
859+
860+
let query_duration = query_start_time.elapsed();
861+
debug!(
862+
"Batched exact timestamp query took: {:.2}ms ({} windows requested, {} matched, {} entries)",
863+
query_duration.as_secs_f64() * 1000.0,
864+
windows.len(),
865+
found_windows.len(),
866+
total_entries
867+
);
868+
869+
Ok(results)
870+
}
871+
759872
fn get_earliest_timestamp_per_aggregation_id(
760873
&self,
761874
) -> Result<HashMap<u64, u64>, Box<dyn std::error::Error + Send + Sync>> {

0 commit comments

Comments
 (0)