From 9322aac7e151ad38f0a2c203f44311c37060ac20 Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Mon, 7 Sep 2026 14:01:29 +0100 Subject: [PATCH] Route bare ottyl_seq/ottyl_ts Promql queries to the primary (ticket 107) Audited every EngineQuery variant for the cross-snapshot join ticket 107 anticipated: Logs/Timeline/Stats/ParamDeclarations only ever read primary data (raw text + param declarations, which never left the primary), and MetricDeclarations/BucketValues/Promql only ever read profile data -- no variant actually needs both, so no join/fan-out layer is needed after all. One real gap surfaced by that audit: a bare ottyl_seq/ottyl_ts Promql query is evaluated by iterating self.logs_range() (evaluate_promql's reserved-metric branch), but such queries were routed to the profile actor's snapshot alongside real declared metrics -- and the profile actor stores no raw log rows, so these queries silently returned empty series instead of real data. EngineHandle::ask now parses the query just far enough to check its metric name and routes ottyl_seq/ottyl_ts to the primary instead, where evaluate_promql already works correctly against real log data. Added an actor-level integration test (promql_bare_ottyl_seq_works_through_the_real_actor_pipeline) that fails without this fix -- verified by temporarily disabling the routing check and confirming the test catches it. Also deduplicated config/profile.rs's local `ask` test helper, which had already once diverged from api::EngineHandle::ask's routing logic and caused ticket 104 test failures (fixed then, but the duplication itself was the root cause) -- it now delegates to the real one. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TeyhRfMYjMditek6geDRoQ --- src/api/mod.rs | 57 +++++++++++++++--- src/config/profile.rs | 18 +----- ...uery-fan-out-across-primary-and-profile.md | 58 ------------------- 3 files changed, 52 insertions(+), 81 deletions(-) delete mode 100644 tickets/107-query-fan-out-across-primary-and-profile.md diff --git a/src/api/mod.rs b/src/api/mod.rs index c602376..79cab6d 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -248,15 +248,23 @@ fn mime_for_extension(extension: &str) -> &'static str { } impl EngineHandle { - async fn ask(&self, query: EngineQuery) -> Result { + pub(crate) async fn ask(&self, query: EngineQuery) -> Result { // Metric declarations and bucket data live on the profile actor (ticket 104), not the - // primary -- route those query variants through its reader instead. - if matches!( - query, - EngineQuery::MetricDeclarations - | EngineQuery::BucketValues { .. } - | EngineQuery::Promql { .. } - ) { + // primary -- route those query variants through its reader instead. Exception: a bare + // `ottyl_seq`/`ottyl_ts` Promql query isn't real metric data, it's derived from the + // primary's raw ingested sequence/timestamps (ticket 107) -- the profile actor's + // snapshot holds no raw log rows to compute that from, so route those to the primary. + let is_reserved_promql = matches!(&query, EngineQuery::Promql { query, .. } + if crate::engine::promql::PromqlQuery::parse(query) + .is_ok_and(|parsed| crate::engine::snapshot::query::ottyl_reserved_numeric_value_fn(&parsed.metric_name).is_some())); + if !is_reserved_promql + && matches!( + query, + EngineQuery::MetricDeclarations + | EngineQuery::BucketValues { .. } + | EngineQuery::Promql { .. } + ) + { let profile_handle = self.profile_reader.handle(); let guard = profile_handle .enter() @@ -1588,6 +1596,39 @@ mod tests { assert_eq!(points[1], json!([2000, 1.0])); } + /// Ticket 107: a bare `ottyl_seq`/`ottyl_ts` Promql query is derived from the primary's raw + /// ingested sequence/timestamps, not from any declared metric's bucket data -- it must keep + /// working through the real actor pipeline (`EngineHandle::ask` routes it to the primary, + /// not the profile actor, which stores no raw log rows to compute it from) with no metric + /// ever declared. + #[tokio::test] + async fn promql_bare_ottyl_seq_works_through_the_real_actor_pipeline() { + let (app, engine, _settings, _dir) = test_app(); + + for (sequence, text) in [(0u64, "first"), (1u64, "second"), (2u64, "third")] { + engine + .tx + .send(EngineMessage::Ingest(IngestedLine { + sequence, + timestamp: Timestamp::from_millis(sequence as i64 * 1000), + text: text.into(), + })) + .await + .unwrap(); + } + engine.barrier().await; + + let (status, body) = get_json(&app, "/api/query?promql=ottyl_seq").await; + assert_eq!(status, StatusCode::OK); + let series = body.as_array().unwrap(); + assert_eq!(series.len(), 1); + let points = series[0]["points"].as_array().unwrap(); + assert_eq!( + *points, + vec![json!([0, 0.0]), json!([1000, 1.0]), json!([2000, 2.0])] + ); + } + #[tokio::test] async fn promql_query_rejects_malformed_query_with_400() { let (app, _engine, _settings, _dir) = test_app(); diff --git a/src/config/profile.rs b/src/config/profile.rs index 9bccdc7..23f7d5a 100644 --- a/src/config/profile.rs +++ b/src/config/profile.rs @@ -751,21 +751,9 @@ mod tests { } async fn ask(engine: &EngineHandle, query: EngineQuery) -> EngineResponse { - // Metric declarations and bucket data live on the profile actor (ticket 104), not the - // primary -- mirrors the routing in `api::EngineHandle::ask`. - let profile_query = matches!( - query, - EngineQuery::MetricDeclarations - | EngineQuery::BucketValues { .. } - | EngineQuery::Promql { .. } - ); - let read_handle = if profile_query { - engine.profile_reader.handle() - } else { - engine.reader.handle() - }; - let guard = read_handle.enter().expect("engine unavailable"); - guard.handle_query_ro(query) + // Delegates to the real routing in `api::EngineHandle::ask` rather than reimplementing + // it -- an earlier duplicate here diverged from it and silently read stale/empty data. + engine.ask(query).await.expect("engine unavailable") } #[test] diff --git a/tickets/107-query-fan-out-across-primary-and-profile.md b/tickets/107-query-fan-out-across-primary-and-profile.md deleted file mode 100644 index 9c05808..0000000 --- a/tickets/107-query-fan-out-across-primary-and-profile.md +++ /dev/null @@ -1,58 +0,0 @@ -# 107 — Fan out and merge queries that need both raw logs and profile-derived data - -## Summary - -Today, `Logs`/`Timeline` queries read raw log text *and* -profile-derived params/metrics from a single `EngineSnapshot` in one -pass — e.g. `query.rs`'s log-entry building calls `log_entry_params` -against `compiled_params` from the same snapshot the raw text came -from. Once params/logs live on the primary and metrics live on the -profile actor (per tickets 103-106), any query needing both becomes a -real join across two snapshots instead of one borrow. This is the -piece of [[0001-actor-split]]'s design explicitly called out as "a -query-fan-out/merge layer is still needed." - -## Where - -- `src/engine/snapshot/query.rs` (`handle_query_inner` and the log- - entry-building helpers around line 70-117 per prior research) — the - read path that currently assumes one `EngineSnapshot` has everything - a `Logs`/`Timeline` response needs. -- `src/engine/message.rs:12-27` (`EngineQuery`) — currently answered - entirely off the primary's `left_right` reader - (`EngineHandle::ask`/`handle_query_ro`, per - [[0001-actor-split]]'s "queries already bypass this actor" note); - after the split, some query variants need data from both actors' - read sides. -- `src/api/mod.rs` — wherever `EngineQuery` results are turned into - HTTP responses; the merge/join logic likely belongs here or in a new - helper alongside `query.rs`, not duplicated per handler. -- Dashboard/PromQL panel queries (`src/engine/promql.rs`) — confirm - whether these are profile-only reads (metrics live entirely on the - profile actor) and therefore need no fan-out, versus `Logs`/ - `Timeline` which do. - -## Scope - -- Identify every `EngineQuery` variant that currently reads both raw - log data and profile-derived data (params, metrics) from one - snapshot, and design the two-snapshot join for each — likely: read - matching log entries from the primary by sequence range, then look - up each entry's profile-derived fields (declared param values, - metric-driven fields) from the profile actor's snapshot by the same - sequence, filtering/merging results. -- Confirm ordering/consistency expectations: the profile actor can lag - the primary (per [[0001-actor-split]]'s accepted eviction/lag - tradeoff), so a joined query may find a primary-side log entry with - no corresponding profile-side data yet — decide and document the - behavior (omit those fields vs. error) rather than leaving it - implicit. -- Update PromQL/dashboard panel query handling only if it's confirmed - to need primary-side data too (e.g. `ottyl_seq`/`ottyl_ts` bare - queries per prior ticket history) — otherwise leave it reading the - profile actor alone. -- Out of scope: query result caching or performance optimization - beyond what's needed for correctness — this ticket is about - producing correct joined results, not making them fast (see - [[0005-additional-read-indexes]] for read-path performance work, - which should be sequenced after this settles).