Skip to content

Commit d3d2db9

Browse files
committed
refactor(Segment Membership): Distribute count refreshes evenly across the window
Per review, drop the per-organisation grouping (and its cumulative-delay overflow handling and STAGGER_SECONDS setting) in favour of spreading all live-segment projects evenly across the stagger window, ordered by organisation so an org's projects still refresh contiguously. Simpler, and no org-slot overflow to guard. beep boop Claude-Session: https://claude.ai/code/session_01EgZ5iHpDASZzCapiHRxHLB
1 parent 8df9042 commit d3d2db9

4 files changed

Lines changed: 33 additions & 122 deletions

File tree

api/app/settings/common.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,9 +1494,6 @@
14941494
"SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_WINDOW_HOURS must not exceed "
14951495
"SEGMENT_MEMBERSHIP_REFRESH_INTERVAL_HOURS."
14961496
)
1497-
SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_SECONDS = env.int(
1498-
"SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_SECONDS", default=10
1499-
)
15001497
SEGMENT_MEMBERSHIP_DELETE_REFRESH_DELAY_SECONDS = env.int(
15011498
"SEGMENT_MEMBERSHIP_DELETE_REFRESH_DELAY_SECONDS",
15021499
default=120, # We can expect the identity deletion to propagate by T+120 seconds based on Edge CDC SLO.

api/segment_membership/tasks.py

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -174,44 +174,23 @@ def refresh_all_segment_counts() -> None:
174174
if not settings.CLICKHOUSE_ENABLED:
175175
return
176176

177-
project_ids = Segment.live_objects.values_list("project_id", flat=True)
178-
num_orgs = (
179-
Project.objects.filter(id__in=project_ids)
180-
.values("organisation_id")
181-
.distinct()
182-
.count()
177+
projects = (
178+
Project.objects.filter(
179+
Exists(Segment.live_objects.filter(project=OuterRef("pk")))
180+
)
181+
.select_related("organisation")
182+
.order_by("organisation_id", "id")
183183
)
184-
if not num_orgs:
184+
total = projects.count()
185+
if not total:
185186
return
186187

187-
org_spacing = timedelta(
188+
spacing = timedelta(
188189
hours=settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_WINDOW_HOURS
189-
) / (num_orgs + 1)
190-
project_spacing = timedelta(
191-
seconds=settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_SECONDS
192-
)
190+
) / (total + 1)
193191
now = timezone.now()
194-
195-
# Accumulate the delay so it never moves backwards: each organisation starts
196-
# at its window slot or after the previous organisation's last project,
197-
# whichever is later. This keeps an organisation with many projects from
198-
# spilling its stagger into the next organisation's slot.
199-
cumulative_delay = timedelta(0)
200-
org_index = -1
201-
current_org_id = None
202-
projects = (
203-
Project.objects.filter(id__in=project_ids)
204-
.select_related("organisation")
205-
.order_by("organisation_id", "id")
206-
)
207-
for project in projects.iterator():
208-
if project.organisation_id != current_org_id:
209-
current_org_id = project.organisation_id
210-
org_index += 1
211-
cumulative_delay = max(cumulative_delay, org_spacing * org_index)
212-
else:
213-
cumulative_delay += project_spacing
214-
enqueue_membership_refresh(project, delay_until=now + cumulative_delay)
192+
for index, project in enumerate(projects.iterator()):
193+
enqueue_membership_refresh(project, delay_until=now + spacing * index)
215194

216195

217196
@register_task_handler(

api/tests/unit/segment_membership/test_unit_segment_membership_tasks.py

Lines changed: 17 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -392,104 +392,39 @@ def test_refresh_all_segment_counts__no_live_segments__does_nothing(
392392
enqueue.assert_not_called()
393393

394394

395-
def test_refresh_all_segment_counts__live_segment_projects__delegates_to_enqueue(
395+
def test_refresh_all_segment_counts__live_segment_projects__staggers_evenly_by_org(
396396
mocker: MockerFixture,
397397
settings: SettingsWrapper,
398398
project: Project,
399399
segment: Segment,
400400
) -> None:
401-
# Given two projects with live segments (flag + debounce gating is the
402-
# helper's job, so refresh_all just delegates one call per project)
403-
project_b = Project.objects.create(
404-
name="project-b", organisation=project.organisation
405-
)
406-
Segment.objects.create(name="seg-b", project=project_b)
407-
settings.CLICKHOUSE_ENABLED = True
408-
enqueue = mocker.patch.object(tasks, "enqueue_membership_refresh")
409-
410-
# When
411-
refresh_all_segment_counts()
412-
413-
# Then
414-
calls = enqueue.call_args_list
415-
assert {call.args[0].id for call in calls} == {project.id, project_b.id}
416-
# enqueues are staggered across the refresh window, not fired all at once
417-
delays = sorted(call.kwargs["delay_until"] for call in calls)
418-
window = timedelta(
419-
hours=settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_WINDOW_HOURS
420-
)
421-
assert delays[0] < delays[1]
422-
assert delays[-1] - delays[0] < window
423-
424-
425-
def test_refresh_all_segment_counts__multiple_orgs__staggers_by_org_then_project(
426-
mocker: MockerFixture,
427-
settings: SettingsWrapper,
428-
project: Project,
429-
segment: Segment,
430-
) -> None:
431-
# Given org A with two live-segment projects and org B with one
401+
# Given three live-segment projects across two organisations
432402
project_a2 = Project.objects.create(name="a2", organisation=project.organisation)
433403
Segment.objects.create(name="seg-a2", project=project_a2)
434404
org_b = Organisation.objects.create(name="org-b")
435405
project_b = Project.objects.create(name="b1", organisation=org_b)
436406
Segment.objects.create(name="seg-b", project=project_b)
437407
settings.CLICKHOUSE_ENABLED = True
438-
settings.SEGMENT_MEMBERSHIP_REFRESH_INTERVAL_HOURS = 6
439-
settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_WINDOW_HOURS = 3
440-
settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_SECONDS = 10
408+
settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_WINDOW_HOURS = 4
441409
enqueue = mocker.patch.object(tasks, "enqueue_membership_refresh")
442410

443411
# When
444412
refresh_all_segment_counts()
445413

446-
# Then
447-
delay_by_id = {
448-
call.args[0].id: call.kwargs["delay_until"] for call in enqueue.call_args_list
449-
}
450-
assert set(delay_by_id) == {project.id, project_a2.id, project_b.id}
451-
# org A's projects cluster a project-stagger apart
452-
org_a = sorted([delay_by_id[project.id], delay_by_id[project_a2.id]])
453-
assert org_a[1] - org_a[0] == timedelta(seconds=10)
454-
# org B sits a full window-slot (window / (orgs + 1)) away, not a project-stagger
455-
org_spacing = timedelta(hours=3) / 3
456-
assert abs(delay_by_id[project_b.id] - org_a[0]) == org_spacing
457-
458-
459-
def test_refresh_all_segment_counts__org_overflows_slot__no_cross_org_collision(
460-
mocker: MockerFixture,
461-
settings: SettingsWrapper,
462-
project: Project,
463-
segment: Segment,
464-
) -> None:
465-
# Given org A with three live-segment projects and org B with one, and a
466-
# project stagger wide enough that A's projects overflow its window slot
467-
# (org_spacing = 1h / 3 = 20min, but the projects are 1h apart).
468-
project_a2 = Project.objects.create(name="a2", organisation=project.organisation)
469-
Segment.objects.create(name="seg-a2", project=project_a2)
470-
project_a3 = Project.objects.create(name="a3", organisation=project.organisation)
471-
Segment.objects.create(name="seg-a3", project=project_a3)
472-
org_b = Organisation.objects.create(name="org-b")
473-
project_b = Project.objects.create(name="b1", organisation=org_b)
474-
Segment.objects.create(name="seg-b", project=project_b)
475-
settings.CLICKHOUSE_ENABLED = True
476-
settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_WINDOW_HOURS = 1
477-
settings.SEGMENT_MEMBERSHIP_REFRESH_PROJECT_STAGGER_SECONDS = 3600
478-
enqueue = mocker.patch.object(tasks, "enqueue_membership_refresh")
479-
480-
# When
481-
refresh_all_segment_counts()
482-
483-
# Then
484-
delay_by_id = {
485-
call.args[0].id: call.kwargs["delay_until"] for call in enqueue.call_args_list
486-
}
487-
org_a = sorted(delay_by_id[p] for p in (project.id, project_a2.id, project_a3.id))
488-
# org A's projects stay a project-stagger apart, overflowing the 20min slot
489-
assert org_a[1] - org_a[0] == timedelta(seconds=3600)
490-
assert org_a[2] - org_a[1] == timedelta(seconds=3600)
491-
# org B never schedules before org A's last project, so the two don't collide
492-
assert delay_by_id[project_b.id] >= org_a[-1]
414+
# Then one call per project, ordered by (organisation_id, id) so an
415+
# organisation's projects refresh contiguously
416+
called = [
417+
(call.args[0].id, call.kwargs["delay_until"]) for call in enqueue.call_args_list
418+
]
419+
expected_order = sorted(
420+
(project, project_a2, project_b), key=lambda p: (p.organisation_id, p.id)
421+
)
422+
assert [pid for pid, _ in called] == [p.id for p in expected_order]
423+
# spread evenly across the window: spacing = window / (total + 1)
424+
spacing = timedelta(hours=4) / 4
425+
delays = [delay for _, delay in called]
426+
assert delays[1] - delays[0] == spacing
427+
assert delays[2] - delays[1] == spacing
493428

494429

495430
def test_refresh_project_segment_counts__no_clickhouse_creds__skips(

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ Attributes:
388388
### `segment_membership.refresh.project.completed`
389389

390390
Logged at `info` from:
391-
- `api/segment_membership/tasks.py:283`
391+
- `api/segment_membership/tasks.py:262`
392392

393393
Attributes:
394394
- `membership_counts.count`
@@ -398,16 +398,16 @@ Attributes:
398398
### `segment_membership.refresh.project.failed`
399399

400400
Logged at `exception` from:
401-
- `api/segment_membership/tasks.py:256`
401+
- `api/segment_membership/tasks.py:235`
402402

403403
Attributes:
404404
- `project.id`
405405

406406
### `segment_membership.refresh.project.skipped`
407407

408408
Logged at `info` from:
409-
- `api/segment_membership/tasks.py:227`
410-
- `api/segment_membership/tasks.py:236`
409+
- `api/segment_membership/tasks.py:206`
410+
- `api/segment_membership/tasks.py:215`
411411

412412
Attributes:
413413
- `project.id`

0 commit comments

Comments
 (0)