fix(deploy): wire controller into multinode harness + replay typo + wave queries + stale bucket key - #394
Merged
Conversation
…ave queries + stale bucket key Four orthogonal fixes that together make the multinode harness validate the post-#290/#291/#292 sum-by-zone wave end-to-end on the ASAP arm. 1. run_demo.sh::backend_up() now brings up an asap-controller container on node2 alongside asap-backend, mirroring the singlenode docker-compose base.yml. Post-Phase-9 the controller and backend are two binaries in the same asap/query-backend:dev image but distinct processes; without the standalone controller the backend stays on the static DDSketch-only fallback (no Sum/Topk roles) and the wave queries silently return empty. Started after the backend so the startup pre-pop replan_all tick has a live POST target (CONTROLLER_BACKEND_ENDPOINT=http://backend:9091/api/v1/streaming-config, full URL form matching backend_client::BackendClient::new contract). Stale CONTROLLER_* env vars on the backend container removed; they were pre-Phase-9 carry-over the backend never reads. topology.env's "controller in-process" comment updated to reflect separate-container reality. 2. scripts/run_demo.sh:387 had `--endpoint`; the actual metricsql_replay.py arg is `--target`. Replay was silently failing argparse and the exception path returned 0 successes. Fixed. 3. queries-e2e.json extended with the wave's 3 marquee queries: `sum by (zone) (http_requests_total)`, `sum by (zone) (rate(http_requests_total[5m]))`, `topk(5, sum by (zone) (rate(http_requests_total[5m])))`. 4. agent yamls (mvp-multinode + mvp-singlenode) still set `bucket: asap-gorilla`, but PR #388 deleted the Bucket field from gorillas3processor/config.go (only `tsdb_bucket` remains). Result: agent crash-loop with `'gorillas3' ... '' has invalid keys: bucket`. Line removed from both yamls; comment references #388. ## Validation (sync + arm asap, ASAP arm) - replay.jsonl: 384 success lines (was 0 pre-fix); all 3 wave queries show success: `sum by (zone) (http_requests_total)` (128), `sum by (zone) (rate(http_requests_total[5m]))` (128), `topk(5, sum by (zone) (rate(http_requests_total[5m])))` (128). - Controller logs confirm typed-stage-split JSON POST succeeded for each (metric, role) pair: top_endpoint_qps/topk, request_size_bytes/quantile, http_requests_total/sum, etc. — all hitting http://backend:9091/api/v1/streaming-config with 2xx. - Agent logs: clean startup, no crash-loop, all 7 gorillas3 pipeline instances (raw_passthrough, ddsketch_path, hll_path, kll_path, countminsketch_path, countsketch_path) starting with `tsdb_bucket: asap-gorilla-tsdb` (no `bucket:` field). TSDB blocks written. - Manual probe against http://10.10.1.3:9091/api/v1/query, all returning `data_source: asap_query`: * `sum by (zone) (http_requests_total)` → z0:981128625, z1:981127610, z2:981127004, z3:981125787 * `sum by (zone) (rate(http_requests_total[5m]))` → z0:4798703.10, z1:4798695.49, z2:4798690.49, z3:4798680.18 * `topk(5, sum by (zone) (rate(http_requests_total[5m])))` → same 4 zones (only 4 exist; topk(5) returns all) * `quantile_over_time(0.99, http_requests_total_latency_ms[5m])` → empty (the multinode producer only emits http_requests_total, no _latency_ms; pre-existing harness gap, orthogonal to this PR). - Teardown clean. ## Out-of-scope follow-up Controller startup replan_all races the backend's /api/v1/streaming-config handler bind: the first POST burst races the route registration and some pushes get 404'd; subsequent OpAMP-on-connect re-fire makes the plan converge anyway. Lives in ASAPQuery-backend, not addressed here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…+ consume/flush (TOCTOU race) (#395) ConsumeMetrics (line 114) and flushWindow (line 298) each called ensureRoleState() which took p.mu, optionally created p.rawBuilder/p.fragmentEncoder/p.fragmentFinalizer, then released p.mu — and then the callers re-acquired p.mu to do their work. Between the two acquisitions another goroutine could mutate the role state. Concretely, for the default (gateway-raw) role: - ConsumeMetrics → ensureRoleState() takes p.mu, lazily creates p.rawBuilder, releases p.mu. - Timer goroutine flushWindow runs, takes p.mu, calls flushGatewayRawLocked which at line 337 sets p.rawBuilder = nil (the next sample is expected to lazily recreate it), releases. - ConsumeMetrics re-acquires p.mu and calls consumeGatewayRawLocked → ingestRawMetricLocked → p.rawBuilder.AddSample → SIGSEGV (rawBuilder is now nil). Observed agent crash in the multinode harness: panic: runtime error: invalid memory address or nil pointer dereference asap-gorilla-go.(*StreamingTSDBBlockBuilder).AddSample(0x0, ...) gorillas3processor/processor.go:233 The agent crash-loops every ~10s, which silently breaks the DDSketch flush path. PR #394's multinode validation masked this because sum-by-zone travels via raw_passthrough and is aggregated at the backend; the quantile_over_time(_latency_ms) queries fail because the DDSketch state never makes it across. Fix (Option 2): rename ensureRoleState → ensureRoleStateLocked, remove its internal lock, and have ConsumeMetrics / flushWindow / Start take p.mu first, then call ensureRoleStateLocked, then do their work — all inside the SAME critical section. The init step and the subsequent read/mutation of the role-state field are now atomic with respect to each other. Same fix protects the flushAgentLocked / flushGatewayFragmentLocked paths that also nil out p.fragmentEncoder / p.fragmentFinalizer. Tests: go test -race ./... passes (3.5s). Validation (multinode, post-fix): panic/SIGSEGV count on node0 agent-a: 0 AddSample (nil-deref site) on node0: 0 node0/node3 agent + node1 gateway uptime: 5+ minutes (no crash loop) quantile_over_time(0.99, _latency_ms[5m]) status:success, asap_query, real values quantile_over_time(0.5, _latency_ms[5m]) status:success, asap_query, real values sum by (zone) (http_requests_total) status:success, 4 zones, ~3e8 ea sum by (zone) (rate(http_requests_total)) status:success, 4 zones, ~2.3e6 ea topk(5, sum by (zone) (rate(...))) status:success, 4 zones ranked Drive-by harness fix in deploy/mvp-multinode/scripts/run_demo.sh: thanos-query --grpc-address defaulted to :10901 which collides with thanos-store-gateway (also :10901) under --network host on node2 (and :10902 collides with the store-gateway HTTP port). Pin query's gRPC listener to :10905 so all three thanos containers coexist. Without this, the 5-query validation can't run at all — asap-backend's thanos_query engine returns connection-refused. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
7 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…nt, aggregate_report (#396) Three orthogonal fixes surfaced by a clean post-#395 all-arms run: 1. queries-e2e.json: change [1m] → [5m] for the three *_over_time entries (quantile_over_time x2, sum_over_time x1). The agent window is 60s, backend tumbling window 30s, multinode soak 90s — a [1m] range fired early in the soak frequently lands between open windows and returns empty. Convention used everywhere else (smoke test, manual probes, #394 wave queries) is [5m]. Leave `quantile by (zone) (...)` alone — no range arg. 2. run_demo.sh::arm_measure: point the b0/b1 PromQL replay endpoint at VictoriaMetrics :8428, not Prometheus :9090. Diagnosis: the harness brings up `asap-victoriametrics` (not Prometheus) on node2 for b0/b1 (see backend_up, line 104–109), and the agents PRW to `http://victoriametrics:8428/api/v1/write`. The query endpoint at :9090 was unreachable — `curl http://10.10.1.3:9090` → connection refused → urllib URLError → `status: timeout` → 100% of 899 attempts failed with http_code=null. Live probe of :8428 against a fresh b0 arm returns 10000 series of http_requests_total and resolves `sum by (zone) (rate(...[5m]))` into 4 zones with real values; a 15s replay yielded 78/138 success (the 60 empties are the 3 *_latency_ms queries — that metric isn't in the b0 workload). 3. New `deploy/mvp-multinode/scripts/aggregate_report.py` to fill the missing script run_demo.sh::all referenced. Pure stdlib, handles the multinode dir shape (per-arm subdir containing replay.jsonl + edge-<node>.csv + stages-<node>.csv). Emits a summary table, per-query success/empty/error breakdown, edge bandwidth and stages aggregates. Idempotent. Failure stays non-fatal (run_demo.sh swallows it). Validation (all-arms run after fixes): Arm Total Success Empty Error asap 413 332 0 81 b0 829 472 357 0 b1 835 476 359 0 - asap wave queries (sum-by-zone, sum-by-zone-rate, topk-of-rate, sum_over_time): 59/59 each → #395 wave-query gate not regressed. - asap [5m] quantile_over_time: 48/59 (was 0/119 with [1m]) → Fix 1 verified. - b0/b1: 472/829 + 476/835 (was 0/899 + 0/899) → Fix 2 verified. The 357/359 "empty" results are the 3 *_latency_ms queries — the b0/b1 workload's producer emits only http_requests_total, so latency_ms returns [] (not an error). - MVP_REPORT.md emitted at run-dir root with full per-arm tables → Fix 3 verified. 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
1 participant
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
Four orthogonal fixes that together make the multinode harness validate the post-#290/#291/#292 sum-by-zone wave end-to-end on the ASAP arm.
Add
asap-controllercontainer todeploy/mvp-multinode/scripts/run_demo.sh::backend_up()(ASAP arm only). Post-Phase-9 the controller and backend are two binaries in the sameasap/query-backend:devimage but distinct processes; the standalone controller was missing from the multinode harness, so the backend stayed on the static DDSketch-only fallback (no Sum/Topk roles) and the wave queries silently returned empty. Started after the backend so the startup pre-popreplan_alltick has a live POST target (CONTROLLER_BACKEND_ENDPOINT=http://backend:9091/api/v1/streaming-config, full URL form matchingbackend_client::BackendClient::newcontract). StaleCONTROLLER_*env vars on the backend container removed; they were pre-Phase-9 carry-over the backend never reads.topology.env's "controller in-process" comment updated to reflect separate-container reality.Fix
metricsql_replay.pyarg name inrun_demo.sh:387: was--endpoint, the actual arg is--target. Replay was silently failing argparse and the exception path returned 0 successes.Extend
deploy/mvp-singlenode/scripts/queries-e2e.jsonwith the wave's 3 marquee queries (sum-by-zone, rate-over-Sum, topk-over-rate).Drop stale
bucket: asap-gorillafrom 2 agent yamls (multinode + singlenode). PR refactor(gorillas3): delete Bucket field + dead PutChunk/PutPostings (B1 downstream Phase 3) #388 deleted theBucketfield fromgorillas3processor/config.go; onlytsdb_bucketremains. Leaving the key caused an agent crash-loop with'gorillas3' ... '' has invalid keys: bucket. Line removed; comment references refactor(gorillas3): delete Bucket field + dead PutChunk/PutPostings (B1 downstream Phase 3) #388.Validation (
sync+arm asap)Replay JSONL: 384 success lines (was 0 pre-fix). All 3 wave queries succeed:
Controller logs confirm typed-stage-split JSON POST succeeded for each (metric, role) pair (
top_endpoint_qps/topk,request_size_bytes/quantile,http_requests_total/sum, etc.) — all hittinghttp://backend:9091/api/v1/streaming-configwith 2xx.Agent logs (
asap-agent-a): clean startup, no crash-loop, all 7 gorillas3 pipeline instances starting withtsdb_bucket: asap-gorilla-tsdb(nobucket:field). TSDB blocks written.Manual probe against
http://10.10.1.3:9091/api/v1/query, all returningdata_source: asap_query:Teardown clean:
bash deploy/mvp-multinode/scripts/run_demo.sh downreturns cleanly with all asap-* containers removed cluster-wide.Out-of-scope follow-up
Controller startup
replan_allraces the backend's/api/v1/streaming-confighandler bind: the first POST burst races the route registration and some pushes get 404'd; subsequent OpAMP-on-connect re-fire makes the plan converge anyway. Lives inASAPQuery-backend, not addressed here.Test plan
bash deploy/mvp-multinode/scripts/run_demo.sh syncsucceedsbash deploy/mvp-multinode/scripts/run_demo.sh arm asapfull lifecycle (stack up, controller posts to backend, agents connect via OpAMP, 90s soak, replay completes, teardown clean)replay.jsonlhas > 0 success lines, with success entries for all 3 wave queries added in Fix 3data_source: asap_queryresultsbash deploy/mvp-multinode/scripts/run_demo.sh downis cleanCo-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com