feat(http): wire EngineRouter into query handler — Gorilla-S3 archive routes via router (Phase 6 follow-up) - #87
Merged
Conversation
… routes via router (Phase 6 follow-up) Per-metric `StreamingConfig::storage_backend()` is now consulted on every PromQL query. Warm-tier metrics keep the direct `SimpleEngine::handle_query` path (preserving `KeyByLabelNames` for the Prometheus adapter); non-warm metrics dispatch through the Phase-5 `EngineRouter` so a `GorillaS3Archive` axis routes to the cold-archive engine and `DoubleWrite` walks the §8 fallback list. The wire response carries a `data_source: <id>` info-line on the Prometheus extension `infos` array so e2e tests / dashboards can byte-compare which engine answered. Closes the `AccuracyExact + DataSourceMarker` SKIP from PR #283. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
May 6, 2026
…_S3_* env (#88) The deployed backend Docker image uses the `precompute_engine` binary (see ASAPCollector/deploy/docker/Dockerfile.backend), but the GorillaQueryEngine env-var registration block from PR #87 only landed in `src/main.rs`. As a result, deployments that set `ASAP_GORILLA_S3_*` env vars never got a `GorillaQueryEngine` plugged into the EngineRouter, and cold-archive queries fell back silently without a `data_source: gorilla_archive` marker on the response. This mirrors the registration block from `src/main.rs` into `src/bin/precompute_engine.rs` so the deployed image actually wires the cold-archive engine when the env vars are present (and remains silent / warn-only when they're absent). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 6, 2026
The Phase-5/6 EngineRouter wiring (PR #87) sourced the per-metric StorageBackend axis from `StreamingConfig::storage_backend()` — a single field that applies to the entire streaming config. In production deploys the YAML loader (`StreamingConfig::from_yaml_data`) constructs via `Self::new(...)` and always defaults `storage_backend` to `SketchWarmTier`, so the HTTP handler always took the `SimpleEngine`-direct-dispatch branch and the EngineRouter was effectively bypassed for every query — `data_source: gorilla_archive` never landed on cold-archive responses (issue #46 v2 criterion 5 PARTIAL). Per the design correction: the streaming engine never sees Gorilla data on its OTLP-ingest path (the agent's `gorillas3processor` writes chunks directly to S3), so there is nothing for `StreamingConfig::from_yaml_data` to learn. The fix lives in a separate per-metric routing layer: - New `BackendStorageRouting` data type (`{metric_name: StorageBackend}` map) loaded once at startup from `--backend-storage-routing` YAML (or its `ASAP_BACKEND_STORAGE_ROUTING` env-var alias). Wired on both the legacy `query_engine_rust` binary and the deployed `precompute_engine` binary so the Docker image picks it up. - HTTP handler's `process_query_request` extracts the metric name from the PromQL AST (via `promql_parser`) and consults the routing table; falls back to the streaming-config single axis only when no routing table is wired (preserves pre-Phase-5 behaviour). - Three new unit tests exercise the **production code path** (routing table loaded, streaming-config default unchanged) — distinct from the existing tests that mock the dispatch by pinning `streaming_cfg.storage_backend` directly. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EngineRouter(PR feat(capability): Phase 5 — QueryEngine trait + StorageBackend routing for Gorilla-S3 #86) onto the backend HTTP query path — per-metricStreamingConfig::storage_backend()now drives dispatch, soGorillaS3Archivemetrics answer from the archive engine andDoubleWritewalks the §8 fallback list.SimpleEngine::handle_querypath (preserving theKeyByLabelNamesthe Prometheus adapter needs) — non-warm-tier metrics dispatch throughrouter.execute(...)so additional engines (Gorilla, ColdJsonl, future stand-ins) compose without further HTTP-layer changes.data_source: <id>info-line on the Prometheus extensioninfosarray — closes theAccuracyExact + DataSourceMarkerSKIP from PR feat(planner): (metric, role)-keyed WorkloadStore + PlanStore (B2 full) #283 e2e.main.rsregistersGorillaQueryEngineagainst the router wheneverASAP_GORILLA_S3_BUCKET+ASAP_GORILLA_S3_REGIONare set; otherwise the router stays single-engine and a non-warm metric trips a fail-loud503 NoEngineRegistered.Files touched
asap-query-engine/src/drivers/query/servers/http.rs— state struct + dispatch + tests.asap-query-engine/src/main.rs— env-gated GorillaQueryEngine registration.(2 files; engine internals from PRs #84/#85/#86 untouched.)
HTTP server state delta
HttpServerandAppStategain aquery_router: Arc<EngineRouter>field alongside the existingArc<SimpleEngine>.HttpServer::newbuilds a fresh router and registers the suppliedSimpleEnginefor the warm tier; a new builderwith_query_engine(Arc<dyn QueryEngine>)plugs in additional engines (called frommain.rsfor the cold-archiveGorillaQueryEngine). The range-query path keeps usingSimpleEnginedirectly until the router gains a range API.Engine registration (main.rs excerpt)
Test additions (6, all in
http.rs::tests)http_routes_warm_tier_metric_to_simple_engine—data_source: sketch_warmlands on the response.http_routes_archive_metric_to_gorilla_engine— pinsstorage_backend = GorillaS3Archive, registers a mock engine, assertsdata_source: gorilla_archive+ that the mock was hit.http_query_with_no_storage_config_defaults_to_warm_tier— back-compat: legacy YAML withoutstorage_backendstill routes to warm.http_returns_503_when_no_engines_registered—ColdJsonlFallback-pinned metric with no engine for that id surfacesNoEngineRegistered → 503.http_passes_through_accuracy_envelope— accuracy envelope anddata_sourceinfo-line both round-trip on the router path.http_router_falls_through_to_jsonl_when_archive_fails— graceful fallback (design.md §8):DoubleWritedeploy with a failing archive engine still answers via JSONL.Verification
cargo build --release -p query_engine_rustclean.cargo test --release -p query_engine_rust http— 74/74 passing (including all 6 new tests).cargo test --release -p query_engine_rust --lib— 838/872 passing; the 34 failures are pre-existing onmain(datafusion plan-execution + schema-timeline tests, unrelated to this PR).cargo clippy --release -p query_engine_rust --all-targets -- -D warnings— 5 pre-existing baseline errors insimple_engine.rs/count_sketch_accumulator.rs/otel.rs(untouched by this PR; confirmed identical onmain).Open questions
storage_backendlookup —StreamingConfigtoday exposes only a single globalstorage_backend()accessor (no per-agg_id/ per-metric variant). The HTTP layer threads that global intorouter.execute, so a deploy with mixed warm-tier and cold-archive metrics under the same backend wouldn't be served correctly — both would route to whichever axis is pinned globally. A separate small PR should add aStreamingConfig::storage_backend_for_metric(metric_name)accessor and anAggregationConfig::storage_backendfield to make this per-metric. Flagging here per the autopilot rules.(Statistic, AccuracyTarget)defaults — the router-path dispatch defaults these to(Sum, Approximate)because the HTTP layer doesn't parse PromQL; the router consults them only forDoubleWritehead-selection, which degenerates safely for the other deploy shapes. A follow-up should thread real values through once the Phase-6 query-tracker exposes them.{ status, errorType, error }matching the existingPrometheusResponse::errorshape; no consumer migration needed.Doc-tightening for design-gorilla-s3-cold-engine.md §8
None needed — the §8 routing matrix is exactly what the router walks; this PR is the wiring step the doc already anticipates.
Test plan
cargo test --release -p query_engine_rust http— all green.cargo build --release -p query_engine_rust— clean.cargo clippy --release -p query_engine_rust --all-targets— no new warnings (5 pre-existing baseline only).AccuracyExact+DataSourceMarkeronce this lands.🤖 Generated with Claude Code