Skip to content

Commit 2a13539

Browse files
authored
fix(Segment membership): Zero out (segment, env) pairs that stopped matching (#7600)
1 parent b000f47 commit 2a13539

3 files changed

Lines changed: 79 additions & 1 deletion

File tree

api/segment_membership/tasks.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,21 @@ def refresh_project_segment_counts(project_id: int) -> None:
161161
now = timezone.now()
162162
for m in membership_counts:
163163
m.last_synced_at = now
164+
165+
new_pairs = {(m.segment_id, m.environment_id) for m in membership_counts}
166+
stale_ids = [
167+
pk
168+
for pk, segment_id, environment_id in (
169+
SegmentMembershipCount.objects.filter(
170+
segment__project=project
171+
).values_list("id", "segment_id", "environment_id")
172+
)
173+
if (segment_id, environment_id) not in new_pairs
174+
]
175+
stale_deleted, _ = SegmentMembershipCount.objects.filter(
176+
id__in=stale_ids,
177+
).delete()
178+
164179
SegmentMembershipCount.objects.bulk_create(
165180
membership_counts,
166181
update_conflicts=True,
@@ -171,4 +186,5 @@ def refresh_project_segment_counts(project_id: int) -> None:
171186
"refresh.project.completed",
172187
project__id=project_id,
173188
membership_counts__count=len(membership_counts),
189+
stale_counts__count=stale_deleted,
174190
)

api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from unittest.mock import MagicMock
22

3+
from django.utils import timezone
34
from pytest_django.fixtures import SettingsWrapper
45
from pytest_mock import MockerFixture
56
from pytest_structlog import StructuredLogCapture
@@ -295,3 +296,63 @@ def test_refresh_project_segment_counts__counts_returned__upserts_per_env_rows(
295296
f":project_{project.id}"
296297
)
297298
)
299+
300+
301+
def test_refresh_project_segment_counts__previously_matching_pair_drops_to_zero__row_deleted(
302+
mocker: MockerFixture,
303+
settings: SettingsWrapper,
304+
project: Project,
305+
environment: Environment,
306+
segment: Segment,
307+
enable_features: EnableFeaturesFixture,
308+
) -> None:
309+
# Given a prior refresh that landed a non-zero count for (segment, env)
310+
enable_features("segment_membership_inspection")
311+
settings.CLICKHOUSE_ENABLED = True
312+
SegmentMembershipCount.objects.create(
313+
segment=segment,
314+
environment=environment,
315+
count=15,
316+
last_synced_at=timezone.now(),
317+
)
318+
cursor = MagicMock()
319+
open_cursor = mocker.patch.object(tasks, "open_clickhouse_cursor")
320+
open_cursor.return_value.__enter__.return_value = cursor
321+
# ... and a new compute that returns no matches for the same pair (the
322+
# rule was edited, the identity set drifted, etc.).
323+
mocker.patch.object(tasks, "compute_segment_counts_for_project", return_value=[])
324+
325+
# When
326+
refresh_project_segment_counts(project.id)
327+
328+
# Then the stale row is gone -- pairs that no longer match drop out of
329+
# the table entirely rather than lingering at the previous count.
330+
assert not SegmentMembershipCount.objects.filter(
331+
segment=segment, environment=environment
332+
).exists()
333+
334+
335+
def test_refresh_project_segment_counts__never_matched_pair__no_row_written(
336+
mocker: MockerFixture,
337+
settings: SettingsWrapper,
338+
project: Project,
339+
environment: Environment,
340+
segment: Segment,
341+
enable_features: EnableFeaturesFixture,
342+
) -> None:
343+
# Given a project with no prior membership rows
344+
enable_features("segment_membership_inspection")
345+
settings.CLICKHOUSE_ENABLED = True
346+
cursor = MagicMock()
347+
open_cursor = mocker.patch.object(tasks, "open_clickhouse_cursor")
348+
open_cursor.return_value.__enter__.return_value = cursor
349+
mocker.patch.object(tasks, "compute_segment_counts_for_project", return_value=[])
350+
351+
# When
352+
refresh_project_segment_counts(project.id)
353+
354+
# Then no row is written: refresh upserts matches, drops misses, and
355+
# leaves never-matched pairs untouched.
356+
assert not SegmentMembershipCount.objects.filter(
357+
segment=segment, environment=environment
358+
).exists()

docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,11 +368,12 @@ Attributes:
368368
### `segment_membership.refresh.project.completed`
369369

370370
Logged at `info` from:
371-
- `api/segment_membership/tasks.py:170`
371+
- `api/segment_membership/tasks.py:185`
372372

373373
Attributes:
374374
- `membership_counts.count`
375375
- `project.id`
376+
- `stale_counts.count`
376377

377378
### `segment_membership.refresh.project.failed`
378379

0 commit comments

Comments
 (0)