-
Notifications
You must be signed in to change notification settings - Fork 0
feat: ClickHouse cluster (ON CLUSTER) support #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
KeKs0r
wants to merge
5
commits into
main
Choose a base branch
from
cluster-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,053
−10
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1a5442d
feat: add ClickHouse cluster (ON CLUSTER) support
KeKs0r 4554f90
fix(cluster): correct RENAME ON CLUSTER placement; guard non-replicat…
KeKs0r 8f67e73
test(cluster): make the cluster e2e comprehensive
KeKs0r aed5496
fix(cluster): use {shard}_{replica} for the journal replica id
KeKs0r d2223ce
refactor(cluster): drop the pre-existing-journal guard (YAGNI)
KeKs0r File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "chkit": patch | ||
| "@chkit/core": patch | ||
| --- | ||
|
|
||
| Add ClickHouse cluster support. Set `clickhouse.cluster` in your config to run all generated DDL `ON CLUSTER <name>` and store the migration journal in a replicated engine — for self-managed multi-node clusters. Your table engines are passed through unchanged (declare `ReplicatedMergeTree` yourself). Leave `cluster` unset for single-node, ClickHouse Cloud, or ObsessionDB, where replication is automatic and `ON CLUSTER` is unnecessary. |
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import { describe, expect, test } from 'bun:test' | ||
| import { mkdtemp, writeFile } from 'node:fs/promises' | ||
| import { tmpdir } from 'node:os' | ||
| import { join } from 'node:path' | ||
|
|
||
| import { | ||
| CORE_ENTRY, | ||
| createJournalTableName, | ||
| createLiveExecutor, | ||
| createPrefix, | ||
| formatTestDiagnostic, | ||
| runCli, | ||
| waitForTable, | ||
| } from '../src/test/e2e-testkit.js' | ||
|
|
||
| // MULTI-SHARD cluster-mode e2e against test/cluster/2shard (2 shards x 2 replicas, | ||
| // replicas named per-shard so the journal's replica id must be unique cluster-wide). | ||
| // | ||
| // This is the topology that exposes a no-`{shard}` journal path: with bare | ||
| // `{replica}` the journal CREATE collides (REPLICA_ALREADY_EXISTS) on the second | ||
| // shard; chkit uses `{shard}_{replica}` so one consistent journal spans all shards. | ||
| // | ||
| // Outside src/, excluded from default CI. Run via `bun run test:cluster:2shard` | ||
| // after `bun run cluster:2shard:up`. Hard-fails (never skips) if unreachable. | ||
| const NODES = ( | ||
| process.env.CHKIT_CLUSTER_2S_NODES ?? | ||
| 'http://localhost:8133,http://localhost:8134,http://localhost:8135,http://localhost:8136' | ||
| ).split(',') | ||
| const CLUSTER = process.env.CHKIT_CLUSTER_2S ?? 'test_cluster_2s' | ||
| const USER = process.env.CLICKHOUSE_USER ?? 'default' | ||
| const PASSWORD = process.env.CLICKHOUSE_PASSWORD ?? 'clusterpass' | ||
| const DATABASE = process.env.CLICKHOUSE_DB ?? 'default' | ||
|
|
||
| // NODES[0] is a shard-1 node (primary endpoint); NODES[2] is a shard-2 node. | ||
| const SHARD1_ENDPOINT = NODES[0] as string | ||
| const SHARD2_ENDPOINT = NODES[2] as string | ||
|
|
||
| function executorFor(url: string) { | ||
| return createLiveExecutor({ clickhouseUrl: url, clickhouseUser: USER, clickhousePassword: PASSWORD, clickhouseDatabase: DATABASE }) | ||
| } | ||
|
|
||
| function configFor(schemaPath: string, dir: string, url: string): string { | ||
| return `export default { | ||
| schema: '${schemaPath}', | ||
| outDir: '${join(dir, 'chkit')}', | ||
| migrationsDir: '${join(dir, 'chkit', 'migrations')}', | ||
| metaDir: '${join(dir, 'chkit', 'meta')}', | ||
| clickhouse: { url: '${url}', username: '${USER}', password: '${PASSWORD}', database: '${DATABASE}', cluster: '${CLUSTER}' }, | ||
| } | ||
| ` | ||
| } | ||
|
|
||
| describe('chkit cluster mode (ON CLUSTER) — multi-shard journal', () => { | ||
| test('journal forms one consistent group across all shards', async () => { | ||
| const table = `${createPrefix('twoshard')}events` | ||
| const journalTable = createJournalTableName('twoshard') | ||
| const cliEnv = { CHKIT_JOURNAL_TABLE: journalTable } | ||
|
|
||
| const dir = await mkdtemp(join(tmpdir(), 'chkit-cluster-2s-e2e-')) | ||
| const schemaPath = join(dir, 'schema.ts') | ||
| const configS1 = join(dir, 'clickhouse.config.ts') | ||
| const configS2 = join(dir, 'clickhouse.shard2.ts') | ||
| await writeFile( | ||
| schemaPath, | ||
| `import { schema, table } from '${CORE_ENTRY}'\n\nexport default schema(table({\n database: '${DATABASE}', name: '${table}', engine: 'ReplicatedMergeTree',\n columns: [{ name: 'id', type: 'UInt64' }], primaryKey: ['id'], orderBy: ['id'],\n}))\n`, | ||
| 'utf8', | ||
| ) | ||
| await writeFile(configS1, configFor(schemaPath, dir, SHARD1_ENDPOINT), 'utf8') | ||
| await writeFile(configS2, configFor(schemaPath, dir, SHARD2_ENDPOINT), 'utf8') | ||
|
|
||
| const generate = runCli(dir, ['generate', '--config', configS1, '--name', 'init', '--json'], cliEnv) | ||
| expect(generate.exitCode, formatTestDiagnostic('generate', generate)).toBe(0) | ||
| const migrate = runCli(dir, ['migrate', '--config', configS1, '--execute', '--json'], cliEnv) | ||
| expect(migrate.exitCode, formatTestDiagnostic('migrate', migrate)).toBe(0) | ||
|
|
||
| // The journal (replicated) and the table must exist on EVERY node across BOTH | ||
| // shards — with bare `{replica}` the journal would be missing on shard-1 nodes. | ||
| const executors = NODES.map(executorFor) | ||
| for (const node of executors) { | ||
| await waitForTable(node, DATABASE, journalTable) | ||
| await waitForTable(node, DATABASE, table) | ||
| const rows = await node.query<{ engine: string }>( | ||
| `SELECT engine FROM system.tables WHERE database = '${DATABASE}' AND name = '${journalTable}'`, | ||
| ) | ||
| expect(rows[0]?.engine).toBe('ReplicatedReplacingMergeTree') | ||
| } | ||
|
|
||
| // Cross-shard: chkit run against a shard-2 endpoint sees the journal written | ||
| // through the shard-1 endpoint — nothing pending. | ||
| const planOnShard2 = runCli(dir, ['migrate', '--config', configS2, '--json'], cliEnv) | ||
| expect(planOnShard2.exitCode, formatTestDiagnostic('plan on shard2', planOnShard2)).toBe(0) | ||
| expect((JSON.parse(planOnShard2.stdout) as { pending: string[] }).pending).toEqual([]) | ||
|
|
||
| const cleanup = executors[0] as ReturnType<typeof executorFor> | ||
| await cleanup.command(`DROP TABLE IF EXISTS ${DATABASE}.${table} ON CLUSTER '${CLUSTER}' SYNC`) | ||
| await cleanup.command(`DROP TABLE IF EXISTS ${DATABASE}.\`${journalTable}\` ON CLUSTER '${CLUSTER}' SYNC`) | ||
| }, 90_000) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
clickhouse.clusteris added to a project that already has the old local_chkit_migrationstable, this upgrade path only runsALTER ... ON CLUSTERand never creates or converts the table to the new replicated engine. That leaves the journal non-replicated (or the distributed ALTER fails on replicas where the old table is absent), sostatus/migratethrough another cluster node can miss applied migrations and replay them; please detect a pre-existing non-ReplicatedReplacingMergeTreejournal and fail with migration instructions or migrate it before marking the store bootstrapped.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 4554f90. In cluster mode,
ensureTablenow checks the existing journal's engine first and fails fast with migration guidance (drop ON CLUSTER, or setCHKIT_JOURNAL_TABLE) if it isn't a replicated engine — before any ON CLUSTER ALTERs run. Added a cluster e2e case covering a pre-existing non-replicated journal.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up: we've intentionally reverted this guard (d2223ce). It only protected the single-node-journal → enable-cluster migration path, which can't occur yet — chkit has no cluster users, and new adopters create the journal as
ReplicatedReplacingMergeTreefrom the first migrate. Rather than carry a probe + error path for a scenario that doesn't exist, we'll design a proper migration if/when a real single-node → cluster upgrade comes up.