Skip to content

Commit 4bb18d1

Browse files
authored
Revert "ref(releases): Bulk read and update on the happy path" (#93097)
Reverts #92508 Previous pull request was merged as a performance optimization to the existing implementation. After monitoring for a few cycles it appears to be slightly slower! Given the lack of dramatic improvement and the cost of replacing long-standing, stable code, I've chosen to revert and leave it as is.
1 parent a268a2c commit 4bb18d1

File tree

3 files changed

+25
-338
lines changed

3 files changed

+25
-338
lines changed

src/sentry/options/defaults.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,13 +499,6 @@
499499
default=5000,
500500
flags=FLAG_AUTOMATOR_MODIFIABLE,
501501
)
502-
# Release health enable bulk queries.
503-
register(
504-
"release-health.tasks.adopt-releases.bulk",
505-
type=Bool,
506-
default=True,
507-
flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK | FLAG_AUTOMATOR_MODIFIABLE,
508-
)
509502
# Disables viewed by queries for a list of project ids.
510503
register(
511504
"replay.viewed-by.project-denylist",

src/sentry/release_health/tasks.py

Lines changed: 25 additions & 168 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
import logging
2-
from collections.abc import Callable, Iterator, Sequence
3-
from typing import Any, TypedDict, TypeVar
2+
from collections.abc import Iterator, Sequence
3+
from typing import Any, TypedDict
44

55
from django.core.exceptions import ValidationError
66
from django.db import IntegrityError
77
from django.db.models import F, Q
88
from django.utils import timezone
99
from sentry_sdk import capture_exception
1010

11-
from sentry import options
1211
from sentry.models.environment import Environment
1312
from sentry.models.project import Project
1413
from sentry.models.release import Release, ReleaseStatus
@@ -37,13 +36,13 @@
3736
namespace=release_health_tasks, retry=Retry(times=5, on=(Exception,))
3837
),
3938
)
40-
@metrics.wraps(
41-
"sentry.tasks.monitor_release_adoption.process_projects_with_sessions", sample_rate=1.0
42-
)
4339
def monitor_release_adoption(**kwargs) -> None:
4440
metrics.incr("sentry.tasks.monitor_release_adoption.start", sample_rate=1.0)
45-
for org_id, project_ids in release_monitor.fetch_projects_with_recent_sessions().items():
46-
process_projects_with_sessions.delay(org_id, project_ids)
41+
with metrics.timer(
42+
"sentry.tasks.monitor_release_adoption.process_projects_with_sessions", sample_rate=1.0
43+
):
44+
for org_id, project_ids in release_monitor.fetch_projects_with_recent_sessions().items():
45+
process_projects_with_sessions.delay(org_id, project_ids)
4746

4847

4948
@instrumented_task(
@@ -60,24 +59,22 @@ def monitor_release_adoption(**kwargs) -> None:
6059
),
6160
),
6261
)
63-
@metrics.wraps("sentry.tasks.monitor_release_adoption.process_projects_with_sessions.core")
6462
def process_projects_with_sessions(org_id, project_ids) -> None:
6563
# Takes a single org id and a list of project ids
66-
# Set the `has_sessions` flag for these projects
67-
Project.objects.filter(
68-
organization_id=org_id,
69-
id__in=project_ids,
70-
flags=F("flags").bitand(~Project.flags.has_sessions),
71-
).update(flags=F("flags").bitor(Project.flags.has_sessions))
72-
73-
totals = release_monitor.fetch_project_release_health_totals(org_id, project_ids)
74-
75-
if options.get("release-health.tasks.adopt-releases.bulk"):
76-
adopted_ids = adopt_releases_new(org_id, totals)
77-
else:
64+
65+
with metrics.timer("sentry.tasks.monitor_release_adoption.process_projects_with_sessions.core"):
66+
# Set the `has_sessions` flag for these projects
67+
Project.objects.filter(
68+
organization_id=org_id,
69+
id__in=project_ids,
70+
flags=F("flags").bitand(~Project.flags.has_sessions),
71+
).update(flags=F("flags").bitor(Project.flags.has_sessions))
72+
73+
totals = release_monitor.fetch_project_release_health_totals(org_id, project_ids)
74+
7875
adopted_ids = adopt_releases(org_id, totals)
7976

80-
cleanup_adopted_releases(project_ids, adopted_ids)
77+
cleanup_adopted_releases(project_ids, adopted_ids)
8178

8279

8380
def adopt_releases(org_id: int, totals: Totals) -> Sequence[int]:
@@ -166,89 +163,14 @@ def adopt_releases(org_id: int, totals: Totals) -> Sequence[int]:
166163
return adopted_ids
167164

168165

169-
@metrics.wraps("sentry.tasks.monitor_release_adoption.process_projects_with_sessions.updates")
170-
def adopt_releases_new(org_id: int, totals: Totals) -> Sequence[int]:
171-
adopted_releases = list(iter_adopted_releases(totals))
172-
if not adopted_releases:
173-
return []
174-
175-
# Happy path. Query the releases and adopt them.
176-
release_project_environments = query_adopted_release_project_environments(
177-
adopted_releases, org_id
178-
)
179-
adopt_release_project_environments(release_project_environments)
180-
adopted_ids = [m.id for m in release_project_environments]
181-
metrics.incr("sentry.tasks.process_projects_with_sessions.updated_rpe", amount=len(adopted_ids))
182-
183-
# For any release which was missing we still need to adopt it still but we may need to
184-
# create some other rows first.
185-
missing_releases = find_adopted_but_missing_releases(
186-
adopted_releases,
187-
[
188-
(m.project_id, m.environment.name, m.release.version)
189-
for m in release_project_environments
190-
],
191-
)
192-
for missing_release in missing_releases:
193-
metrics.incr("sentry.tasks.process_projects_with_sessions.creating_rpe")
194-
try:
195-
env = Environment.objects.get_or_create(
196-
name=missing_release["environment"], organization_id=org_id
197-
)[0]
198-
try:
199-
release = Release.objects.get_or_create(
200-
organization_id=org_id,
201-
version=missing_release["version"],
202-
defaults={
203-
"status": ReleaseStatus.OPEN,
204-
},
205-
)[0]
206-
except IntegrityError:
207-
release = Release.objects.get(
208-
organization_id=org_id, version=missing_release["version"]
209-
)
210-
except ValidationError:
211-
release = None
212-
logger.exception(
213-
"sentry.tasks.process_projects_with_sessions.creating_rpe.ValidationError",
214-
extra={
215-
"org_id": org_id,
216-
"release_version": missing_release["version"],
217-
},
218-
)
219-
220-
if release:
221-
release.add_project(Project.objects.get(id=missing_release["project_id"]))
222-
223-
ReleaseEnvironment.objects.get_or_create(
224-
environment=env, organization_id=org_id, release=release
225-
)
226-
227-
rpe = ReleaseProjectEnvironment.objects.create(
228-
project_id=missing_release["project_id"],
229-
release_id=release.id,
230-
environment=env,
231-
adopted=timezone.now(),
232-
)
233-
adopted_ids.append(rpe.id)
234-
except (
235-
Project.DoesNotExist,
236-
Environment.DoesNotExist,
237-
Release.DoesNotExist,
238-
ReleaseEnvironment.DoesNotExist,
239-
) as exc:
240-
metrics.incr("sentry.tasks.process_projects_with_sessions.skipped_update")
241-
capture_exception(exc)
242-
243-
return adopted_ids
244-
245-
246-
@metrics.wraps("sentry.tasks.monitor_release_adoption.process_projects_with_sessions.cleanup")
247166
def cleanup_adopted_releases(project_ids: Sequence[int], adopted_ids: Sequence[int]) -> None:
248167
# Cleanup; adopted releases need to be marked as unadopted if they are not in `adopted_ids`
249-
ReleaseProjectEnvironment.objects.filter(
250-
project_id__in=project_ids, unadopted__isnull=True
251-
).exclude(Q(adopted=None) | Q(id__in=adopted_ids)).update(unadopted=timezone.now())
168+
with metrics.timer(
169+
"sentry.tasks.monitor_release_adoption.process_projects_with_sessions.cleanup"
170+
):
171+
ReleaseProjectEnvironment.objects.filter(
172+
project_id__in=project_ids, unadopted__isnull=True
173+
).exclude(Q(adopted=None) | Q(id__in=adopted_ids)).update(unadopted=timezone.now())
252174

253175

254176
class AdoptedRelease(TypedDict):
@@ -257,71 +179,6 @@ class AdoptedRelease(TypedDict):
257179
version: str
258180

259181

260-
def query_adopted_release_project_environments(
261-
adopted_releases: list[AdoptedRelease],
262-
organization_id: int,
263-
) -> list[ReleaseProjectEnvironment]:
264-
"""Fetch a list of release project environment rows which match the adopted releases."""
265-
query_filters = Q()
266-
for release in adopted_releases:
267-
query_filters |= Q(
268-
release__organization_id=organization_id,
269-
release__version=release["version"],
270-
project_id=release["project_id"],
271-
environment__name=release["environment"],
272-
environment__organization_id=organization_id,
273-
)
274-
275-
return list(ReleaseProjectEnvironment.objects.filter(query_filters))
276-
277-
278-
def adopt_release_project_environments(models: list[ReleaseProjectEnvironment]) -> None:
279-
"""Bulk update release project environments with adopted time."""
280-
for model in models:
281-
if not model.adopted:
282-
model.adopted = timezone.now()
283-
if model.unadopted is not None:
284-
model.unadopted = None
285-
286-
ReleaseProjectEnvironment.objects.bulk_update(models, fields=["adopted", "unadopted"])
287-
288-
289-
def find_adopted_but_missing_releases(
290-
adopted_releases: list[AdoptedRelease],
291-
found_rows: list[tuple[int, str, str]],
292-
) -> list[AdoptedRelease]:
293-
"""Return a list of adopted releases which do not exist in the database.
294-
295-
Releases are not always defined in Sentry in advance. When we see a release that isn't in the
296-
database we need to do some additional processing.
297-
"""
298-
299-
def hash_release(q: AdoptedRelease) -> tuple[int, str, str]:
300-
return (q["project_id"], q["environment"], q["version"])
301-
302-
return find_missing(adopted_releases, hash_release, found_rows, lambda a: a)
303-
304-
305-
A = TypeVar("A")
306-
B = TypeVar("B")
307-
C = TypeVar("C")
308-
309-
310-
def find_missing(
311-
superset: list[A],
312-
superset_hasher: Callable[[A], C],
313-
subset: list[B],
314-
subset_hasher: Callable[[B], C],
315-
) -> list[A]:
316-
"""Return the difference of two sets.
317-
318-
This functionally is essentially set(A) - set(B) but when the types in the superset and subset
319-
are different and their hash values can't be directly compared.
320-
"""
321-
found_set = {subset_hasher(member) for member in subset}
322-
return [release for release in superset if superset_hasher(release) not in found_set]
323-
324-
325182
def iter_adopted_releases(totals: Totals) -> Iterator[AdoptedRelease]:
326183
"""Iterate through the totals set yielding the totals which are valid.
327184

0 commit comments

Comments
 (0)