Skip to content

feat: StreamingConfig hot-reload — phase 1 API + endpoint (PR E) - #10

Merged
zzylol merged 1 commit into
mainfrom
feat/streaming-config-hot-reload
Apr 15, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/streaming-config-hot-reload

Conversation

@zzylol

@zzylol zzylol commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

PR E phase 1 — gives the backend a runtime-swappable StreamingConfig so an external controller (or a test harness) can change the active aggregation plan without restarting the query engine binary.

The problem this solves

Today the backend loads StreamingConfig — the map of agg_id → AggregationConfig that defines every precomputed sketch, its window, grouping labels, and retention — once at startup from a YAML file. Any change to that plan requires restarting the binary, which drops in-flight windows, loses seconds of ingest, and trips health checks.

That restart-only model breaks the broader pipeline architecture:

  1. The DataCollector controller's whole purpose is to continuously re-plan which sketches to precompute as the query workload shifts (SLA violations, new query shapes, capability misses reported by PR #11).
  2. When the planner decides on a new plan, it needs a push target to deliver that plan to the backend.
  3. Without a hot-reload path, every plan change means a backend restart → ingest gap → operator pain. The loop between controller intelligence and backend behavior only closes on a restart, which isn't acceptable in a steady-state production setting.

This PR adds the narrowest shippable slice that makes runtime plan changes possible: the swap machinery and an HTTP endpoint the controller can POST to. Query execution and ingest routing still take a startup snapshot (documented; deferred to phase 2), but the contract between the controller and the backend is now end-to-end testable.

How this PR fits in the stack

This is one half of the feedback loop that PR #11 (PR G) also lands:

query arrives → SimpleEngine miss (no compatible aggregation)
    ↓
PR #11 fires fire-and-forget POST to controller's /api/v1/plan
    ↓
current query returns §5.2 fallback answer
    ↓
controller's replanner generates a new sketch plan
    ↓
controller POSTs the new plan to backend via THIS PR's endpoint   ← PR #10
    ↓
next query for the same shape finds a precomputed match
  • PR #11 is the signal — the query plane tells the controller what's missing.
  • This PR is the return path — the controller tells the backend "here is the new plan, start computing it."

Without this PR, the controller has nowhere to push; without PR #11, the controller has no signal to push about. They ship as a pair but are independent PRs so they can be reviewed separately.

What's new

data_model::hot_reload_config (new module)

HotReloadStreamingConfig, cloneable, built on Arc<ArcSwap<StreamingConfig>>:

  • new(StreamingConfig) / from_arc(Arc<StreamingConfig>) — two constructors so callers don't double-allocate
  • snapshot() -> Arc<StreamingConfig> — cheap, lock-free read
  • swap(StreamingConfig) -> Arc<StreamingConfig> — atomic replace, returns the old Arc so callers can diff added/removed agg_ids

Four unit tests: initial snapshot, atomic swap, clone-sharing, concurrent race (writer + reader threads never see a torn state).

GET / POST /api/v1/streaming-config

New routes next to /api/v1/precompute, /api/v1/health, /api/v1/store/metrics:

  • GET — return the currently active config as JSON for verification by tests and operators
  • POST — accept a YAML body matching the existing StreamingConfig::from_yaml_data shape, parse, validate, and atomically swap via HotReloadStreamingConfig::swap. Returns {status, agg_ids_added, agg_ids_removed, new_aggregation_count}. Logs a warning when a swap removes agg_ids that may have in-flight worker state (phase 1 limitation — see below)

Both endpoints return 503 Service Unavailable when the server was constructed without HttpServer::with_hot_reload_config(...), a clear failure mode for misconfigured deployments.

HttpServer::with_hot_reload_config(handle) builder

New opt-in builder. Without it, the endpoints return 503, so existing code paths (unit tests, legacy binaries) are unaffected by this PR. main.rs calls it to attach the production handle.

main.rs wiring

Constructs a single HotReloadStreamingConfig from the startup Arc<StreamingConfig> and passes it to HttpServer::with_hot_reload_config. Other consumers (SimpleEngine, PrecomputeEngine, SimpleMapStore, KafkaConsumer) still receive the startup Arc<StreamingConfig> and will ignore swaps until phase 2.

What's NOT hot-reloaded yet (documented on the module)

  • SimpleEngine query execution — holds a startup snapshot. A query landing after a swap still sees the old config. Phase 2 will re-snapshot on query entry (one Arc::clone per query).
  • Ingest router / OTLP receiver — routes by startup AggregationConfig clones. New metric/labels mapped to new agg_ids after a swap would not be routed until phase 2.
  • In-flight precompute worker GroupState — each GroupState caches Arc<AggregationConfig> at group creation. Existing groups complete with their original config (correct semantics, not a bug). New groups created after a swap use the new config. The only case where phase 1 is visibly incomplete is removing an agg_id mid-window — existing groups for that id continue ingesting until their tumbling window closes. The POST handler logs a warning.

Phase 1 is still useful on its own: it proves the contract end-to-end, lets the controller start pushing plans, and makes the backend observable via GET for operators. Phase 2 extends the swap to take effect immediately on the query and ingest hot paths.

Tests

  • Unit (4 in hot_reload_config.rs): snapshot / swap / clone-sharing / concurrent race
  • HTTP integration (3 in http.rs::tests):
    • test_streaming_config_hot_reload_round_trip — starts a test server with a hot-reload handle, POSTs a 2-agg YAML config, asserts the POST response, then GETs and verifies. Also asserts the externally-held handle sees the swap, confirming shared ArcSwap.
    • test_streaming_config_hot_reload_missing_handle_503 — no handle attached → both GET and POST return 503
    • test_streaming_config_hot_reload_rejects_bad_yaml — garbage YAML → 400 + error message

Validation (post-rebase onto latest main)

  • cargo check --all-targets: clean
  • cargo clippy --all-targets -- -D warnings: clean
  • cargo fmt --check: clean
  • cargo test -p query_engine_rust --lib: 487 passed
  • cargo test -p query_engine_rust --lib --features sketchlib-tests: green
  • ✅ Rebased onto main — the de-flake fix and persistence clippy drive-bys were inherited from PR #9 and auto-dropped by the rebase.

Follow-ups

  • Phase 2: re-snapshot per query in SimpleEngine, per message in the ingest router. Likely changes SimpleEngine's field from Arc<StreamingConfig> to Arc<HotReloadStreamingConfig>.
  • Controller → backend channel: once this PR merges, DataCollector's controller/src/replan.rs can call this endpoint from its replanner as a new transport alongside the existing OpAMP push to agents.
  • Graceful drain on removal: currently warns but does not block reloads that remove in-flight agg_ids. A stronger guarantee would explicitly drain affected groups before applying the swap.

🤖 Generated with Claude Code

Adds `HotReloadStreamingConfig`, a thin `arc_swap::ArcSwap` wrapper,
plus `GET/POST /api/v1/streaming-config` HTTP endpoints so an external
controller (or a test harness) can push a new `StreamingConfig` at
runtime without restarting the query engine binary.

This is **phase 1** — the narrowest useful slice. The machinery exists
end-to-end (wire format, endpoint, swap), tests pin the contract, and
the controller has a working push target. Query execution and ingest
routing do NOT yet re-snapshot per request; those are phase 2 and are
documented on `HotReloadStreamingConfig` so the boundaries are clear.

## What's new

### `asap-query-engine/src/data_model/hot_reload_config.rs`
New module with `HotReloadStreamingConfig`, cloneable, built on
`Arc<ArcSwap<StreamingConfig>>`:

  * `new(StreamingConfig)` / `from_arc(Arc<StreamingConfig>)` —
    construct from initial config (two entry points so callers don't
    have to double-allocate)
  * `snapshot() -> Arc<StreamingConfig>` — cheap, lock-free read
  * `swap(StreamingConfig) -> Arc<StreamingConfig>` — atomic replace,
    returns the old `Arc` so callers can diff added/removed agg_ids

Four unit tests pin:
  * initial snapshot reflects constructor
  * swap replaces atomically (old handle unchanged, new reads fresh)
  * clones share the underlying `ArcSwap` (so server-held handle and
    test-held handle see the same swaps)
  * concurrent reader never observes a torn state while a writer
    swaps in a loop

### `GET / POST /api/v1/streaming-config` in `http.rs`
Two new routes on the existing control-plane surface, next to
`/api/v1/precompute`, `/api/v1/health`, `/api/v1/store/metrics`:

  * **GET** — return the currently active config as JSON
    (`{status, aggregation_count, aggregation_ids,
    streaming_config}`). Used by tests and operators to verify a
    push landed.

  * **POST** — accept a YAML body matching the existing
    `StreamingConfig::from_yaml_data` shape (the same format the
    binary loads at startup), parse, validate, and atomically swap
    via `HotReloadStreamingConfig::swap`. Returns `{status,
    agg_ids_added, agg_ids_removed, new_aggregation_count}` so the
    caller can confirm the transition without a round-trip to GET.
    Logs a warning if any agg_ids were removed, since in-flight
    precompute worker groups for those ids continue with their
    construction-time config until they close naturally (phase 1
    limitation).

Both endpoints return `503 Service Unavailable` when the server was
constructed without `HttpServer::with_hot_reload_config(...)` — a
clear failure mode for misconfigured deployments.

### `HttpServer::with_hot_reload_config(handle)` builder
New opt-in builder method on `HttpServer`. Without it, the endpoints
return 503 — so existing code paths (unit tests, legacy binaries) are
unaffected by this PR. `main.rs` calls it to attach the production
handle.

### `main.rs` wiring
Constructs a single `HotReloadStreamingConfig` from the startup
`Arc<StreamingConfig>` and passes it to `HttpServer::with_hot_reload_config`.
Other consumers (SimpleEngine, PrecomputeEngine, SimpleMapStore,
KafkaConsumer) still receive the startup `Arc<StreamingConfig>` and
will ignore swaps until phase 2.

## What's NOT hot-reloaded yet (documented on the module)

  * **SimpleEngine query execution** — holds a startup snapshot,
    doesn't re-snapshot per query. A query landing after a swap
    sees the old config. Phase 2 will change this to re-snapshot on
    query entry (one `Arc::clone` per query, negligible cost).

  * **Ingest router / OTLP receiver** — routes by startup
    `AggregationConfig` clones. New metric/labels mapped to new
    agg_ids after a swap would not be routed until phase 2 wires
    the router to re-snapshot per incoming message.

  * **In-flight precompute worker `GroupState`** — each
    `GroupState` caches `Arc<AggregationConfig>` at group creation.
    Existing groups complete with their original config (correct
    semantics, not a bug). New groups created after a swap use the
    new config. This is the intended behavior for graceful add and
    for param-tuning that only matters for future windows. The
    only case where phase 1 is visibly incomplete is **removing**
    an agg_id mid-window — the existing groups for that id keep
    ingesting until their tumbling window closes. The POST handler
    logs a warning when this happens.

## Drive-by fixes

Same two clippy-on-rust-1.91 issues in persistence code that
PR #9 also fixes:
  * `persistence/cache.rs` — unnecessary `u64 as u64` cast
  * `persistence/part.rs` — test `&[snap.clone()]` → `std::slice::from_ref(&snap)`

Inherited from main (persistence PR #4); fixed here so CI's clippy
gate passes on this branch too. These will be harmless duplicates if
PR #9 lands first.

## Tests

  * **Unit tests** (4, in `hot_reload_config.rs`) — snapshot /
    swap / clone-sharing / concurrent race.

  * **HTTP integration tests** (3, in `http.rs::tests`):
    - `test_streaming_config_hot_reload_round_trip`: starts a test
      server with a hot-reload handle, POSTs a 2-agg YAML config,
      asserts the POST response (agg_ids_added, new count), then
      GETs and verifies the count + ids. Also asserts that the
      externally-held `HotReloadStreamingConfig::snapshot()` sees
      the swap, confirming the ArcSwap is shared.
    - `test_streaming_config_hot_reload_missing_handle_503`: no
      handle attached, both GET and POST return 503.
    - `test_streaming_config_hot_reload_rejects_bad_yaml`: POST
      with garbage YAML returns 400 + error message.

## Validation

  * cargo check --all-targets: clean
  * cargo clippy --all-targets -- -D warnings: clean
  * cargo fmt --check: clean
  * cargo test -p query_engine_rust --lib: 481 passed

## Follow-ups

  * **Phase 2**: re-snapshot per query in `SimpleEngine`, per message
    in the ingest router. Will likely change `SimpleEngine`'s field
    from `Arc<StreamingConfig>` to `Arc<HotReloadStreamingConfig>`
    and add a helper that snapshots at query entry.
  * **Controller → backend channel**: once PR E's endpoint is merged,
    the DataCollector controller can call it from its replanner
    (`controller/src/replan.rs`) as a new transport alongside the
    existing OpAMP push to agents. Tracked independently.
  * **Graceful drain on removal**: PR E currently warns but does not
    block config reloads that remove in-flight agg_ids. A stronger
    guarantee would explicitly drain affected groups before
    applying the swap.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/streaming-config-hot-reload branch from 84643e7 to a14a463 Compare April 15, 2026 16:57
@zzylol
zzylol merged commit 9511df5 into main Apr 15, 2026
5 of 6 checks passed
@zzylol
zzylol deleted the feat/streaming-config-hot-reload branch April 15, 2026 18:45
zzylol added a commit that referenced this pull request Apr 15, 2026
Makes `POST /api/v1/streaming-config` actually useful for query-time
behavior. Before this PR, SimpleEngine snapshotted the startup config
into `Arc<StreamingConfig>` once at construction and ignored every
subsequent swap — meaning a controller push via PR #10's endpoint
was observable only by the GET debug endpoint, not by queries.

Phase 2 threads the shared `HotReloadStreamingConfig` handle all the
way into SimpleEngine and re-snapshots per query:

  main.rs builds one HotReloadStreamingConfig
    ├─> HttpServer::with_hot_reload_config(handle.clone())
    │       ↑ POST /api/v1/streaming-config swaps via this handle
    │
    └─> SimpleEngine::new_with_hot_reload(handle.clone())
            ↑ each query calls streaming_config_snapshot() which
              load_full()s from the same underlying ArcSwap

Because `HotReloadStreamingConfig` clones share the same inner
`Arc<ArcSwap<StreamingConfig>>`, a swap on one clone is immediately
visible through the other. That's the entire mechanism.

## Changes

### `SimpleEngine`

  * Field `streaming_config: Arc<StreamingConfig>` → `streaming_config_source: HotReloadStreamingConfig`
  * New constructor `SimpleEngine::new_with_hot_reload(...)` takes the
    shared handle. `main.rs` uses this path.
  * Existing `SimpleEngine::new(...)` signature preserved for tests,
    binaries, and legacy callers. Internally wraps the provided
    `Arc<StreamingConfig>` in a FRESH `HotReloadStreamingConfig` so
    external unrelated handles don't leak in — the `legacy_new_constructor_is_independent_of_external_handle`
    test pins this behavior.
  * New accessor `streaming_config_snapshot(&self) -> Arc<StreamingConfig>`.
    Each call observes whatever was most recently pushed via PR #10.
    Doc comment explains the per-query-binding discipline internal
    callers must follow.

### 13 internal call sites in `simple_engine.rs`

Each site that reads from `streaming_config` is converted to use
either:

  * Inlined `self.streaming_config_snapshot().get_aggregation_config(...).map(|c| c.X.clone())...`
    when the access is a single-expression chain whose result copies
    out of the borrowed `AggregationConfig` (e.g. `window_size * 1000`
    or a `.clone()` on an owned field).
  * A per-helper `let streaming_config = self.streaming_config_snapshot();`
    binding when a borrowed `&AggregationConfig` outlives the initial
    expression (e.g. `create_store_query_plan`, which binds the
    config once and reads multiple fields from it). The binding
    ensures a consistent view for that helper's entire execution
    and keeps the borrowed reference valid through the local Arc's
    lifetime.

Per-query entry points do NOT bind a single top-level snapshot today.
Instead each helper re-snapshots independently. Under a concurrent
swap that races with a running query, two helpers in the same query
could see different configs — but both views are internally
consistent, and the resulting query answer remains correct under
EITHER view (old or new). Swaps are rare; the torn-state window
is microseconds. Binding a single per-query snapshot and plumbing
it through helper signatures would be a much larger refactor and
is tracked as phase 3.

### `main.rs`

Switches from `SimpleEngine::new(streaming_config.clone(), ...)` to
`SimpleEngine::new_with_hot_reload(hot_reload_config.clone(), ...)`.
The same `HotReloadStreamingConfig` handle now flows to both the HTTP
server (which handles POST) and the engine (which reads), so a POST
is observable by the next query.

## Tests

Three new unit tests in a `hot_reload_phase2_tests` module at the
end of `simple_engine.rs`:

1. `streaming_config_snapshot_starts_at_initial_config` — baseline
   sanity check that the initial config is observable through the
   new accessor.

2. `streaming_config_snapshot_observes_post_construction_swap` —
   **the core PR E phase 2 guarantee**: build the engine with a
   shared handle, swap the handle from outside, call the accessor
   again, and verify the new config is visible. Also asserts the
   previous snapshot remains internally consistent (per-query
   stability).

3. `legacy_new_constructor_is_independent_of_external_handle` —
   verifies that the backwards-compat `SimpleEngine::new` path
   snapshots into a FRESH internal `HotReloadStreamingConfig`, so
   a swap on an unrelated external handle does NOT leak into
   test/bin-only constructions.

## Validation

  * cargo check --all-targets: clean
  * cargo clippy --all-targets -- -D warnings: clean
  * cargo fmt --check: clean
  * cargo test -p query_engine_rust --lib: 495 passed (up 3 from
    main — the phase 2 unit tests)

## What closes now

Paired with PR #10 (endpoint) and PR #11 (capability-miss notify),
phase 2 closes the query-side of the control loop end-to-end:

  query hits SimpleEngine miss
    → PR #11 fires fire-and-forget POST to controller /api/v1/plan
    → §5.2 fallback serves the current query
    → controller generates new plan
    → controller POSTs to backend /api/v1/streaming-config (PR #10)
    → swap lands in the shared HotReloadStreamingConfig
    → next query re-snapshots via PR E phase 2 and finds a match

## What's still NOT hot-reloaded

  * **Ingest router / OTLP receiver** — still routes by startup
    `AggregationConfig` clones. Adding a new agg_id via POST does
    not yet cause incoming metrics to be routed into that agg_id
    on the ingest side. Tracked as phase 3.
  * **In-flight precompute worker GroupState** — unchanged from
    phase 1. Existing groups complete with construction-time config;
    new groups pick up the new config. Intended semantics.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 11, 2026
… + typed Predicate (#136)

Additive lift of all 10 A-classified legacy variants into the canonical
`intent_algebra::query_expr::QueryExpr`, plus the minimal typed
`Predicate` IR for `Filter.pred` covering the 4 used-and-cleanly-shaped
ScalarExpr variants.

## What landed

### Canonical `QueryExpr` grew from 5 → 15 variants

Pre-existing (PR #130 / canonical): `Scan`, `Window`, `Aggregate`,
`LetBinding`, `Ref`.

New (this PR): `Filter`, `Project`, `Partition`, `Distinct`, `Merge`,
`Join`, `SetOp`, `Sort`, `Limit`, `BinaryOp`.

Naming convention: single-input variants use `child:` (matches existing
canonical `Window`/`Aggregate`/`LetBinding`). Multi-input variants
follow design.md §6 shapes exactly: `Merge { children }`,
`Join { kind, pred, left, right }`, `SetOp { kind, all, left, right }`,
`BinaryOp { op, lhs, rhs, vector_match }`.

### Supporting types lifted alongside

`ColumnRef`, `PartitionKeys`, `BinaryOpKind`, `JoinKind`, `SetOpKind`,
`SortKey`, `VectorMatch`/`VectorMatchKind`/`VectorGrouping`/`GroupSide`,
`LiteralValue`, `ProjectItem`. All re-exported from
`intent_algebra::mod.rs`.

### New typed `Predicate`

```rust
pub enum Predicate {
    Column(ColumnRef),
    Literal(LiteralValue),
    BinaryOp { op: BinaryOpKind, lhs: Box<Predicate>, rhs: Box<Predicate> },
    IsNull { expr: Box<Predicate>, negated: bool },
}
```

Plus `Predicate::from_legacy_scalar(&legacy::ScalarExpr) ->
Result<Predicate, QueryExprError>`. Translates the 4 supported
variants (Column / Literal / BinaryOp / IsNull); returns
`QueryExprError::UnsupportedLegacyScalar(name)` for the 4 E-deferred
(FunctionCall / ScalarSubquery / InList / Between). Per user decision,
those stay in legacy until a real consumer demands the typed shape.

Translation handles `LiteralValue::Duration → Int(nanos)` fold (canonical
LiteralValue is deliberately narrower).

### Consumer migration deferred to subsequent batches

**Zero consumer-site redirects in this PR** — and that's the right
call. Every legacy-A-variant consumer (`query_parser/`, `physical/
{stage_split,planner,allocator}.rs`, `optimizer/engine.rs`,
`legacy_lower.rs`) simultaneously matches A-variants AND C-variants
(`Source`, `Aggregate`, `Window`, `SketchAgg`, `WindowedAgg`, `TopK`,
`HistogramQuantile`, `PromQLSubquery`) AND constructs `ScalarExpr`
predicates with E-deferred variants. Migrating them mid-pipeline would
require lifting C-variants and E-variants in the same PR — explicit
out-of-scope per the batch plan.

The consumers migrate one C-batch at a time:
- Batch 3a (PR #11): `AggFunc → AggIntent`
- Batch 3b (PR #10): `TopK` split
- Batch 3c (PR #8): `WindowedAgg` un-fusion
- Batch 3d (PR #9): `SketchAgg → Aggregate@L3`
- Batch 3e (PR #12): PromQL parser `histogram_quantile → Quantile`
- Batch 13: retire `legacy_expr.rs` + `legacy_lower.rs`

### Catch-all arms added in 6 canonical-side consumers

Files: `optimizer/cost/mod.rs`, `physical/colored_dag/{allocator,emitter}.rs`,
`sketch_algebra/lower.rs`, `warm_tier_analysis.rs`. These previously
matched canonical `QueryExpr` exhaustively over the 5 pre-existing
variants; now they need conservative handling for the 10 new ones
(Edge stage / Logical wrap / child-walk / zero-cost) with TODO
markers for the follow-up reshape batches.

### Serde tag note

Initially gave `ColumnRef`/`LiteralValue`/`PartitionKeys`/`Predicate`
internally-tagged `#[serde(tag = "kind", rename_all = "snake_case")]`
attrs, but serde rejects internally-tagged newtype variants containing
primitives (e.g. `Named(String)`). Switched to externally-tagged
`#[serde(rename_all = "snake_case")]`. Caught by the round-trip test.

## Build + test

- `cargo build --release -p controller` — clean
- `cargo build --release -p query_engine_rust` — clean
- `cargo test -p controller --lib` — **666 passed** (was 655 after
  Batch 1; +11 from new typed-Predicate + schema tests)
- `cargo test -p query_engine_rust --lib -- engines::warm_tier` — 13/13

## Diff: 8 files, +794 / -8

Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant