phughk/ottyl
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Seek log/metric-bucket queries by key range instead of scanning

Browse Source 
Log reads (logs_since, the unsorted Logs query path) now seek the
IngestedLog range directly from since_sequence/before_sequence instead
of scanning from IndexKey::Start and filtering every prior entry out
in a closure. Metric-comparison filters and bucket_values now seek to
a single metric's Bucket key range instead of scanning every declared
metric's buckets. evaluate_promql sits on top of bucket_values, so its
per-poll dashboard evaluation is narrowed by the same fix without
further changes; caching prior evaluations across polls is left for a
follow-up if profiling ever shows it's needed (per ticket 081's own
scope, not built speculatively).

Resolves tickets 077, 078, 081.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TeyhRfMYjMditek6geDRoQ
This commit is contained in:
Hugh Kaznowski
2026-09-06 20:51:24 +01:00
co-authored by Claude Sonnet 5
parent 65330d37e0
commit 946fd50d53
8 changed files with 128 additions and 195 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/engine/snapshot/metrics.rs
+11 -1
View File
@@ -298,8 +298,18 @@ impl EngineSnapshot {
labels_filter: Option<&'a BTreeMap<String, MetricPrimitive>>,
) -> impl Iterator<Item = (&'a BTreeMap<String, MetricPrimitive>, &'a (u64, Timestamp, MetricPrimitive))> + 'a
{
// IndexKey::Bucket sorts by (metric_name, labels), so bound the range to just this
// metric's buckets instead of ranging MetricBucketSplit..End over every declared
// metric. '\u{10FFFF}' is the maximum Unicode scalar value, so `metric_name` followed
// by it sorts after every real key for this metric (any labels) and before any key
// for a lexicographically greater metric name.
let lower = IndexKey::Bucket { metric_name: metric_name.to_string(), labels: BTreeMap::new() };
let upper = IndexKey::Bucket {
metric_name: format!("{metric_name}\u{10FFFF}"),
labels: BTreeMap::new(),
};
self.index
.range(IndexKey::MetricBucketSplit..IndexKey::End)
.range(lower..upper)
.filter_map(move |(key, value)| match (key, value) {
(IndexKey::Bucket { metric_name: name, labels }, DataValue::BucketValue { values })
if name == metric_name && labels_filter.is_none_or(|filter| filter == labels) =>
src/engine/snapshot/mod.rs
+33 -7
View File
@@ -157,13 +157,41 @@ impl EngineSnapshot {
}
pub(crate) fn logs_range(&self) -> impl Iterator<Item = (u64, Timestamp, &str)> {
self.index.range(IndexKey::Start..IndexKey::LogParamSplit).filter_map(|(key, value)| {
match (key, value) {
self.logs_in_sequence_range(..)
}
/// Seeks directly to `sequence_range` within the `IngestedLog` key range instead of
/// scanning from `IndexKey::Start` and filtering after the fact. `IngestedLog` sorts by
/// `(sequence, timestamp)`, and `sequence` is assigned monotonically at ingest
/// (`handle.rs`), so a bound on `sequence` alone is an exact seek — `Timestamp::MIN` is
/// used as the synthetic sub-key since it sorts below any real timestamp for that sequence.
pub(crate) fn logs_in_sequence_range(
&self,
sequence_range: impl std::ops::RangeBounds<u64>,
) -> impl Iterator<Item = (u64, Timestamp, &str)> {
let lower = match sequence_range.start_bound() {
std::ops::Bound::Included(&sequence) => {
IndexKey::IngestedLog { sequence, timestamp: Timestamp::MIN }
}
std::ops::Bound::Excluded(&sequence) => {
IndexKey::IngestedLog { sequence: sequence.saturating_add(1), timestamp: Timestamp::MIN }
}
std::ops::Bound::Unbounded => IndexKey::Start,
};
let upper = match sequence_range.end_bound() {
std::ops::Bound::Included(&sequence) => {
IndexKey::IngestedLog { sequence: sequence.saturating_add(1), timestamp: Timestamp::MIN }
}
std::ops::Bound::Excluded(&sequence) => {
IndexKey::IngestedLog { sequence, timestamp: Timestamp::MIN }
}
std::ops::Bound::Unbounded => IndexKey::LogParamSplit,
};
self.index.range(lower..upper).filter_map(|(key, value)| match (key, value) {
(IndexKey::IngestedLog { sequence, timestamp }, DataValue::LogLiteral { text }) => {
Some((*sequence, *timestamp, text.as_str()))
}
_ => None,
}
})
}
@@ -173,10 +201,8 @@ impl EngineSnapshot {
since_ms: i64,
now_ms: i64,
) -> impl Iterator<Item = (u64, Timestamp, &str)> {
self.logs_range().filter(move |(sequence, timestamp, _)| {
*sequence < before_sequence
&& timestamp.as_millis() > since_ms
&& timestamp.as_millis() <= now_ms
self.logs_in_sequence_range(..before_sequence).filter(move |(_, timestamp, _)| {
timestamp.as_millis() > since_ms && timestamp.as_millis() <= now_ms
})
}
}
src/engine/snapshot/query.rs
+11 -4
View File
@@ -56,11 +56,14 @@ pub(super) fn handle_query_inner(
}
let limit = params.limit.unwrap_or(500).min(10_000);
let compiled_params = snapshot.compiled_param_declarations();
let since_bound = match params.since_sequence {
Some(since) => std::ops::Bound::Excluded(since),
None => std::ops::Bound::Unbounded,
};
let filtered = snapshot
.logs_range()
.filter(|(sequence, timestamp, _)| {
params.since_sequence.is_none_or(|since| *sequence > since)
&& params.start_ms.is_none_or(|start| timestamp.as_millis() >= start)
.logs_in_sequence_range((since_bound, std::ops::Bound::Unbounded))
.filter(|(_, timestamp, _)| {
params.start_ms.is_none_or(|start| timestamp.as_millis() >= start)
&& params.end_ms.is_none_or(|end| timestamp.as_millis() <= end)
})
.filter(|(sequence, timestamp, text)| {
@@ -80,6 +83,10 @@ pub(super) fn handle_query_inner(
})
.collect(),
Some(sort_by) => {
// Not range-bounded by since_sequence/start_ms: SortHelperKey::WithParam
// sorts by the param's value first (that's the point — it's what gives
// sort-by-param order), so sequence/time bounds can't narrow a BTreeMap
// range seek here the way they do for the unsorted path above.
static EMPTY_HELPER: std::sync::LazyLock<BTreeMap<SortHelperKey, Timestamp>> =
std::sync::LazyLock::new(BTreeMap::new);
let helper_index =
src/engine/tests.rs
+65
View File
@@ -381,6 +381,34 @@ use std::collections::{BTreeMap, HashMap, HashSet};
assert_eq!(index.approx_bytes_total, expected_bytes);
}
#[test]
fn logs_in_sequence_range_seeks_instead_of_scanning_from_start() {
let mut index = EngineSnapshot::default();
for sequence in 0..1000u64 {
index.index.insert(
IndexKey::IngestedLog { sequence, timestamp: Timestamp::from_millis(sequence as i64) },
DataValue::LogLiteral { text: format!("line {sequence}") },
);
}
// An excluded lower bound (as `since_sequence` polling uses) must start exactly past
// the bound, not include it or anything before it.
let since_990: Vec<_> =
index.logs_in_sequence_range(990..).map(|(seq, _, _)| seq).collect();
assert_eq!(since_990, vec![990, 991, 992, 993, 994, 995, 996, 997, 998, 999]);
let after_990: Vec<_> = index
.logs_in_sequence_range((std::ops::Bound::Excluded(990u64), std::ops::Bound::Unbounded))
.map(|(seq, _, _)| seq)
.collect();
assert_eq!(after_990, vec![991, 992, 993, 994, 995, 996, 997, 998, 999]);
// An exclusive upper bound (as `logs_since`'s `before_sequence` uses) must stop
// strictly before it.
let before_5: Vec<_> = index.logs_in_sequence_range(..5u64).map(|(seq, _, _)| seq).collect();
assert_eq!(before_5, vec![0, 1, 2, 3, 4]);
}
#[test]
fn evict_excess_logs_is_noop_when_unlimited_or_under_cap() {
let mut index = EngineSnapshot::default();
@@ -1381,6 +1409,43 @@ use std::collections::{BTreeMap, HashMap, HashSet};
assert_eq!(*value, MetricPrimitive::Number(Number::Int(5)));
}
#[test]
fn bucket_values_seeks_by_metric_name_and_ignores_lexicographic_neighbors() {
let mut index = EngineSnapshot::default();
declare_test_param(
&mut index,
"value",
r"value=(?P<value>\d+)",
"value",
MetricPrimitiveKind::Number,
);
// "aaa" and "aab" are adjacent lexicographically, so this exercises the exact
// boundary the metric-name seek relies on: each metric's own message_match_regex
// gates it to only its own line, so if the seek pulled in the wrong metric's bucket
// range, the value asserted below would be wrong.
index
.declare_metric(metric_request_with_value("aaa", r"^aaa value=\d+$", vec![], Some("value")))
.unwrap();
index
.declare_metric(metric_request_with_value("aab", r"^aab value=\d+$", vec![], Some("value")))
.unwrap();
ingest_log(&mut index, 0, "aaa value=1");
ingest_log(&mut index, 1, "aab value=2");
let aaa_values: Vec<_> = index
.bucket_values("aaa", None)
.map(|(_, (_, _, value))| value.clone())
.collect();
assert_eq!(aaa_values, vec![MetricPrimitive::Number(Number::Int(1))]);
let aab_values: Vec<_> = index
.bucket_values("aab", None)
.map(|(_, (_, _, value))| value.clone())
.collect();
assert_eq!(aab_values, vec![MetricPrimitive::Number(Number::Int(2))]);
}
fn declare_durations_metric(index: &mut EngineSnapshot) {
let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
declare_test_param(index, "path", pattern, "path", MetricPrimitiveKind::String);
src/engine/timestamp.rs
+5
View File
@@ -5,6 +5,11 @@ use std::time::{SystemTime, UNIX_EPOCH};
pub struct Timestamp(i64);
impl Timestamp {
/// The smallest representable timestamp — used as a synthetic sub-key when seeking
/// `IndexKey::IngestedLog { sequence, timestamp }` by `sequence` alone, since it sorts
/// below any real timestamp recorded for that sequence.
pub const MIN: Timestamp = Timestamp(i64::MIN);
pub fn now() -> Self {
let nanos = SystemTime::now()
.duration_since(UNIX_EPOCH)
tickets/077-log-queries-scan-full-index-instead-of-seeking-by-key-range.md
-72
View File
@@ -1,72 +0,0 @@
# 077 — Log queries scan the full index instead of seeking by key range
## Summary
Every log read — including polling reads that only want logs newer
than `since_sequence` — walks the *entire* retained log range and
filters with a closure, rather than seeking the `BTreeMap` directly to
the relevant key range. This is the read-path optimization the
`left-right`/sort-helper-index work never got around to: iterators
should narrow the underlying key range as far as the caller's bounds
allow, and accept range bounds as a parameter instead of scanning from
`Start`/`MetricBucketSplit` and filtering after the fact.
## Where
- `EngineSnapshot::logs_range` (`src/engine/snapshot/mod.rs:159-168`)
always ranges `IndexKey::Start..IndexKey::LogParamSplit` — i.e. every
ingested log ever retained — with no way for a caller to pass a
narrower bound.
- `EngineSnapshot::logs_since` (`src/engine/snapshot/mod.rs:170-181`)
calls `logs_range()` and then filters by `sequence < before_sequence`
and a timestamp window in a closure — it has all the information
needed to seek instead (`before_sequence` bounds `sequence`
directly), but throws that away and scans linearly regardless of how
small the actual window is.
- The unsorted log-query path in `handle_query_inner`
(`src/engine/snapshot/query.rs:59-65`) does the same thing:
`snapshot.logs_range().filter(|(sequence, timestamp, _)| { since ...
start_ms ... end_ms ... })`. On every poll (frontend passes
`since_sequence` — see `webapp/src/lib/api.ts:263,343`), this walks
every previously-seen log line again just to throw it away in the
filter, before even getting to the new ones.
- `IndexKey::IngestedLog { sequence, timestamp }`
(`src/engine/index_key.rs:10-13`) derives `Ord` field-order-first on
`sequence`, so a lower bound of
`IndexKey::IngestedLog { sequence: since + 1, timestamp: Timestamp::MIN }`
(or equivalent) is a valid, correct seek — sequence is assigned
monotonically at ingest (`src/engine/handle.rs:118-119`), so this
isn't a heuristic, it's exact.
## Scope
- Change `logs_range` to accept a key range (e.g.
`RangeBounds<IndexKey>` or a `(Bound<u64>, Bound<u64>)` over
sequence) instead of hardcoding `Start..LogParamSplit`, so callers
can seek.
- Rework `logs_since` and the `EngineQuery::Logs` handler in
`query.rs:59-65` to build a lower bound from `since_sequence` (and,
where possible, from `start_ms`/note below) and pass it through,
instead of filtering post-hoc from the very start of the index.
- Note: because `IngestedLog` keys sort by `(sequence, timestamp)` and
not `(timestamp, sequence)`, a `start_ms`/`end_ms`-only query (no
`since_sequence`) can't be turned into an exact key-range seek from
this index alone even though ingestion is roughly time-ordered by
sequence — call this out explicitly rather than seeking incorrectly;
it may be acceptable to leave that case as a scan, or to binary-search
for the first sequence at/after `start_ms` since timestamps are
non-decreasing with sequence in practice.
- The sorted branch (`query.rs:82-128`) iterates `sort_helper_index`
directly rather than `logs_range`, so it's less affected, but still
does a per-candidate `index.get()` (`query.rs:106-107`) with no early
bound — check whether the helper index itself can be range-bounded
once `since_sequence`/`start_ms` are known.
- Add/extend a test asserting that a `since_sequence` poll on a large
log store does not touch (or does not compare against) log entries
at or before `since_sequence` — a range-based seek should make this
observably true, not just faster.
## Depends on
None. Related to ticket 060/061 (sort helper indexes) — same
underlying principle (avoid full scans by indexing/seeking).
tickets/078-metric-comparison-filter-scans-all-buckets-for-every-query.md
-56
View File
@@ -1,56 +0,0 @@
# 078 — Metric-comparison filters scan every metric's buckets on every query
## Summary
Same underlying issue as ticket 077 (iterators should narrow the key
range as far as possible, and take a range as a parameter), applied to
metrics: a metric-comparison filter (e.g. `latency_ms[value]<200` in
the filter bar) rebuilds its matching-sequence set from scratch on
every single query by scanning *all* buckets for *all* metrics, then
discarding everything that doesn't match the requested metric name —
instead of seeking directly to that metric's key range.
## Where
- `EngineSnapshot::bucket_values`
(`src/engine/snapshot/metrics.rs:295-312`) ranges
`IndexKey::MetricBucketSplit..IndexKey::End` — every bucket for every
declared metric — and filters by `name == metric_name` inside the
`filter_map` closure. `IndexKey::Bucket { metric_name, labels }`
(`src/engine/index_key.rs:23-26`) derives `Ord` on `metric_name`
first, so this is seekable: a lower bound of
`IndexKey::Bucket { metric_name: metric_name.into(), labels:
BTreeMap::new() }` and an upper bound one metric name past it would
narrow the range to just that metric's buckets.
- The compare-filter compiler,
`compile_metric_comparison` (`src/engine/snapshot/filter.rs:83-104`),
calls `self.bucket_values(metric, None)` and builds a fresh
`HashSet<u64>` of matching sequences on every call — this runs once
per filter compile, and filters are compiled fresh per query
(`compile_filter_query`, called from
`src/engine/snapshot/query.rs:46`), so a dashboard or search bar
polling with a metric-comparison filter re-scans all bucket values
every poll interval. Unlike the log sort-helper indexes
(`src/engine/snapshot/sort.rs`), there is no incremental cache here
keyed by `(metric, param, op, value)`.
## Scope
- Narrow `bucket_values` to accept/derive a key range for the
requested `metric_name` instead of ranging
`MetricBucketSplit..End` and filtering by name in the closure. This
alone removes the "every other metric's buckets" part of the scan
regardless of caching.
- Separately (bigger win, more invasive — may warrant its own
follow-up ticket): consider whether metric-comparison results can be
incrementally maintained similarly to `sort_helper_indexes`
(`src/engine/snapshot/sort.rs`) rather than recomputed per query, if
profiling shows compare-filtered dashboard polling is a hot path.
Keep this ticket scoped to the range-narrowing fix; note the caching
idea here so it isn't lost, but split it out if it turns out to need
design work (cache invalidation on new bucket values, per unique
filter expression).
## Depends on
None. Same principle as ticket 077.
tickets/081-promql-dashboard-queries-rebuild-full-series-from-scratch-per-poll.md
-52
View File
@@ -1,52 +0,0 @@
# 081 — PromQL dashboard queries rebuild full label→points maps from scratch every poll
## Summary
A dashboard panel backed by a PromQL-style query re-evaluates its
entire series (all labels, all points) from scratch on every polling
interval, with no reuse of the previous evaluation's work and no
incremental narrowing by the time range the panel actually needs —
same "narrow the range, don't rescan everything" principle as the
other read-path tickets, applied to the dashboard/metrics query path
this time.
## Where
- `evaluate_promql` and its helpers
(`src/engine/snapshot/query.rs:412-548`) build label→points maps for
the query's underlying metric(s) fresh on every call — there is no
reuse across repeated calls with the same query string and an
overlapping time window, which is exactly the pattern dashboard
polling produces (`webapp/src/lib/polling.ts` re-requests the same
panel query on an interval).
- This sits on top of `bucket_values` (ticket 078), so once that's
narrowed to a metric's own key range, `evaluate_promql` still
recomputes the full aggregation/label-grouping logic over whatever
`bucket_values` returns, every single call, even for the portion of
the time range it already computed on the previous poll.
## Scope
- Start by checking whether panels pass a time range/step down to
`evaluate_promql` at all — if they do, apply the same key-range
narrowing as ticket 077/078 first (bound the underlying
`bucket_values` range by the panel's requested window) before
considering caching, since that alone may remove most of the
redundant work.
- If polling the *same* query with an advancing time window is common
(sliding dashboard windows), investigate caching prior evaluation
results keyed by `(query string, label set)` and only computing the
delta for the newly-advanced portion of the window — this is more
involved (needs a cache invalidation story tied to new bucket
ingestion, similar to `sort_helper_indexes`'
`purge_stale_sort_helper_indexes`,
`src/engine/snapshot/sort.rs`) — do not build this speculatively
without first confirming via profiling that repeated PromQL
evaluation (not `bucket_values` scanning, addressed by ticket 078)
is the actual bottleneck for dashboards with multiple panels.
## Depends on
Should land after ticket 078 (metric bucket range narrowing), since
that changes what `evaluate_promql` is scanning underneath and may
reduce the size of this problem before any caching work is designed.