Repository navigation
chore(clickhouse): drop precalculated warpstream kafka engine tables - #79807
trunk-io[bot] merged 5 commits into
Conversation
|
😎 Merged successfully - details. |
|
@greptile review |
|
Reviews (1): Last reviewed commit: "chore(cohorts): drop warpstream-calculat..." | Re-trigger Greptile |
🤖 CI report
|
a275685 to
485db5f
Compare
|
Reviews (2): Last reviewed commit: "chore(cohorts): drop warpstream-calculat..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Drops ClickHouse Kafka engine tables and their materialized views that point at the warpstream_calculated_events named collection, so the warpstream-calculated-events WarpStream cluster can be decommissioned without Kafka-engine poll error loops.
Changes:
- Add ClickHouse migration
0296to drop the two precalculated_wsKafka/MV pairs unconditionally, and the cohort-membership_wspair behind the same cloud-only guard used when creating it. - Update
max_migration.txtto point to the new migration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| posthog/clickhouse/migrations/max_migration.txt | Bumps ClickHouse migration pointer to 0296_*. |
| posthog/clickhouse/migrations/0296_drop_warpstream_calculated_events_kafka_engines.py | Drops WarpStream-calculated-events Kafka engine tables and MVs (MV-first ordering; cloud guard preserved for cohort membership). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| else [ | ||
| run_sql_with_exceptions( | ||
| "DROP TABLE IF EXISTS cohort_membership_ws_mv", | ||
| node_roles=[NodeRole.INGESTION_MEDIUM], | ||
| ), | ||
| run_sql_with_exceptions( | ||
| "DROP TABLE IF EXISTS kafka_cohort_membership_ws", | ||
| node_roles=[NodeRole.INGESTION_MEDIUM], | ||
| ), |
There was a problem hiding this comment.
Good catch — fixed in 361df20. Switched both to DROP_COHORT_MEMBERSHIP_WS_MV_SQL() / DROP_COHORT_MEMBERSHIP_WS_KAFKA_TABLE_SQL().
The emitted SQL is byte-identical, so no behaviour change, and it now mirrors migration 0245 which created these objects using the same module.
485db5f to
0df5491
Compare
|
Reviews (3): Last reviewed commit: "chore(cohorts): drop warpstream-calculat..." | Re-trigger Greptile |
|
@greptile review |
|
Reviews (4): Last reviewed commit: "chore(cohorts): use shared cohort member..." | Re-trigger Greptile |
haacked
left a comment
There was a problem hiding this comment.
Looks good. The drop set is complete and nothing on master reads these objects. One non-blocking suggestion inline.
One thing for the companion infra PR rather than this one: 0229 creates its WS Kafka tables unconditionally and hard-depends on the warpstream_calculated_events named collection, so once that collection leaves an environment's config, a fresh bootstrap replaying migrations from zero fails at 0229. That's pre-existing on master, but worth timing the collection removal around.
ab0e113 to
da2d825
Compare
|
hey @PostHog/clickhouse, can I get a stamp here please? |
|
hey @rorylshanks, since you were involved in this, do you mind giving this a look? |
Generated-By: PostHog Code Task-Id: 08dc6a1f-fda7-4722-af9e-a292f90c1f09
7e42a7a to
2f7ae8f
Compare
The migration drops kafka_precalculated_person_properties_ws and its MV, but the HCL still declared them, so the convergence gate saw the migrated cluster diverge from the golden schema. Removes both from the ingestion_medium local layer and regenerates golden/ and sql/. Only the local goldens move: the events pair is already absent off-cloud, so nothing else in the composition changes. Generated-By: PostHog Desktop Task-Id: 6664f40e-4d5f-4818-89a8-b9ebdcefc718
|
/trunk merge |
Problem
Nobody can decommission the
warpstream-calculated-eventscluster while ClickHouse still holds Kafka engine tables against it. Those tables serve no one: both precalculated topics lost their producers, so the four objects reading them consume nothing.They cannot just be left behind. A Kafka engine table whose named collection has been removed from the ClickHouse config error-loops on every poll, so the tables have to go before the collection does.
Changes
Drops the Kafka engine table and its materialized view for each of the two precalculated topics.
clickhouse_prefiltered_eventskafka_precalculated_events_wsclickhouse_precalculated_person_propertieskafka_precalculated_person_properties_wsWithin each pair the materialized view is dropped first, so it stops feeding its writable table before the Kafka table disappears.
Both drops are unconditional, because
0229created both pairs unconditionally.0243later dropped the events pair everywhere and recreated it cloud-only, since off-cloudwarpstream_calculated_eventsresolves to the same brokers asmsk_clusterand the pair double-writes. That migration leftkafka_precalculated_person_properties_wsbehind, which this drop finally removes. Every statement isDROP TABLE IF EXISTS, so the migration is idempotent either way.kafka_cohort_membership_wsreads from the same collection but stays. It is dropped separately, so its removal lands with the cohort work that owns it rather than inside a migration about precalculated tables.Data tables are untouched.
precalculated_eventsandprecalculated_person_propertiesboth stay, as do the MSK-fed Kafka tables, which resolve a different named collection and are unaffected by this cluster going away.How did you test this code?
hogli ci:preflight --stricton the push: ruff lint, ruff format, and the migration conflict check all pass.DROP_*_SQLhelpers exist onmaster, so the migration needs no model change.Not done: the migration was never run against a real ClickHouse, and no automated test exercises it.
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Claude Code (PostHog Code). Skills invoked:
/clickhouse-migrationsfor the original migration,/writing-pr-descriptionsfor this body. Also readposthog/clickhouse/migrations/AGENTS.mdfor the node-role and guard conventions.The migration started as cohort-membership-only, widened to all three topics on the same named collection, then narrowed to the two precalculated pairs so the cohort drop can ship on its own schedule.
Kept migration-only per
AGENTS.md. The matching consumer removal is a separate PR, and the now-unused SQL builders stay in place so no already-applied migration needs rewriting.Part of a cross-repo decommission, with companion PRs in
PostHog/chartsandposthog-cloud-infra. This one is safe to land first and independently.