Skip to content

feat(controller): Phase E — stage_split L5 (StageAllocator + ThreeStageEmitter) - #279

Merged
zzylol merged 1 commit into
mainfrom
feat/controller-phase-e-stage-split
May 6, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/controller-phase-e-stage-split

Conversation

@zzylol

@zzylol zzylol commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Lands the typed L5 framework per controller/docs/design.md §6 core::physical:
takes a Phase-C-bound SketchExpr DAG and colors each node by StageId, then
emits one structured StageConfig per stage for the DC three-stage topology
(edge → gateway → backend).

  • controller/src/stage_split/ — new module: stage_id.rs (StageId + Topology),
    colored_dag.rs (ColoredDag IR with cut-edge analysis), allocator.rs
    (StageAllocator::allocate), emitter.rs (Emitter trait +
    ThreeStageEmitter), tests.rs (module-level integration tests).
  • Coloring rules per design.md §6 batched-queries example:
    Logical(Scan/Window/Aggregate{exact}) + SketchAgg → Edge;
    SketchMerge → Gateway; SketchEstimate → Backend; LetBinding/Ref
    resolve to their bound expression's stage.
  • StageConfig carries the structural facts each downstream consumer needs
    (sketch processor names — kllprocessor, ddsketchprocessor, etc. — plus
    aggregation ids threaded edge → gateway → backend, plus OTLP exporter
    targets). Phase G+ wires the actual OpAMP push and StreamingConfig POST.
  • controller/src/planner/stage_split.rs gets an additive typed call path
    (split_typed_three_stage) gated by USE_TYPED_STAGE_SPLIT=1, mirroring
    Phase C's USE_TYPED_SKETCH_ALGEBRA pattern. Existing untyped callers run
    unchanged.
  • controller/docs/design.md adds an Implementation-status subsection in §6
    describing what Phase E lands and what's deferred to Phase G+.

Test plan

  • cargo build --release -p controller — clean
  • cargo test --release -p controller stage_split — 38 passed, 0 failed
  • cargo clippy --release -p controller --all-targets -- -D warnings
    same 186 errors as origin/main (all pre-existing; Phase E introduces
    zero new clippy warnings)
  • No docker activity; no submodule pointer drift; touched only the
    allowed files (controller/src/stage_split/, controller/src/main.rs,
    controller/src/planner/stage_split.rs, controller/docs/design.md).

Tests added (12)

stage_split::tests:

  • allocator_three_stage_basic
  • allocator_sketch_agg_under_scan_pinned_edge
  • allocator_sketch_estimate_pinned_backend
  • allocator_let_binding_color_propagates
  • allocator_ref_resolves_to_binding_stage
  • allocator_sketch_merge_lands_gateway
  • emitter_three_stage_emits_three_configs
  • emitter_edge_config_has_correct_processor_kll
  • emitter_edge_config_has_correct_processor_ddsketch
  • emitter_backend_config_routes_aggregation_id
  • emitter_unsupported_topology_errors_cleanly
  • end_to_end_quantile_workload

planner::stage_split::tests:

  • typed_three_stage_path_returns_three_configs

Plus per-module unit tests in stage_id.rs, colored_dag.rs, allocator.rs
(9 more).

Open question / doc tightening

design.md §6's L5 placement table (line ~1380) puts Aggregate{Max} on Edge in
one column and "root of q3" on Backend in another. The natural reading is that
the same Aggregate{Max} node maintains per-window state at the edge AND
re-aggregates the merged stream at the backend — which Phase E's SketchExpr
IR cannot directly express today (there's no Logical(Merge) SketchExpr
variant for the gateway hop over exact streams). Phase E colours the
Logical(Aggregate) reachable in L4 as Edge unconditionally and notes the
deferral in allocator.rs + the Implementation-status doc note. Phase G+ that
adds the explicit Logical(Merge) / SketchExpr-level exact-merge variant is
the natural place to revisit the backend re-aggregation rule.

🤖 Generated with Claude Code

…geEmitter)

Lands the typed L5 framework per design.md §6 `core::physical`: a
`SketchExpr` DAG (Phase C output) is colored by `StageId` and lowered
into one `StageConfig` per stage for the DC three-stage topology
(edge → gateway → backend).

Module layout — `controller/src/stage_split/`:
- `stage_id.rs`   — `StageId` enum + `Topology` enum
- `colored_dag.rs` — `ColoredDag` IR with cut-edge analysis
- `allocator.rs`  — `StageAllocator::allocate(expr, topology)`
- `emitter.rs`    — `Emitter` trait + `ThreeStageEmitter`
- `tests.rs`      — module-level integration tests

Coloring rules (per design.md §6 batched-queries example):
- `Logical(Scan/Window/Aggregate{exact})` + `SketchAgg` → Edge
- `SketchMerge`                                          → Gateway
- `SketchEstimate`                                       → Backend
- `LetBinding`/`Ref`                                     → bound expr's stage

`StageConfig` carries the structural facts each downstream consumer
needs (sketch processor names, aggregation ids threaded edge → gateway
→ backend, OTLP exporter targets); Phase G+ wires the actual OpAMP
push and StreamingConfig POST.

Refactor in `controller/src/planner/stage_split.rs` is purely additive:
new `split_typed_three_stage` helper + `USE_TYPED_STAGE_SPLIT` env-var
gate, mirroring Phase C's `USE_TYPED_SKETCH_ALGEBRA` pattern. Existing
untyped `split_expr_by_stage` callers run unchanged.

Verification:
- cargo build --release -p controller — clean
- cargo test --release -p controller stage_split — 38 passed, 0 failed
- cargo clippy --all-targets -D warnings — same 186 errors as main
  (all pre-existing; no new warnings introduced by Phase E)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 0f5defd into main May 6, 2026
@zzylol
zzylol deleted the feat/controller-phase-e-stage-split branch May 6, 2026 03:41
zzylol added a commit that referenced this pull request May 18, 2026
…methods (B1 downstream Phase 3) (#388)

Final phase of the gorillas3 Bucket-field retire. After:
  * PR #387 — Phase 1: removed validation + log + TSDBBucket-fallback;
    field kept in Config for mapstructure compat
  * ASAPQuery-backend PR #279 — Phase 2: controller stops emitting
    `bucket:` in its YAML

Phase 3 (this PR) removes the field entirely + all the dead
write-path machinery that referenced it:

  Deleted:
    * `Config.Bucket` field
    * `s3Sink.PutChunk` / `PutPostings` methods (zero non-test callers)
    * `s3Sink.putWithRetry` / `putOnce` (callers were PutChunk/PutPostings)
    * `s3Sink.updateIndex` / `fetchIndex` (callers were PutChunk path)
    * `s3Sink.indexCache` + `indexMu` fields + initialization
    * `chunkHints` / `indexEntry` / `indexFile` types (only the dead
      chunk write-path consumed them)
    * `chunkSink` interface methods for PutChunk / PutPostings
    * `mockSink.PutChunk` / `PutPostings` + chunks/postings test
      tracking + `TestFlushWindow_DoesNotEmitLegacyChunkOrPostings`
      (now redundant — the code path is gone)
    * stale `Bucket: "asap-gorilla"` literal in
      TestGatewayFragmentRole_FinalizesFragmentsToTSDBBlock

Build verification:
  * `go test ./...` in processor/gorillas3processor: green (19 tests,
    down from 20 — the deleted redundant negative test)
  * `bash build_asap_otel.sh --skip-patches`: clean
  * empirical confmap-strict check: a yaml carrying a stale
    `bucket: legacy-bucket` is now rejected with `invalid keys: bucket`
    (OTel confmap's `ErrorUnused` is on by default since v0.79). Post
    Phase 2 the controller no longer emits it, so this only affects
    hand-rolled operator configs — those should be cleaned up too.

Closes the gorillas3 Bucket-field retire thread.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 18, 2026
…eConfig (ASAPCollector#381 Issue #4) (#389)

Two paired fixes to close the apply-loop the smoke test surfaced
after ASAPQuery-backend PR #279 landed:

(1) No-op short-circuit in processRemoteConfig

Before writing the pushed body to disk + exiting, compare to the
current on-disk file. If they match byte-for-byte, just report
APPLIED and return — don't restart. This handles two scenarios:

  * Defense-in-depth against a controller resending the same
    config (which can happen any time the OpAMP server's
    LastRemoteConfigHash cache is empty or stale — e.g. on
    reconnect before the agent has reported APPLIED).
  * The deterministic-emit guarantee from ASAPQuery-backend #281
    means same-semantic-content → same bytes, so this check is
    accurate for the controller's emit.

(2) Advertise ReportsRemoteConfig capability

In addition to AcceptsRemoteConfig, set
`AgentCapabilities_AgentCapabilities_ReportsRemoteConfig` when
`remote_config_path` is configured. Without this bit,
`opampClient.SetRemoteConfigStatus(APPLIED)` returns
`ErrReportsRemoteConfigNotSet` (opamp-go
client/internal/clientcommon.go:21), the client lib refuses to
record the applied hash, and the server has no way to learn the
config was processed — keeping it stuck in the resend loop. Both
bits are needed for the full feedback cycle.

End-to-end smoke (post-#281 deterministic emit + this PR):
  * Agent boots from bootstrap YAML
  * Receives RemoteConfig push, bytes differ, applies + exits
  * Restarts, loads new YAML
  * Receives RemoteConfig push, bytes IDENTICAL (deterministic emit)
  * No-op short-circuit fires → SetRemoteConfigStatus(APPLIED) →
    keeps running, no more restarts
  * Pipeline emits sketches normally; smoke test Axis D registers
    them

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