feat(run-ops): ClickHouse multi-source replication fan-in + admin ops#4119
Open
d-cs wants to merge 7 commits into
Open
feat(run-ops): ClickHouse multi-source replication fan-in + admin ops#4119d-cs wants to merge 7 commits into
d-cs wants to merge 7 commits into
Conversation
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (12)
|
| Area | Changes |
|---|---|
| Replication service core | Multi-source runtime, per-source transaction and acknowledgment handling, source validation, and per-source metrics/versioning |
| Global registry | New configured-sources storage and accessors |
| Instance initialization | Split-aware boot flow, dual-source rebuild, and fatal misconfiguration handling |
| Admin endpoints | New status loader plus backfill/worker selection of the active replication service |
| Docs | New changelog entry for fan-in replication |
| Tests | Unit and integration coverage for source building, split checks, leader locks, dedup, backfill versioning, and metrics |
Sequence Diagram(s)
sequenceDiagram
participant Init as initializeRunsReplicationInstance
participant Global as runsReplicationGlobal
participant Split as isSplitEnabled
participant Service as RunsReplicationService
Init->>Service: create legacy-only service
Init->>Global: setRunsReplicationGlobal(legacy)
Init->>Split: resolve split gate
Split-->>Init: splitEnabled
Init->>Init: buildReplicationSources()
Init->>Init: assertReplicationCoversSplit()
Init->>Service: recreate with dual sources (if enabled)
Init->>Global: setRunsReplicationGlobal(dual service)
Init->>Service: start()
sequenceDiagram
participant Loader as admin.api.v1.runs-replication.status loader
participant Global as getRunsReplicationConfiguredSources
participant Redis
Loader->>Global: fetch configured sources
Loader->>Redis: check leader-lock key per source
Redis-->>Loader: exists boolean
Loader-->>Loader: build JSON response with leader flags
Related PRs: None identified.
Suggested labels: area: webapp, type: feature
Suggested reviewers: None identified.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly matches the main change: multi-source ClickHouse replication fan-in with admin operations. |
| Description check | ✅ Passed | The description covers the key What/Why/Tests points, but it omits the template's issue reference, checklist items, and screenshots section. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
runops/pr07-replication
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 @coderabbitai help to get the list of available commands.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
515b897 to
cb97148
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
5de29cb to
5c2d010
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
c59d9c5 to
d5d7fa1
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
8fb6e8a to
4e08dc7
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
a1ff262 to
a8068e9
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
4e08dc7 to
11dc0b7
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
a8068e9 to
0ef3a6b
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
11dc0b7 to
a9bc9e6
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
0db90f0 to
d5415e8
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
2 times, most recently
from
277ecea to
2bba3b8
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
aa55b6b to
3153bc4
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
2bba3b8 to
0f1da3f
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
3153bc4 to
d561590
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
0f1da3f to
5a17a98
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
d561590 to
9e7c367
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
5a17a98 to
6b7fb6d
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
9e7c367 to
e23432d
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
6b7fb6d to
ac94ac3
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
e23432d to
8dff8b2
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
ac94ac3 to
62ce160
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
8dff8b2 to
891d81a
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
62ce160 to
2be7e51
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
891d81a to
5140cbc
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
2be7e51 to
b4bee3f
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
5140cbc to
f8f3096
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
b4bee3f to
3538e4e
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
f8f3096 to
4bda37a
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
3538e4e to
e497155
Compare
d-cs
force-pushed
the
runops/pr06-write-path
branch
from
4bda37a to
ea22f52
Compare
d-cs
force-pushed
the
runops/pr07-replication
branch
from
e497155 to
ef571f9
Compare
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…comments/test names Remove the internal plan-enumeration labels from runs-replication comments and test names, keeping the behavioral descriptions intact. Comment/label hygiene only; no product logic or test behavior changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, fix test status type - Register the implicit single source with id "legacy" so its leader-lock key matches the id the admin status route probes; otherwise leadership always reads false in the non-split config. - Guard the shutdown-path client.stop() fan-out against re-firing per incoming transaction and add a catch so rejections don't surface as unhandled. - Use the TaskRunStatus type alias (not the const value) for status annotations in the dual-source dedup tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion fan-in Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d-cs
force-pushed
the
runops/pr07-replication
branch
from
ef571f9 to
766138c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…fore replacing it The legacy-only instance constructed at boot opens a replication client (Redis + Redlock) eagerly; when the split gate resolves to a multi-source service it was replaced without cleanup, leaking one Redis connection per split-enabled boot. shutdown() the bootstrap instance first (idempotent, safe on the never-started instance). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+51
to
+79
| runsReplicationService = new RunsReplicationService({ | ||
| clickhouseFactory: new TestReplicationClickhouseFactory(clickhouse), | ||
| serviceName: "runs-replication", | ||
| redisOptions, | ||
| sources: [ | ||
| { | ||
| id: "legacy", | ||
| pgConnectionUrl: legacyUrl, | ||
| slotName: "tr_legacy_v1", | ||
| publicationName: "tr_legacy_v1_pub", | ||
| originGeneration: 0, | ||
| }, | ||
| { | ||
| id: "new", | ||
| pgConnectionUrl: newUrl, | ||
| slotName: "tr_new_v1", | ||
| publicationName: "tr_new_v1_pub", | ||
| originGeneration: 1, | ||
| }, | ||
| ], | ||
| maxFlushConcurrency: 1, | ||
| flushIntervalMs: 100, | ||
| flushBatchSize: 1, | ||
| leaderLockTimeoutMs: 5000, | ||
| leaderLockExtendIntervalMs: 1000, | ||
| ackIntervalSeconds: 5, | ||
| tracer, | ||
| logLevel: "warn", | ||
| }); |
Contributor
There was a problem hiding this comment.
🚩 Test files construct RunsReplicationService without required pgConnectionUrl/slotName/publicationName fields
The integration tests in runsReplicationService.part8.test.ts (lines 51-79) and runsReplicationService.part9.test.ts construct RunsReplicationService with sources but without the required pgConnectionUrl, slotName, and publicationName fields. At runtime this works because those fields are only used as fallback when sources is empty/undefined (runsReplicationService.server.ts:261-272). However, this would fail TypeScript strict type checking. Consider making these fields optional when sources is provided (e.g., via a discriminated union type), or pass dummy values in tests to satisfy the type contract.
Was this helpful? React with 👍 or 👎 to provide feedback.
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.
What
Extends the ClickHouse runs-replication service to fan in from multiple Postgres sources (the control-plane DB and the run-ops DB) instead of a single source, plus the admin operations to run and observe it.
services/runsReplicationService.server.ts, newrunsReplicationInstance.server.ts,runsReplicationGlobal.server.ts): factors the replication service into per-source instances and a coordinator so a single ClickHouse target is fed from more than one Postgres source.routes/admin.api.v1.runs-replication.status.ts,admin.api.v1.runs-replication.backfill.ts,v3/services/adminWorker.server.ts): adds a status endpoint reporting per-source replication state and updates the backfill entrypoint for the multi-source shape.Why
PR7 of the run-ops split stack, and the final piece: once run state can live in a separate run-ops DB (earlier PRs), the analytics replication into ClickHouse has to consume both sources so runs remain queryable regardless of residency. Behavior-changing for the replication service internals; the ClickHouse-facing output is unchanged (still one runs stream), and single-source operation is preserved when the split is not enabled.
Tests
New vitest coverage:
runsReplicationInstance.test.ts(per-source instance behavior) andrunsReplicationService.part8/part9suites exercising the multi-source coordinator. Testcontainers-backed (ClickHouse + Postgres); no mocks.Notes
Draft, stacked on #4118 (
runops/pr06-write-path). Review that first; this diff is against it.Server-change / changeset note to be added at stack-assembly time.
🤖 Generated with Claude Code