From 795818869b22105caa9774299d81cd52f8961851 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 3 Sep 2026 19:03:02 +0100 Subject: [PATCH 1/3] feat(Segment Membership): Skip traitless identities when seeding An identity carrying no traits at all can only match a segment by percentage split or `is not set`, and some environments hold millions of them. Leave them out of the ClickHouse mirror, matching the Edge CDC filter that now drops them at the stream. System traits count as traits here, so cohort members are kept. Co-Authored-By: Claude Opus 5 --- api/segment_membership/tasks.py | 9 ++++ .../test_unit_segment_membership_tasks.py | 44 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/api/segment_membership/tasks.py b/api/segment_membership/tasks.py index b8ab6709869d..3fb3d28341bf 100644 --- a/api/segment_membership/tasks.py +++ b/api/segment_membership/tasks.py @@ -61,6 +61,9 @@ def seed_organisation_identities(organisation_id: int) -> None: Rows are versioned at scan start via `inserted_at` so writes arriving mid-scan win ReplacingMergeTree dedup over the seeded row. + + Identities carrying no traits at all are skipped, keeping the mirror off the + large populations of empty identities some environments accumulate. """ log = logger.bind(organisation__id=organisation_id) if not settings.CLICKHOUSE_ENABLED: @@ -104,7 +107,13 @@ def seed_organisation_identities(organisation_id: int) -> None: scan_started_at, ) for doc in batch + # An identity with nothing on it can only match + # a segment by percentage split or `is not set`. + if doc.get("identity_traits") + or doc.get("system_traits") ] + if not rows: + continue # Django's CursorWrapper stub forbids dicts in # the params sequence; clickhouse-driver accepts # them as JSON-column payloads. diff --git a/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py b/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py index 2dbefed4c56a..e4d40d3fe1f1 100644 --- a/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py +++ b/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py @@ -151,6 +151,50 @@ def test_seed_organisation_identities__insert_fails__logs_and_continues( ] +def test_seed_organisation_identities__traitless_identities__are_not_mirrored( + mocker: MockerFixture, + settings: SettingsWrapper, + project: Project, + environment: Environment, + segment: Segment, + flagsmith_identities_table: Table, + dynamo_identities: None, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features("segment_membership_inspection") + settings.CLICKHOUSE_ENABLED = True + for identifier, extra in ( + ("dave", {}), + ("erin", {"system_traits": {"flagsmith_cohort_e2b1": True}}), + ): + flagsmith_identities_table.put_item( + Item={ + "composite_key": f"{environment.api_key}_{identifier}", + "environment_api_key": environment.api_key, + "identifier": identifier, + "identity_uuid": f"f47ac10b-58cc-4372-a567-0e02b2c3d4{identifier[:2]}", + "identity_traits": [], + **extra, + } + ) + cursor = MagicMock() + open_cursor = mocker.patch.object(tasks, "open_clickhouse_cursor") + open_cursor.return_value.__enter__.return_value = cursor + mocker.patch.object(tasks, "enqueue_membership_refresh") + + # When + seed_organisation_identities(project.organisation_id) + + # Then + # `dave` has nothing on him at all; `erin`'s cohort membership lives in + # `system_traits`, so she stays in the mirror. + mirrored_identifiers = sorted( + row[1] for call in cursor.executemany.call_args_list for row in call.args[1] + ) + assert mirrored_identifiers == ["alice", "carol", "erin"] + + @pytest.mark.clickhouse def test_seed_organisation_identities__matching_identities__inserts_rows_versioned_at_scan_start( mocker: MockerFixture, From 815cb669633b21bb03c72ab3b7089e667eea572f Mon Sep 17 00:00:00 2001 From: "flagsmith-engineering[bot]" Date: Thu, 3 Sep 2026 18:05:48 +0000 Subject: [PATCH 2/3] chore: Update documentation artefacts --- .../observability/_events-catalogue.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 6045c4df06ce..42bb898dd665 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -618,7 +618,7 @@ Attributes: ### `segment_membership.refresh.project.completed` Logged at `info` from: - - `api/segment_membership/tasks.py:266` + - `api/segment_membership/tasks.py:275` Attributes: - `membership_counts.count` @@ -628,7 +628,7 @@ Attributes: ### `segment_membership.refresh.project.failed` Logged at `exception` from: - - `api/segment_membership/tasks.py:239` + - `api/segment_membership/tasks.py:248` Attributes: - `project.id` @@ -636,8 +636,8 @@ Attributes: ### `segment_membership.refresh.project.skipped` Logged at `info` from: - - `api/segment_membership/tasks.py:206` - - `api/segment_membership/tasks.py:218` + - `api/segment_membership/tasks.py:215` + - `api/segment_membership/tasks.py:227` Attributes: - `project.id` @@ -647,7 +647,7 @@ Attributes: ### `segment_membership.seed.environment.completed` Logged at `info` from: - - `api/segment_membership/tasks.py:121` + - `api/segment_membership/tasks.py:130` Attributes: - `environment.id` @@ -658,7 +658,7 @@ Attributes: ### `segment_membership.seed.environment.failed` Logged at `exception` from: - - `api/segment_membership/tasks.py:114` + - `api/segment_membership/tasks.py:123` Attributes: - `environment.id` @@ -668,9 +668,9 @@ Attributes: ### `segment_membership.seed.skipped` Logged at `warning` from: - - `api/segment_membership/tasks.py:67` - - `api/segment_membership/tasks.py:72` - - `api/segment_membership/tasks.py:77` + - `api/segment_membership/tasks.py:70` + - `api/segment_membership/tasks.py:75` + - `api/segment_membership/tasks.py:80` Attributes: - `organisation.id` From 63bfe417b4c24157930d5289ee10dfaca99259be Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Thu, 3 Sep 2026 20:08:27 +0100 Subject: [PATCH 3/3] test(Segment Membership): Cover the all-ineligible seed batch One identity per batch, so the traitless one forms a batch with nothing left to write, and assert no INSERT is issued with an empty row list. Co-Authored-By: Claude Opus 5 --- .../test_unit_segment_membership_tasks.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py b/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py index e4d40d3fe1f1..7dacb692510b 100644 --- a/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py +++ b/api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py @@ -164,6 +164,8 @@ def test_seed_organisation_identities__traitless_identities__are_not_mirrored( # Given enable_features("segment_membership_inspection") settings.CLICKHOUSE_ENABLED = True + # One identity per batch, so `dave` forms a batch with nothing left to write. + mocker.patch.object(tasks, "_INSERT_BATCH_SIZE", 1) for identifier, extra in ( ("dave", {}), ("erin", {"system_traits": {"flagsmith_cohort_e2b1": True}}), @@ -189,10 +191,11 @@ def test_seed_organisation_identities__traitless_identities__are_not_mirrored( # Then # `dave` has nothing on him at all; `erin`'s cohort membership lives in # `system_traits`, so she stays in the mirror. - mirrored_identifiers = sorted( - row[1] for call in cursor.executemany.call_args_list for row in call.args[1] - ) + payloads = [call.args[1] for call in cursor.executemany.call_args_list] + mirrored_identifiers = sorted(row[1] for payload in payloads for row in payload) assert mirrored_identifiers == ["alice", "carol", "erin"] + # `dave`'s batch is skipped outright rather than written as an empty INSERT. + assert all(payloads) @pytest.mark.clickhouse