Drive ClickHouse metrics DDL from .sql templates + preinit bulker metrics destination#1411
Open
absorbb wants to merge 1 commit into
Open
Drive ClickHouse metrics DDL from .sql templates + preinit bulker metrics destination#1411absorbb wants to merge 1 commit into
absorbb wants to merge 1 commit into
Conversation
… metrics destination
Console: make prisma/{events_log,metrics}.clickhouse.sql the single source of
truth for the metrics ClickHouse DDL, rendered and executed by
clickhouse-init.ts instead of hardcoded query strings.
- Rename events_log.sql / metrics.sql -> *.clickhouse.sql and templatize them
(${db}, ${onCluster}, ${engine:Family:znode}, ${user}/${password}, and a
"-- @ch-settings" directive for per-statement ClickHouse settings).
- clickhouse-init.ts renders + splits + runs the templates; the events_log DDL
now includes task_log/dead_letter (the old file had drifted and lacked them).
- Add initMetricsTables and wire it into admin/events-log-init; the integration
test harness now runs the same templates instead of an inlined DDL copy.
- Add a vitest raw-sql loader and a *.sql module type declaration.
- Drop the unused mv_active_incoming3 / to_mv_active_incoming3 objects.
Helm: render BULKER_INTERNAL_METRICS_DESTINATION into the bulker Deployment so
the metrics ClickHouse destination is preinitialized (mirrors prod). Its
connection resolves per field as: explicit values.bulker.internalMetricsDestination
> derived from env.common.CLICKHOUSE_URL (hostname, userinfo, /database, and
scheme->protocol) > in-cluster helm-deps fallback. An external CLICKHOUSE_URL
therefore moves the metrics destination with it, avoiding a split-brain where
bulker writes metrics to one ClickHouse while the console reads from another.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the ClickHouse init/template migration, integration setup updates, and Helm metrics destination wiring. I did not find clear correctness, security, or user-visible regression issues in this diff. The template rendering and statement execution order look consistent with existing behavior, and the Helm fallback/override precedence appears internally coherent.
| PARTITION BY toYYYYMM(timestamp) | ||
| ORDER BY (actorId, type, timestamp); | ||
|
|
||
| create table IF NOT EXISTS ${db}.task_log${onCluster} |
Contributor
There was a problem hiding this comment.
How about something like this
create table IF NOT EXISTS ${db}.task_log${onCluster}
(
task_id String,
sync_id LowCardinality(String),
timestamp DateTime64(3)
)
engine = ${engine:MergeTree:task_log}
PARTITION BY toYYYYMM(timestamp)
ORDER BY (task_id, sync_id, timestamp)
TTL toDateTime(timestamp) + INTERVAL 3 MONTH DELETE;
ALTER TABLE ${db}.task_log${onCluster} ADD COLUMN IF NOT EXISTS level LowCardinality(String);
ALTER TABLE ${db}.task_log${onCluster} ADD COLUMN IF NOT EXISTS logger LowCardinality(String);
ALTER TABLE ${db}.task_log${onCluster} ADD COLUMN IF NOT EXISTS message String;It establishing a pattern for further schema changes (adding columns).
| `timestamp` DateTime, | ||
| `count` AggregateFunction(uniq, String) | ||
| ) | ||
| ENGINE = ${engine:AggregatingMergeTree:mv_active_incoming2_0} |
Contributor
There was a problem hiding this comment.
How changes in the view is handled? E.g. we add a column, will the column be added to a view after deploy?
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
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
Two related pieces of ClickHouse metrics-infra cleanup.
Console — single source of truth for the metrics ClickHouse DDL
clickhouse-init.tspreviously hardcoded the events-log / metrics DDL as query strings, andprisma/events_log.sql/prisma/metrics.sqlwere separate, hand-applied copies that had drifted (the.sqlfile was missingtask_loganddead_letter, which the TS created). The metrics pipeline had no code counterpart at all — its DDL was duplicated a third time, inlined in the integration test harness.This makes the
.sqlfiles the single source, rendered and executed by the TS:prisma/{events_log,metrics}.sql→*.clickhouse.sqland templatize them with placeholders the runner fills in:${db},${onCluster},${engine:Family:znode}(→Replicated<Family>(...)on a cluster, plain<Family>()single-node),${user}/${password}, and a-- @ch-settings k=vdirective for per-statement ClickHouse settings (the TTL'sallow_suspicious_ttl_expressions, the refresh MV'sallow_experimental_refreshable_materialized_view).clickhouse-init.tsrenders → splits → runs the templates.initEventsLogTablesnow also emitstask_log/dead_letter; newinitMetricsTablesruns the metrics pipeline (ordered dependencies-first).admin/events-log-initcalls both; the integration harness now runs the same templates instead of its inlined DDL copy..sqlloader (Next already hadraw-loader) and a*.sqltype declaration.mv_active_incoming3/to_mv_active_incoming3objects (referenced nowhere).Helm — preinitialize bulker's
metricsClickHouse destinationRender
BULKER_INTERNAL_METRICS_DESTINATIONinto the bulker Deployment (mirrors the prod config), so bulker's specialmetricsdestination and its predefined tables are set up. Connection resolves per field, highest precedence first:bulker.internalMetricsDestination.*;env.common.CLICKHOUSE_URLwhen set — hostname,user:passworduserinfo,/databasepath, and scheme → protocol (https→clickhouse-secure, elseclickhouse);So pointing the stack at an external ClickHouse via
CLICKHOUSE_URLmoves the metrics destination with it — no split-brain where bulker writes metrics to one ClickHouse while the console reads from another.Testing
tsc --noEmit: clean.newjitsubefore this change (the 6 are a pre-existingdefaultauth failure on the cutoff dictionary in this environment, not a regression).helm lintclean;helm templateverified for in-cluster fallback, external http (→ nativeclickhouse), external https (→clickhouse-secure), and explicit-override-wins.🤖 Generated with Claude Code