Skip to content

Commit b51e6b7

Browse files
authored
perf(Segment Membership): Count all segments in one scan per environment (#8018)
1 parent 0e3c8c7 commit b51e6b7

3 files changed

Lines changed: 88 additions & 24 deletions

File tree

api/segment_membership/services.py

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,10 @@ def compute_segment_counts_for_project(
110110
project: Project, cursor: CursorWrapper
111111
) -> list[SegmentMembershipCount]:
112112
"""Count identity matches per (canonical-segment, environment) for
113-
`project` in one `UNION ALL` query.
113+
`project`, scanning each environment once.
114+
115+
A single `GROUP BY environment_id` over `IDENTITIES FINAL` counts every
116+
segment in one pass via `countIf(<predicate>)` per segment.
114117
115118
Returns unsaved `SegmentMembershipCount` instances with `count` and
116119
keys populated; the caller stamps `last_synced_at` consistently
@@ -128,7 +131,8 @@ def compute_segment_counts_for_project(
128131

129132
dialect = ClickHouseDialect()
130133
binder = Binder(PyformatParamStyle())
131-
select_clauses: list[str] = []
134+
count_columns: list[str] = []
135+
counted_segment_ids: list[int] = []
132136
for seg in segments:
133137
translate_ctx = TranslateContext(
134138
evaluation_context=EvaluationContext(
@@ -149,33 +153,35 @@ def compute_segment_counts_for_project(
149153
reason="untranslatable",
150154
)
151155
continue
152-
select_clauses.append(
153-
f"SELECT {seg.id} AS segment_id, "
154-
f"i.environment_id AS env_key, count() AS c "
155-
f"FROM IDENTITIES AS i FINAL "
156-
f"WHERE i.environment_id IN %(env_keys)s "
157-
f"AND i.is_deleted = false AND ({predicate}) "
158-
f"GROUP BY i.environment_id"
159-
)
156+
count_columns.append(f"countIf({predicate}) AS c{seg.id}")
157+
counted_segment_ids.append(seg.id)
160158

161-
if not select_clauses:
159+
if not count_columns:
162160
return []
163161

164-
sql = "\nUNION ALL\n".join(select_clauses)
162+
sql = (
163+
f"SELECT i.environment_id AS env_key, {', '.join(count_columns)} "
164+
f"FROM IDENTITIES AS i FINAL "
165+
f"WHERE i.environment_id IN %(env_keys)s AND i.is_deleted = false "
166+
f"GROUP BY i.environment_id"
167+
)
165168
cursor.execute(sql, {"env_keys": tuple(env_id_by_key), **binder.params})
166169
rows: list[tuple[Any, ...]] = cursor.fetchall()
167170
membership_counts: list[SegmentMembershipCount] = []
168171
for row in rows:
169-
env_id = env_id_by_key.get(str(row[1]))
172+
env_id = env_id_by_key.get(str(row[0]))
170173
if env_id is None:
171174
continue
172-
membership_counts.append(
173-
SegmentMembershipCount(
174-
segment_id=int(row[0]),
175-
environment_id=env_id,
176-
count=int(row[2]),
177-
)
178-
)
175+
# Columns line up with counted_segment_ids; a zero-match pair is absent.
176+
for segment_id, count in zip(counted_segment_ids, row[1:]):
177+
if count:
178+
membership_counts.append(
179+
SegmentMembershipCount(
180+
segment_id=segment_id,
181+
environment_id=env_id,
182+
count=int(count),
183+
)
184+
)
179185
return membership_counts
180186

181187

api/tests/unit/segment_membership/test_unit_segment_membership_services.py

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from django.db import connections
77
from django.utils import timezone
88
from flag_engine.segments.constants import EQUAL, REGEX
9-
from pytest_django.fixtures import SettingsWrapper
9+
from pytest_django.fixtures import DjangoAssertNumQueries, SettingsWrapper
1010
from pytest_mock import MockerFixture
1111
from task_processor.models import Task
1212
from task_processor.task_run_method import TaskRunMethod
@@ -131,7 +131,7 @@ def test_compute_segment_counts_for_project__unknown_env_key_in_row__skips(
131131
return_value="TRUE",
132132
)
133133
cursor = MagicMock()
134-
cursor.fetchall.return_value = [(segment.id, "ghost-env", 99)]
134+
cursor.fetchall.return_value = [("ghost-env", 99)]
135135

136136
# When
137137
result = compute_segment_counts_for_project(project, cursor)
@@ -198,6 +198,27 @@ def percent_regex_segment(segment: Segment) -> Segment:
198198
return segment
199199

200200

201+
@pytest.fixture
202+
def three_segments(matching_segment: Segment, project: Project) -> dict[str, Segment]:
203+
"""`matching_segment` matches foo == bar (alice, bob); add one matching a
204+
different value (carol) and one matching nobody."""
205+
seg_baz = Segment.objects.create(name="baz", project=project)
206+
Condition.objects.create(
207+
rule=SegmentRule.objects.create(segment=seg_baz, type=SegmentRule.ALL_RULE),
208+
property="foo",
209+
operator=EQUAL,
210+
value="baz",
211+
)
212+
seg_none = Segment.objects.create(name="none", project=project)
213+
Condition.objects.create(
214+
rule=SegmentRule.objects.create(segment=seg_none, type=SegmentRule.ALL_RULE),
215+
property="foo",
216+
operator=EQUAL,
217+
value="zzz",
218+
)
219+
return {"bar": matching_segment, "baz": seg_baz, "none": seg_none}
220+
221+
201222
@pytest.mark.clickhouse
202223
def test_get_segment_members_page__deleted_identity__excluded(
203224
segment_membership_identities: None,
@@ -288,6 +309,43 @@ def test_compute_segment_counts_for_project__deleted_identity__excluded_from_cou
288309
assert counts[0].count == 2
289310

290311

312+
@pytest.mark.clickhouse
313+
def test_compute_segment_counts_for_project__multiple_segments__maps_each_count_drops_zeros(
314+
segment_membership_identities: None,
315+
three_segments: dict[str, Segment],
316+
project: Project,
317+
) -> None:
318+
# Given / When
319+
with connections["clickhouse"].cursor() as cursor:
320+
counts = compute_segment_counts_for_project(project, cursor)
321+
322+
# Then
323+
by_segment = {c.segment_id: c.count for c in counts}
324+
assert by_segment == {
325+
three_segments["bar"].id: 2,
326+
three_segments["baz"].id: 1,
327+
}
328+
assert three_segments["none"].id not in by_segment
329+
330+
331+
@pytest.mark.clickhouse
332+
def test_compute_segment_counts_for_project__multiple_segments__uses_single_query(
333+
segment_membership_identities: None,
334+
three_segments: dict[str, Segment],
335+
project: Project,
336+
django_assert_num_queries: DjangoAssertNumQueries,
337+
) -> None:
338+
# Given
339+
# three_segments + identities are set up by fixtures
340+
341+
# When / Then
342+
with (
343+
django_assert_num_queries(1, connection=connections["clickhouse"]),
344+
connections["clickhouse"].cursor() as cursor,
345+
):
346+
compute_segment_counts_for_project(project, cursor)
347+
348+
291349
@pytest.mark.clickhouse
292350
def test_get_segment_members_page__regex_with_percent__returns_matches(
293351
segment_membership_identities: None,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ Attributes:
369369
### `segment_membership.compute.segment.skipped`
370370

371371
Logged at `error` from:
372-
- `api/segment_membership/services.py:145`
372+
- `api/segment_membership/services.py:149`
373373

374374
Attributes:
375375
- `project.id`
@@ -379,7 +379,7 @@ Attributes:
379379
### `segment_membership.members.segment.skipped`
380380

381381
Logged at `error` from:
382-
- `api/segment_membership/services.py:209`
382+
- `api/segment_membership/services.py:215`
383383

384384
Attributes:
385385
- `reason`

0 commit comments

Comments
 (0)