feat(app): end a removed tenant's streams, one Pebble for every tenant - #611
Conversation
|
@coderabbitai review |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)Create `*_test.go` files in the same package as the code under test.📄 CodeRabbit inference engine (AGENTS.md) Files:
Editors see WH001 in `.md` only.📄 CodeRabbit inference engine (AGENTS.md) Files:
🔇 Additional comments (3)
📝 SummarySummary by CodeRabbit
WalkthroughThis pull request consolidates tenant dedupe storage into one shared Pebble instance. It also changes tenant reload and stream handling, supports removal of the final nested tenant, and routes batches without a ClickHouse connection through whole-batch DLQ handling. ChangesShared dedupe storage
Tenant reload and stream lifecycle
Batches without a ClickHouse connection
Estimated code review effort: 4 (Complex) | ~55 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant App
participant Hub
participant Subscriber
participant StreamHandler
App->>Hub: Prune tenants no longer served
Hub->>Subscriber: Evict subscriber
Subscriber->>StreamHandler: Close Evicted channel
StreamHandler->>StreamHandler: Cancel replay and end stream
sequenceDiagram
participant Worker
participant ClickHouseTarget
participant DLQSwitch
participant NATS
Worker->>ClickHouseTarget: Insert batch
ClickHouseTarget-->>Worker: No connection
Worker->>DLQSwitch: Check switch once for batch
alt DLQ enabled
Worker->>NATS: Publish batch rows to DLQ
else DLQ disabled
Worker->>NATS: Leave batch unacked for redelivery
end
Suggested reviewers: Merge Risk: 🟡 Moderate · up to Existing dedupe history may not be recognized after upgrade, allowing previously accepted events to be accepted again. Resolve the migration concern before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 76.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 50 functions across 28 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
|
📚 Docs preview is live → https://88273850-wavehouse-docs.wave-rf.workers.dev |
Code Coverage OverviewLanguages: Go GoThe overall line coverage in commit 3c00e16 in the Show a line coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 1a5f56a5-330f-421b-9784-799539936873
📒 Files selected for processing (44)
AGENTS.mdCHANGELOG.mdclients/ts/src/stream/sse.tsconfig.yamldeployments/Dockerfiledeployments/Dockerfile.goreleaserdeployments/compose/standalone.yamldocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/settings-directory.mdxinternal/api/ingest_test.gointernal/api/router.gointernal/api/stream.gointernal/api/stream_test.gointernal/app/app.gointernal/app/app_test.gointernal/app/wire.gointernal/config/config.gointernal/dedupe/dedupe.gointernal/dedupe/embedded.gointernal/dedupe/embedded_test.gointernal/dedupe/managed.gointernal/dedupe/managed_test.gointernal/dedupe/stores.gointernal/dedupe/stores_test.gointernal/ingest/worker.gointernal/ingest/worker_test.gointernal/observability/metrics.gointernal/settings/registry.gointernal/settings/registry_test.gointernal/settings/settings.gointernal/settings/tree.gointernal/settings/tree_test.gointernal/stream/bucket.gointernal/stream/hub.gointernal/stream/hub_test.gointernal/stream/subscriber.gointernal/stream/subscriber_test.gointernal/tenant/tenant.gointernal/tenant/tenant_test.gointernal/testutil/mocks.go
💤 Files with no reviewable changes (4)
- internal/dedupe/dedupe.go
- internal/testutil/mocks.go
- internal/tenant/tenant_test.go
- internal/settings/tree_test.go
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (12)
- GitHub Check: Integration tests
- GitHub Check: Unit tests
- GitHub Check: Docs build
- GitHub Check: Coverage
- GitHub Check: E2E tests
- GitHub Check: Lint
- GitHub Check: Validate snapshot build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (11)
See [AGENTS.md](AGENTS.md) for project conventions, architecture notes, and AI agent instructions.
📄 CodeRabbit inference engine (CLAUDE.md)
Files:
AGENTS.md
Add the field to the appropriate struct in `internal/config/config.go` with `yaml`, `env`, and `env-default` tags.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/config/config.go
Register the route in `internal/api/router.go`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/api/router.go
Wire dependencies in `internal/app/wire.go`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/app/wire.go
Create `*_test.go` files in the same package as the code under test.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/stream/hub_test.gointernal/settings/registry_test.gointernal/stream/subscriber_test.gointernal/ingest/worker_test.gointernal/dedupe/stores_test.gointernal/dedupe/managed_test.gointernal/dedupe/embedded_test.gointernal/api/ingest_test.gointernal/app/app_test.gointernal/api/stream_test.go
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
AGENTS.md
Create the package under `internal/`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/config/config.gointernal/stream/hub_test.gointernal/stream/bucket.gointernal/settings/registry_test.gointernal/observability/metrics.gointernal/stream/subscriber_test.gointernal/api/router.gointernal/dedupe/managed.gointernal/settings/settings.gointernal/app/app.gointernal/stream/subscriber.gointernal/ingest/worker_test.gointernal/tenant/tenant.gointernal/dedupe/stores_test.gointernal/dedupe/managed_test.gointernal/dedupe/stores.gointernal/stream/hub.gointernal/dedupe/embedded_test.gointernal/api/ingest_test.gointernal/ingest/worker.gointernal/app/app_test.gointernal/settings/tree.gointernal/settings/registry.gointernal/api/stream_test.gointernal/dedupe/embedded.gointernal/api/stream.gointernal/app/wire.go
**In MDX, leave a blank line between a JSX tag and a code fence.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/configuration.mdxdocs/src/content/docs/settings-directory.mdx
Document in `docs/src/content/docs/configuration.mdx`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/configuration.mdx
Document in `docs/src/content/docs/architecture.md`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/architecture.md
Document in `docs/src/content/docs/api.md`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/api.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-09-24T19:17:33.496Z
Learning: **Go 1.26**, strict formatting (`gofumpt`, enforced by CI)
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/stream/hub_test.go
🪛 LanguageTool
docs/src/content/docs/api.md
[grammar] ~615-~615: Ensure spelling is correct
Context: ...ployment#multi-tenant-deployments)), so where tenant 0 is not served or its list do...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/src/content/docs/settings-directory.mdx
[style] ~188-~188: Since ownership is already implied, this phrasing may be redundant.
Context: ...itch is on: each tenant's store follows its own folder's dedupe.enabled the same way;...
(PRP_OWN)
CHANGELOG.md
[typographical] ~13-~13: Consider using an em dash in dialogues and enumerations.
Context: - **A tenant removed or rejected at runti...
(DASH_RULE)
[style] ~13-~13: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: - A tenant removed or rejected at runtime has its open streams ended (internal/stream/{hub,subscriber,bucket}.go (+ tests), internal/api/stream.go (+ tests), internal/api/router.go, internal/settings/{registry,tree}.go (+ tests), internal/ingest/worker.go (+ tests), internal/app/wire.go (+ tests), clients/ts/src/stream/sse.ts, docs/src/content/docs/{api,deployment,architecture,ingest-pipeline}.md, docs/src/content/docs/settings-directory.mdx, AGENTS.md): story 3 of the multi-tenant epic (#583). A GET /v1/stream used to outlive i...
(TOO_LONG_SENTENCE)
[style] ~13-~13: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ld pass, and meets its DLQ switch once, whole, logged once per batch rather than twic...
(EN_REPEATEDWORDS_WHOLE)
[style] ~13-~13: Since ownership is already implied, this phrasing may be redundant.
Context: ...on, so its queued rows are parked under its own subject rather than dropped, inserted i...
(PRP_OWN)
[style] ~14-~14: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...s completed a first discovery yet. - **One ClickHouse pool per tuple and one schema registry per tenant** (internal/chconn/chconn.go(+ tests),internal/discovery/discovery.go(+ tests),internal/app/discoveries.go(new),internal/app/{app,wire}.go(+ tests),internal/api/{schema,ingest,structured_query,pipes,query,health,errors,clickhouse_exec}.go(+ tests),internal/stream/hub.go, internal/ingest/worker.go, internal/cache/{cache,local,version_manager}.go(+ tests),internal/testutil/{testutil,mocks}.go, tests/integration/{setup,tenants,boot_resilience,query_limits}_test.go, clients/ts/src/{schema,table,sql,client,types}.ts(+ tests),tests/e2e/sdk/admin.test.ts, docs/src/content/docs/{api,deployment,architecture,ingest-pipeline}.md, docs/src/content/docs/{settings-directory,configuration,access-control,reverse-proxy}.mdx, docs/src/content/docs/sdk/{admin,reference,queries}.md, AGENTS.md): the second slice of story 6 of the multi-tenant epic ([#583`](#583)), with no behavior change for a settings directory that holds the four files beyond the three noted at the end. The process opens one native pool per d...
(TOO_LONG_SENTENCE)
[style] ~14-~14: Since ownership is already implied, this phrasing may be redundant.
Context: ...of its own over its pool, kept fresh by its own loop — the boot retry until the first s...
(PRP_OWN)
[style] ~14-~14: Since ownership is already implied, this phrasing may be redundant.
Context: ...e ingest worker inserts each batch into its own tenant's ClickHouse — the tenant the me...
(PRP_OWN)
[style] ~18-~18: Since ownership is already implied, this phrasing may be redundant.
Context: ...o restructure. A tenant's store follows its own folder's dedupe.enabled rather than t...
(PRP_OWN)
[typographical] ~79-~79: Consider using an em dash in dialogues and enumerations.
Context: - **An insert invalidates a table's cache...
(DASH_RULE)
docs/src/content/docs/deployment.md
[style] ~388-~388: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...t the parameter — and on SIGHUP — the whole directory is reloaded and mirrors its f...
(EN_REPEATEDWORDS_WHOLE)
[style] ~388-~388: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ved: delete its folder, then reload the whole directory. Its open streams end, its ro...
(EN_REPEATEDWORDS_WHOLE)
[style] ~388-~388: Since ownership is already implied, this phrasing may be redundant.
Context: ...queued rows are parked on the DLQ under its own subject; nothing it stored is deleted, ...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ... does.** A request is evaluated against its own tenant's policies.json and `pipes.jso...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...s-directory#clickhouse) — and discovers its own tables from its own database on its own...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...se) — and discovers its own tables from its own database on its own `schema.refresh_int...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: .../v1/streamconnection is authorized by its own tenant'spolicies.json` and receives i...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...n tenant's policies.json and receives its own tenant's rows alone, the ingest worker ...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...e, the ingest worker inserts a row into its own tenant's ClickHouse, a failed row is pa...
(PRP_OWN)
[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...lickHouse, a failed row is parked under its own tenant's dlq.enabled and subject (`dl...
(PRP_OWN)
[style] ~394-~394: Since ownership is already implied, this phrasing may be redundant.
Context: ... while every other tenant's routes keep their own list. Tenant 0's own dedupe store clo...
(PRP_OWN)
AGENTS.md
[style] ~43-~43: Consider using the typographical ellipsis character here instead.
Context: ...equest; settings.Store in production, Static(q...) in tests) - policy/ — Hasura-st...
(ELLIPSIS)
[style] ~47-~47: Since ownership is already implied, this phrasing may be redundant.
Context: ... table — and evaluates each event under its own tenant's policy and schema registry; `P...
(PRP_OWN)
docs/src/content/docs/architecture.md
[grammar] ~85-~85: Ensure spelling is correct
Context: ...ave-RF/WaveHouse/issues/319)). Gap-fill replay (mq.Replayer.ReplaySince on the conne...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~92-~92: This phrase is redundant. Consider writing “last”.
Context: ...nt. The SIGHUP registration is released last of all. Handler, Registry, and MQ expose...
(LAST_OF_ALL)
[style] ~92-~92: Since ownership is already implied, this phrasing may be redundant.
Context: ...ons.Listenerlets one serve the API on its own listener instead ofserver.port`. - **...
(PRP_OWN)
[style] ~99-~99: Since ownership is already implied, this phrasing may be redundant.
Context: ...ame — and each event is evaluated under its own tenant's policy (the PolicySource rea...
(PRP_OWN)
[style] ~99-~99: Since ownership is already implied, this phrasing may be redundant.
Context: ...th's: ReplayProjector tracks drift in its own state and the two are not reconciled ([...
(PRP_OWN)
[style] ~187-~187: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...t as it was. The tenant map is replaced whole by a reload, so a lookup is one lock-fr...
(EN_REPEATEDWORDS_WHOLE)
🔇 Additional comments (37)
internal/ingest/worker_test.go (1)
1937-1997: LGTM!internal/settings/registry.go (1)
34-37: LGTM!Also applies to: 189-189, 250-259
internal/settings/registry_test.go (1)
96-100: LGTM!Also applies to: 393-432, 567-567
internal/settings/tree.go (1)
86-87: LGTM!Also applies to: 107-120
internal/tenant/tenant.go (1)
30-31: LGTM!internal/api/router.go (1)
147-154: LGTM!docs/src/content/docs/api.md (1)
114-114: LGTM!Also applies to: 155-155, 615-615, 876-876
internal/stream/hub.go (1)
174-193: LGTM!internal/stream/hub_test.go (1)
1576-1602: LGTM!internal/stream/subscriber.go (1)
41-47: LGTM!Also applies to: 159-166
internal/stream/subscriber_test.go (1)
30-30: LGTM!Also applies to: 40-48
internal/api/stream_test.go (1)
181-258: LGTM!clients/ts/src/stream/sse.ts (1)
347-354: LGTM!internal/stream/bucket.go (1)
10-10: LGTM!AGENTS.md (1)
32-32: LGTM!Also applies to: 38-38, 46-48, 61-61
docs/src/content/docs/architecture.md (1)
85-85: LGTM!Also applies to: 93-93, 99-100, 126-128, 139-139, 157-157, 187-187, 193-193, 244-245
internal/dedupe/embedded.go (1)
61-107: LGTM!Also applies to: 136-147
internal/dedupe/embedded_test.go (1)
14-136: LGTM!internal/dedupe/managed.go (1)
25-27: LGTM!internal/dedupe/managed_test.go (1)
14-14: LGTM!Also applies to: 38-38, 59-59, 90-93
internal/dedupe/stores.go (1)
14-23: LGTM!Also applies to: 54-59
internal/dedupe/stores_test.go (1)
14-130: LGTM!internal/api/ingest_test.go (1)
689-697: LGTM!docs/src/content/docs/settings-directory.mdx (1)
123-127: LGTM!Also applies to: 188-188, 213-219
internal/app/app.go (1)
108-111: LGTM!internal/app/wire.go (1)
156-171: LGTM!Also applies to: 188-191, 398-400, 420-423, 438-458, 472-526, 565-565, 615-621, 766-766, 775-778, 844-844
internal/app/app_test.go (1)
215-216: LGTM!Also applies to: 469-566, 1114-1228, 1388-1468
internal/observability/metrics.go (1)
22-23: LGTM!internal/config/config.go (1)
14-15: LGTM!config.yaml (1)
5-8: LGTM!deployments/Dockerfile (1)
28-28: LGTM!deployments/Dockerfile.goreleaser (1)
7-7: LGTM!deployments/compose/standalone.yaml (1)
20-21: LGTM!docs/src/content/docs/configuration.mdx (1)
38-38: LGTM!Also applies to: 183-183
docs/src/content/docs/deployment.md (1)
172-188: LGTM!Also applies to: 321-321, 348-348, 384-394, 449-449
internal/settings/settings.go (1)
140-142: LGTM!CHANGELOG.md (1)
13-13: LGTM!Also applies to: 18-18, 79-79
|
@coderabbitai review |
✅ Action performedReview finished.
|
#711) ## Summary A `settings.dir` holding folders but none of the four files is read as nested. When every folder was rejected, or none was named by a tenant id (a fresh volume whose only entry is `lost+found`, a parent directory mounted by mistake), `settings.Open` still returned a registry and the server came up serving no tenant, answering every tenant route `unknown tenant`. Before #598 that root refused boot. Boot now refuses a nested root that would serve no tenant, with the findings plus one naming the rule, through the same path a flat invalid root takes, so the error still points at `wavehouse validate` and `wavehouse bootstrap`. The rule is boot's alone: a whole-directory reload that finds every folder gone or broken still drops every tenant and keeps running (#611), and `wavehouse validate` is unchanged. A root with one valid folder beside a rejected or stray one still boots and serves that tenant, and an empty or missing root still refuses as the four files, missing. ## Test plan - [x] `settings.Open` refuses a root whose every folder is rejected, and a root whose only folder is `lost+found`, each with the findings - [x] `settings.Open` on one valid folder beside a broken one and a stray one opens and serves the valid tenant - [x] Existing pins unchanged: an emptied directory on reload removes every tenant and keeps running; empty and missing roots refuse as "config.json: missing"; `wavehouse validate` exit codes - [x] `make ci` passes locally, integration and e2e included ## Related Issues Closes #599 Part of #583 <!-- Checklist for the author (not kept in the squash commit message): - `make ci` passes locally - Docs updated per AGENTS.md "Documentation & Consistency Sync" rules - CHANGELOG.md [Unreleased] entry added - Tests cover new / changed behavior (70 % minimum, 80 %+ preferred) The PR title is the squash commit subject — use Conventional Commits (`feat:`, `fix:`, `docs:`, `refactor:`, `test:`, `chore:`, `ci:`, `deps:`, `build:`, `perf:`, `revert:`, `style:`). The PR body below is the squash commit message, so keep it tight. -->
Summary
Story 3 of the multi-tenant epic: what removing or rejecting a tenant at runtime does. A settings directory that holds the four files sees no change beyond the dedupe store moving back to
<data_dir>/pebble.GET /v1/streamused to outlive a tenant a reload removed or rejected: the hub read no policy for it and withheld every row while the keepalive wheel held the connection open, so the client could not tell it from a quiet table. After every reload the hub now evicts the subscribers of each tenant no longer served (Hub.Prune, through a close-onceSubscriber.Evict), and the handler ends the stream. That covers a gap-fill in progress (replayContextnow also watches the eviction) and a streamTenantMWadmitted just before the reload but registered with the hub just after the prune: the handler checks right after registering (StreamHandler.Served), and since the registry swaps its map before its hooks run, one of the two always sees it. The reconnect gets404(removed) or503(rejected). The SDK needs no change: it reconnects after a server-ended stream, stops on a404, and retries a503, resuming fromLast-Event-IDonce the folder is back — in a browser going cross-origin, only where the refusal passes CORS (see Follow-ups). The stale comment insse.tsthat read a404as a missed route is fixed.Validate's flat directory missing its files), which rejected the reload whole and left that tenant served. A registry started with tenant folders now reads it as no folder left, so the reload removes the tenant like any other. Boot still reads an empty directory as the four files, missing. Reloading a deleted folder by name still leaves its tenant rejected (503), asregistry_test.gohas pinned since feat(settings): nested settings directory with a per-tenant registry #598. The flip side: a whole-directory reload that finds the directory emptied by accident drops every tenant until the folders are back and reloaded, as it already drops any one tenant whose folder is deleted.wavehouse validatestill reads an empty directory as boot does, as the four files missing, so a server restarted before a folder is written back refuses to boot./v1/healthkeeps resolving a tenant, deliberately. Its404tells a caller with no token no more than every tenant route's does, since they all answer before authenticating, which needs the tenant's verifier; exempting it would take its CORS answer from tenant0's list, which a nested directory need not have. Recorded at the route and inapi.md.dlqForreads the registry miss as on, so the rows are parked underdlq.<tenant>.<table>: never dropped, never inserted into another tenant's ClickHouse, and not left unacked, where they would hold the ack floor, stop the sweeper, and fill the one shared stream towardmq.max_bytes_gbuntil every tenant's ingest got503. A batch with no ClickHouse connection (its tenant no longer served, or no pool could be opened for it, such as by the connection ceiling) now also skips the row-by-row retry it could never pass and meets its dead-letter switch once, whole, logged once per batch rather than twice per row; a served tenant on no pool still honors itsdlq.enabled./livezdrops a gone tenant's diagnostic. Over a nested directory, a discovery error naming a tenant that stops being served before any tenant has loaded goes back tono tenant has completed a first discovery yet(a feat(clickhouse): one pool per tuple and one schema registry per tenant #610 follow-up). The schema side needed nothing new: story 6 already stops a gone tenant's refresh loop, now pinned by the removal test.data_dironce (dedupe.NewEmbedded) and asks it for each tenant's store (Embedded.Tenant, theFactory); the implementation keeps every tenant's seen ids in one instance at<data_dir>/pebble, each key<tenant>\x00<id>. Each tenant behaves as before: the instance is open only while some served tenant has dedupe on (a server with dedupe off opens nothing), a tenant switched off, rejected or removed keeps its seen ids, and the Pebble gauges are one set (Embedded.Stats;Statsleaves the per-tenantDeduplicator,ManagedandStores). Measured with Pebble v1.1.5, built like the release: an instance per tenant cost 13 goroutines, 7 open files and about 0.5 MB of heap each, so 1,000 tenants meant 13,000 goroutines, 7,000 files, about 500 MB of heap and 7.6 s of opens at boot, against 13, 7, about 4 MB and 14 ms for one instance; synced writes were 1.7x faster shared. There is nothing to migrate:moveLegacyDedupeStoreand the old<data_dir>/pebblehandling are deleted, andtenant.Parseno longer reservesnatsandpebble, which it did only because a tenant's state lived at<data_dir>/<tenant>. One consequence of sharing: an instance that cannot open fails closed every tenant with dedupe on, not one.perTenantcomment, the CHANGELOG entry on a removed-and-restored tenant's cache (settled by feat(clickhouse): one pool per tuple and one schema registry per tenant #610'sCache.InvalidateTenant),deployment.md's "What a lost tenant0costs", and the per-tenant dedupe layout across the wiring, config, compose, Dockerfiles,AGENTS.md, the CHANGELOG entry for feat(dedupe): one store per tenant under data_dir/<tenant>/dedupe #602, and the architecture, configuration, deployment and settings-directory pages.Test plan
make cipasses locallyship_it, and the docs review's last round (three wording fixes) is applied without a further roundstream:Evictis close-once;Pruneevicts every subscriber of a tenant no longer served, on each topic and under each role, and no one else'sapi: a stream ends on its own when its tenant stops being served, whether idle, mid-gap-fill, or admitted before the reload and registered after the pruneapp, over a real listener: the stop ends every stream; a reload that rejects one tenant and then removes another ends each one's stream at once and cleanly while the other's stays open, and the reconnects get503and404; a reload a flat directory rejects leaves the stream openapp, remove then restore: a rejected and then a removed tenant release pool, registry, refresh loop, verifier and dedupe store; the removed one's routes answer404, the worker gets no target and the switch on, and/livezgoes back to the no-tenant line; restored, it is served over a fresh pool, registry and verifier (the JWKS is fetched again), and an id sent before the removal is still a duplicatesettings: removing every folder removes every tenant and runs the hooks, and a folder written back is a tenant again; an emptied flat directory is still a rejected reload; an empty directory still refuses bootingest: a batch with no ClickHouse connection meets its switch once, parked under its own topic and acked, or left unacked, with no request made, while the served tenant beside it inserts its own rows alonededupe: tenants' ids never meet in the shared instance, however their keys would join; the instance opens with the first store and closes with the last, its ids kept;Statsis the instance's one set; an unopenable instance fails every tenant's store and the next apply retrieswh.from(table).stream()open on each; remove one folder and reload, watch its stream end and the SDK stop on the404; restore it and send an id it sent beforeFollow-ups
Over a nested directory serving no tenant
0, a removed tenant's404carries no CORS headers (refusals read tenant0's list, feat(cors): decide the allowlist from the request's tenant #600), so the browser SDK sees a network error and keeps re-dialing instead of stopping. Whatever turns away an unknown subdomain in front of WaveHouse should answer with CORS headers, the preflight included. If browser clients should stop on their own, one option is a final event on the evicted stream before it closes, which that connection's own CORS lets the browser read; that is a design question for Eric.sdk/streaming.mdandsdk/reference.mdsay the tenant-resolution404comes "when you sendX-Tenant-ID"; over a nested directory with no0folder a client that sends no header gets404 unknown tenant: 0too, and removing a0folder now ends such a client's stream into it. The wording predates this branch.Related Issues
Part of #583 (story 3).