Repository navigation
chore(cohorts): remove warpstream-calculated-events cohort membership path - #79799
gustavohstrassburger wants to merge 2 commits into
Conversation
… path The cohort_membership_changed topic has no producer left, and the warpstream-calculated-events cluster is being decommissioned. This removes the remaining consumers that bind the app to it: - ClickHouse migration 0293 drops cohort_membership_ws_mv and kafka_cohort_membership_ws on INGESTION_MEDIUM (cloud-only, matching 0245). - Removes the _ws Kafka table/MV SQL definitions and their consumer group. - Deletes the cdp-cohort-membership Node consumer and its wiring. - Removes the cohort_membership_changed topic constant and its routing entry. Generated-By: PostHog Code Task-Id: f7c0c71a-192b-485b-9cef-879e131761bc
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
@greptile review |
|
Reviews (1): Last reviewed commit: "chore(cohorts): remove warpstream-calcul..." | Re-trigger Greptile |
…bles Extends the drop migration to cover the precalculated_events and precalculated_person_properties WarpStream Kafka engine tables and MVs, not just cohort_membership. Those two pairs are still defined against the warpstream_calculated_events named collection on master, so dropping only the cohort pair would have left the cluster still bound to ClickHouse and the named collection unsafe to remove. Renamed the migration accordingly since it no longer covers only cohort membership. Generated-By: PostHog Code Task-Id: f7c0c71a-192b-485b-9cef-879e131761bc
There was a problem hiding this comment.
Pull request overview
Removes the legacy WarpStream calculated-events cohort-membership ingestion path (dead cohort_membership_changed topic + remaining consumers) so the WarpStream cluster can be decommissioned, while leaving the Rust cohort-stream-processor shadow path untouched.
Changes:
- Adds ClickHouse migration
0293to drop the WarpStream cohort-membership Kafka table + MV onNodeRole.INGESTION_MEDIUM(cloud-only US/EU/DEV), and turns migration0245into a no-op. - Removes the WarpStream cohort-membership ClickHouse DDL helpers + consumer group constant, and deletes the Node
cdp-cohort-membershipconsumer (capability + server wiring + tests). - Removes the
cohort_membership_changedtopic constant and itsCALCULATED_EVENTSrouting entry, plus updates related docs/config comments.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| posthog/settings/kafka.py | Updates routing-override example comment away from the removed topic. |
| posthog/models/cohortmembership/sql.py | Removes WarpStream _ws Kafka table/MV SQL definitions and related imports. |
| posthog/kafka_client/topics.py | Drops KAFKA_COHORT_MEMBERSHIP_CHANGED constant. |
| posthog/kafka_client/routing.py | Removes routing entry tying cohort-membership topic to CALCULATED_EVENTS. |
| posthog/clickhouse/migrations/max_migration.txt | Bumps max migration to 0293. |
| posthog/clickhouse/migrations/0293_drop_warpstream_calculated_events_cohort_membership.py | New migration dropping cohort_membership_ws_mv and kafka_cohort_membership_ws (cloud-only). |
| posthog/clickhouse/migrations/0245_warpstream_cohort_membership_kafka_engine.py | Converts prior create-migration into a no-op with explanatory note. |
| posthog/clickhouse/kafka_engine.py | Removes CONSUMER_GROUP_COHORT_MEMBERSHIP_WS. |
| nodejs/tests/helpers/kafka.ts | Removes removed topic from test topic list/imports. |
| nodejs/src/types.ts | Removes cdpCohortMembership capability type member. |
| nodejs/src/server.ts | Removes CdpCohortMembershipConsumer import and startup wiring. |
| nodejs/src/common/config/kafka-topics.ts | Removes KAFKA_COHORT_MEMBERSHIP_CHANGED constant. |
| nodejs/src/common/config.ts | Keeps PluginServerMode.cdp-cohort-membership as a stub enum member with TODO note. |
| nodejs/src/cdp/consumers/cdp-cohort-membership.consumer.ts | Deletes the consumer implementation. |
| nodejs/src/cdp/consumers/cdp-cohort-membership.consumer.test.ts | Deletes the consumer test suite. |
| nodejs/src/cdp/_tests/fixtures.ts | Removes cohort-membership-specific test fixtures/types. |
| nodejs/src/capabilities.ts | Removes realtime cohort capability group and returns {} for cdp_cohort_membership mode. |
| .agents/skills/monitoring-ingestion-pipeline/SKILL.md | Updates WarpStream virtual-cluster topic mapping documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const CAPABILITY_GROUP_MAP: Record<string, PluginServerCapabilities> = { | ||
| cdp: CAPABILITIES_CDP, | ||
| cdp_workflows: CAPABILITIES_CDP_WORKFLOWS, | ||
| realtime_cohorts: CAPABILITIES_REALTIME_COHORTS, | ||
| feature_flags: CAPABILITIES_FEATURE_FLAGS, | ||
| } |
|
Superseded by a split, per
The split also widened the migration. This PR only dropped the Dropping this PR's other change too: it neutralized the already-applied migration |
🤖 CI reportℹ️ ClickHouse migration SQL — 2 migration(s)ClickHouse migration SQL per cloud environment
|
Problem
The
warpstream-calculated-eventscluster is being decommissioned and itscohort_membership_changedtopic is dead — its only producer was removed when the legacy precalculated realtime-cohort path was deleted. What is left are consumers, and they have to go before the cluster can be destroyed.The replacement is the Rust
cohort-stream-processor, which writes tocohort_membership_changed_shadowon the warpstream-ingestion cluster. Nothing in that path is touched here.Changes
0293dropscohort_membership_ws_mvthenkafka_cohort_membership_wsonNodeRole.INGESTION_MEDIUM, cloud-only for US/EU/DEV — the same guard and node role migration0245used to create them.0245becomes a no-op with a NOTE, following the pattern in0004_kafka.py, because the SQL builders it imported are removed._wsKafka table and MV definitions fromposthog/models/cohortmembership/sql.pyplus theCONSUMER_GROUP_COHORT_MEMBERSHIP_WSconstant.cdp-cohort-membershipNode consumer, its capability, itsserver.tswiring, and its tests.KAFKA_COHORT_MEMBERSHIP_CHANGEDfrom the Python and Node topic constants and itsCALCULATED_EVENTSrouting entry.This removes the last writer to the Postgres
cohort_membershiptable.rust/feature-flagsstill reads that table, and its realtime read path is gated off by default, so no flag behavior changes here.The
cdp-cohort-membershipPluginServerModemember is kept as a no-capability stub with a TODO, matching howcdp-precalculated-filterswas handled. An unknownPLUGIN_SERVER_MODEthrows at boot, so removing the member before charts stops setting it would crash-loop the pod.Ordering
0293runs.warpstream-calculated-eventscluster.Not removed
KafkaClusterProfile.CALCULATED_EVENTSandCLICKHOUSE_KAFKA_WARPSTREAM_CALCULATED_EVENTS_NAMED_COLLECTIONstay. The named collection is still used bykafka_precalculated_events_wsandkafka_precalculated_person_properties_ws, which also sit on this cluster. Those need their own cleanup before the cluster is actually free.The MSK-cluster
kafka_cohort_membership+cohort_membership_mvpair also stays. It reads the same dead topic, but it is on the default cluster, migration0248already drops it in cloud, and it is the local/CI definition of the table — so it does not block the decommission.How did you test this code?
No new tests; this is a removal. What was run:
ruff checkandruff format --checkon every changed Python file — clean.tsc --noEmit -p .innodejs/— the only errors are pre-existing unbuilt workspace packages (@posthog/hogvm,@posthog/cyclotron,@posthog/replay-anonymizer). None are in the files this PR touches.COHORT_MEMBERSHIP_CHANGED,cohort_membership_changed,CALCULATED_EVENTS, andcdp-cohort-membership; every remaining hit is the shadow topic,cohort-stream-processor,cohort-seeder,feature-flags, or the precalculated-events tables listed above.Not run: mypy (not installed here), the Python and Jest suites, and the migration itself against a real ClickHouse. No ClickHouse snapshot changed — the
_wstables were never inschema.py.Automatic notifications
Docs update
Updated the WarpStream virtual-cluster table in
.agents/skills/monitoring-ingestion-pipeline/SKILL.mdand a stale routing-override example inposthog/settings/kafka.py.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude via PostHog Code. No repo skills were available to invoke in this environment;
posthog/clickhouse/migrations/AGENTS.mdwas read directly instead and its conventions followed for the drop migration.Two decisions worth a reviewer's attention, both deviations from the original plan:
CALCULATED_EVENTSand the named-collection setting to be fully removable. They are not —precalculated_eventsandprecalculated_person_propertiesstill bind_wsKafka tables to this cluster, so the cluster is not free after this PR alone.cdp-cohort-membershipmode enum member to be deleted. It is stubbed instead, for the crash-loop reason above.Also note
posthog/clickhouse/migrations/AGENTS.mdasks for migration-only PRs. This one mixes a migration with consumer removal because the two are one decommission step; splitting it into a stack is reasonable if a reviewer prefers.Created with PostHog Code