Skip to content

Commit 247744a

Browse files
niajkitirclaude
andauthored
perf(cohorts): re-key the cohort summary MVs for the criteria that read them (#458)
* perf(cohorts): re-key the cohort summary MVs for the criteria that read them profile_event_summary_mv is ordered (project_id, profile_id, name, event_date), and the property variant similarly with property_key after name. Cohort criteria filter on name, a date range and optionally a property, then GROUP BY profile_id. They never filter profile_id, so with profile_id second the usable key prefix ends at project_id and every criterion reads the project's entire slice of the MV however narrow the criterion is. Cohorts run one such query per criterion, on a schedule. Measured on a ~100M-row summary MV: a criterion that can only prune on project_id reads 103,985,861 rows (10.9s); the same criterion with name and event_date in the key prefix reads 115,141 rows (0.11s). Reproduced from scratch on a seeded 373K-row MV: 373,500 rows read vs 28,348. A sort key cannot be altered in place, so migration 20 creates replacement MVs keyed for the consumer: event_profile_summary_mv (project_id, name, event_date, profile_id) event_property_profile_summary_mv (project_id, name, property_key, property_value, event_date, profile_id) The SELECT bodies, the identity filter and the aggregate columns are copied verbatim, so the new tables hold exactly the same rows as the old ones and only the physical order differs. Verified locally: both old and new MVs receive identical row counts from the same inserts. populate: false, with history backfilled by a companion script that is deliberately not a numbered migration so it cannot run inside the migration container. These are AggregatingMergeTree tables, so re-running a populated range double counts; the script is month-partition-aligned with an explicit --replace retry path and requires --until to avoid double counting the window the live trigger already covers. The old MVs are left in place and keep receiving inserts, so this is reversible by pointing cohort.service back at them. Dropping them is a one-line follow-up migration once the new ones are verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(cohorts): run the summary MV backfill as an idempotent migration The backfill was a hand-run script: it appended to the MVs, so it needed an exact --until to avoid double counting rows the live trigger had already written, and it refused to run without one. It also mixed topologies, resolving its target to the node-local <mv>_replicated while reading from the Distributed events. Make it migration 21 and rebuild rather than append. Each month's partition is dropped and rebuilt from events, which is idempotent by construction: an evicted migration pod is resumed by running again, and a month that was double counted heals on the next pass. That in turn removes the need to know when the MVs were created, so no operator input is required and a small deployment gets working cohorts with no flags. The bound is now read per month, straight after that month's drop. A run can take hours and the current month is dropped at the end of it, so a single bound taken at the start would delete every row the trigger wrote during the run and then decline to rebuild them. Boundary strings keep milliseconds for the same reason: created_at is DateTime64(3), and truncating the final batch to whole seconds drops the events in the boundary second after their trigger rows are gone. Reads and writes both go through the Distributed tables, so one run from one node covers every shard. DROP PARTITION is the exception, since the Distributed engine rejects partitioning, so it goes ON CLUSTER against the local table. Rebuilding a large events table unattended is still not wanted, so it steps aside above COHORT_BACKFILL_MAX_EVENTS rows and prints the manual command. The same file remains directly executable for supervised runs. Verified on standalone 26.1.3 and a keeper-backed 2-shard cluster 25.3: fresh install no-ops, history plus live-trigger overlap matches ground truth computed from events for both tables, one run writes both shards, re-runs from either node are stable, and a deliberately double-counted month returns to exact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(cohorts): drop the size guard, rebuild newest month first The COHORT_BACKFILL_MAX_EVENTS threshold and its --force escape hatch are gone. The constant was an invented number standing in for "how long will this take", which row count is a poor proxy for, and it made the automatic path conditional on a guess. Months now rebuild newest first. Cohort criteria mostly use relative timeframes, so the recent months are the ones that make cohorts correct again, and an interrupted run leaves the useful end done. Also drop the caveat about cohorts under-counting mid-rebuild needing attention: the cohortRefresh cron already recomputes every non-static cohort every 30 minutes, so it resolves on its own. Re-verified on standalone 26.1.3 and a 2-shard cluster 25.3 from empty databases: both tables match ground truth computed from events, re-runs from either node are stable, and a deliberately double-counted month returns to exact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent a62e387 commit 247744a

5 files changed

Lines changed: 531 additions & 4 deletions

File tree

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { TABLE_NAMES } from '../src/clickhouse/client';
4+
import {
5+
createMaterializedView,
6+
getExistingTables,
7+
runClickhouseMigrationCommands,
8+
} from '../src/clickhouse/migration';
9+
import { getIsCluster } from './helpers';
10+
11+
/**
12+
* Re-key the cohort summary MVs for the queries that actually read them.
13+
*
14+
* profile_event_summary_mv is ordered (project_id, profile_id, name,
15+
* event_date), and the property variant similarly with property_key after
16+
* name. Cohort criteria filter on name, a date range and optionally a
17+
* property, then GROUP BY profile_id — they never filter profile_id. With
18+
* profile_id second, the usable key prefix ends at project_id, so every
19+
* criterion reads the project's entire slice of the MV however narrow the
20+
* criterion is. Cohorts run one such query per criterion, on a schedule.
21+
*
22+
* Measured on a ~100M-row summary MV: a criterion that can only prune on
23+
* project_id reads 103,985,861 rows (10.9s); the same criterion with name
24+
* and event_date in the key prefix reads 115,141 rows (0.11s).
25+
*
26+
* A sort key cannot be altered in place, so this creates replacement MVs
27+
* keyed for the consumer:
28+
*
29+
* event_profile_summary_mv
30+
* (project_id, name, event_date, profile_id)
31+
* event_property_profile_summary_mv
32+
* (project_id, name, property_key, property_value, event_date, profile_id)
33+
*
34+
* The SELECT bodies, the identity filter and the aggregate columns are
35+
* unchanged, so the new tables hold exactly the same rows as the old ones.
36+
* Only the physical order differs.
37+
*
38+
* populate: false — these index events inserted after CREATE. History is
39+
* filled by migration 21, which rebuilds them month by month from events.
40+
*
41+
* The old MVs are left in place and keep receiving inserts. Once the new
42+
* ones are verified, dropping them is a one-line follow-up migration.
43+
*/
44+
export async function up() {
45+
const replicatedVersion = '1';
46+
const existingTables = await getExistingTables();
47+
const isClustered = getIsCluster();
48+
const sqls: string[] = [];
49+
50+
if (
51+
!existingTables.includes(
52+
`${TABLE_NAMES.event_profile_summary_mv}_distributed`,
53+
) &&
54+
!existingTables.includes(TABLE_NAMES.event_profile_summary_mv)
55+
) {
56+
sqls.push(
57+
...createMaterializedView({
58+
name: TABLE_NAMES.event_profile_summary_mv,
59+
tableName: 'events',
60+
engine: 'AggregatingMergeTree()',
61+
orderBy: ['project_id', 'name', 'event_date', 'profile_id'],
62+
partitionBy: 'toYYYYMM(event_date)',
63+
query: `SELECT
64+
project_id,
65+
profile_id,
66+
name,
67+
toStartOfDay(created_at) AS event_date,
68+
countState() AS event_count,
69+
minState(created_at) AS first_event_time,
70+
maxState(created_at) AS last_event_time,
71+
sumState(duration) AS total_duration
72+
FROM {events}
73+
WHERE profile_id != device_id
74+
GROUP BY project_id, profile_id, name, event_date`,
75+
distributionHash: 'cityHash64(project_id, profile_id)',
76+
replicatedVersion,
77+
isClustered,
78+
populate: false,
79+
}),
80+
);
81+
}
82+
83+
if (
84+
!existingTables.includes(
85+
`${TABLE_NAMES.event_property_profile_summary_mv}_distributed`,
86+
) &&
87+
!existingTables.includes(TABLE_NAMES.event_property_profile_summary_mv)
88+
) {
89+
sqls.push(
90+
...createMaterializedView({
91+
name: TABLE_NAMES.event_property_profile_summary_mv,
92+
tableName: 'events',
93+
engine: 'AggregatingMergeTree()',
94+
orderBy: [
95+
'project_id',
96+
'name',
97+
'property_key',
98+
'property_value',
99+
'event_date',
100+
'profile_id',
101+
],
102+
partitionBy: 'toYYYYMM(event_date)',
103+
query: `SELECT
104+
project_id,
105+
profile_id,
106+
name,
107+
property_key,
108+
property_value,
109+
toStartOfDay(created_at) AS event_date,
110+
countState() AS event_count,
111+
minState(created_at) AS first_event_time,
112+
maxState(created_at) AS last_event_time
113+
FROM {events}
114+
ARRAY JOIN mapKeys(properties) AS property_key, mapValues(properties) AS property_value
115+
WHERE profile_id != device_id
116+
AND property_key != ''
117+
AND property_value != ''
118+
GROUP BY project_id, profile_id, name, property_key, property_value, event_date`,
119+
distributionHash: 'cityHash64(project_id, profile_id)',
120+
replicatedVersion,
121+
isClustered,
122+
populate: false,
123+
}),
124+
);
125+
}
126+
127+
fs.writeFileSync(
128+
path.join(import.meta.filename.replace('.ts', '.sql')),
129+
sqls
130+
.map((sql) => sql.trim().replace(/;$/, '').replace(/\n{2,}/g, '\n').concat(';'))
131+
.join('\n\n---\n\n'),
132+
);
133+
134+
if (process.argv.includes('--dry')) {
135+
console.log('🔍 DRY RUN — CREATE statements:');
136+
for (const sql of sqls) {
137+
console.log(`\n${sql}\n`);
138+
}
139+
return;
140+
}
141+
142+
await runClickhouseMigrationCommands(sqls);
143+
}

0 commit comments

Comments
 (0)