fix: org-scoped ClickHouse issues#3
Open
deepshekhardas wants to merge 33 commits into
Open
Conversation
The only way to get a ClickHouse client now is through the factory. Refactored all existing code to use that and pass in an org. The runReplication and otlpExporter are the hot paths here which need special attention in reviews.
…resenter clients Co-Authored-By: Matt Aitken <matt@mattaitken.com>
Co-Authored-By: Matt Aitken <matt@mattaitken.com>
The factory should not import eventRepository.server.ts — doing so pulls
the tracePubSub singleton into any module graph that imports the factory,
which eagerly connects to Redis at module load time (see singleton.ts).
The fallback now lives in index.server.ts via getEventRepositoryForStore,
which is called from RunPresenter, SpanPresenter, and recordRunEvent.
This restores runsReplicationBenchmark.test.ts's module isolation while
still handling non-ClickHouse ("taskEvent"/"postgres") stores.
Co-Authored-By: Matt Aitken <matt@mattaitken.com>
Sort findMany() by `key` for a stable winner when multiple rows assign the
same `${orgId}:${kind}`, and log an error identifying the winning and
ignored rows instead of overwriting silently. Does not fail the load —
failing the registry would break every customer, not just the misconfigured
orgs.
Co-Authored-By: Matt Aitken <matt@mattaitken.com>
…stry.server.ts Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Remove unused schema fields (keepAliveEnabled, keepAliveIdleSocketTtl, maxOpenConnections) - Move ClickHouse lookup inside deferred error handling in logs route These fixes address review comments from PR triggerdotdev#3333.
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
2 participants
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.
Removes unused schema fields (keepAliveEnabled, keepAliveIdleSocketTtl, maxOpenConnections) from the runs-replication-create API that were being silently dropped. Also moves the ClickHouse client lookup inside the deferred error handling in the logs route so failures are properly handled by TypedAwait.
Summary by cubic
Fixes org-scoped ClickHouse routing by moving client selection into deferred error handling, and cleans up the runs replication API schema to avoid silently dropped fields.
TypedAwaitand returned correctly.keepAliveEnabled,keepAliveIdleSocketTtl,maxOpenConnections) from the runs-replication-create schema to prevent confusion and silently ignored input.Written for commit 41b3b46. Summary will update on new commits.