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..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 @@ -151,6 +151,53 @@ 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 + # 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}}), + ): + 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. + 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 def test_seed_organisation_identities__matching_identities__inserts_rows_versioned_at_scan_start( mocker: MockerFixture, 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`