refactor(gorillas3): delete Bucket field + dead PutChunk/PutPostings (B1 downstream Phase 3) - #388
Merged
Merged
Conversation
…methods (B1 downstream Phase 3) 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>
5 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…ave queries + stale bucket key (#394) 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>
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
Final phase of the three-step gorillas3
Bucket-field retire.TSDBBucket==""->Bucketfallback. Field kept inConfigfor mapstructure compat because the controller still emittedbucket:.build_gorillas3_yamlno longer emits thebucket:line.Bucketfield + all dead write-path machinery that referenced it.Deletions
Config:Bucket stringfield (was retained in Phase 1 for mapstructure compat; Phase 2 removed the only producer)s3Sink+chunkSink:PutChunk/PutPostingsmethods (zero non-test callers post Phase 1 —uploadArtifactonly callsPutTSDBBlock)putWithRetry/putOnce(only callers werePutChunk/PutPostings)updateIndex/fetchIndex(only caller wasPutChunk)indexCache+indexMufields + initialization (only consumer wasupdateIndex)chunkHints,indexEntry,indexFiletypes (only the dead chunk path consumed them)chunkSinkinterface methods forPutChunk/PutPostingsencoding/json,io,path,sync)Tests:
mockSink.PutChunk/PutPostings+chunks/postingsfields +chunkCount/postingsCountaccessors +mockChunk/mockPostingstypesTestFlushWindow_DoesNotEmitLegacyChunkOrPostings— redundant now that the code path is goneBucket: "asap-gorilla"literal inTestGatewayFragmentRole_FinalizesFragmentsToTSDBBlockNet:
3 files changed, 11 insertions(+), 324 deletions(-).Test plan
go test ./...inprocessor/gorillas3processorpasses (19 tests, down from 20 — the deleted redundant test)bash build_asap_otel.sh --skip-patchesbuilds cleanlybucket: legacy-bucketis now rejected with'' has invalid keys: bucket(OTel confmap hasErrorUnusedenabled by default since v0.79). This is the desired outcome — Phase 2 already stopped the controller from emitting it; any hand-rolled operator configs carrying the stale key should be cleaned up.bucket:validates cleanly.Closes the gorillas3
Bucket-field retire thread.Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com