ref(releases): Remove DISTINCT query modifier and use EXISTS subqueries to join #95229
+65
−21
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Backend
cmanallen
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Re Cursor:
This is wrong. If environments can not be fetched the resource aborts 404. Even if it didn't abort environment names are only appended to the filter_params if all the environments are returned.
This is wrong. Previous query pattern checked if the environments key existed prior to applying filters.
This is wrong. Project ids will always be populated in this context. If not an exception is raised: sentry/src/sentry/api/bases/organization.py Lines 566 to 567 in 156f296 |
There was a problem hiding this comment.
Bug: API Breaks on Empty Environment Filters
The filter_releases_by_environments function incorrectly skips filtering when environment_ids is empty. Previously, an empty environment filter would return no results (filtering out all releases). Now, it returns unfiltered results, which is a breaking change in API behavior for the OrganizationReleasesEndpoint and OrganizationReleasesStatsEndpoint. This can lead to unexpected data exposure when users apply empty or invalid environment filters.
src/sentry/models/release.py#L839-L857
sentry/src/sentry/models/release.py
Lines 839 to 857 in 3f33d1f
src/sentry/api/endpoints/organization_releases.py#L315-L320
sentry/src/sentry/api/endpoints/organization_releases.py
Lines 315 to 320 in 3f33d1f
src/sentry/api/endpoints/organization_releases.py#L678-L683
sentry/src/sentry/api/endpoints/organization_releases.py
Lines 678 to 683 in 3f33d1f
Was this report helpful? Give feedback by reacting with 👍 or 👎
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem statement:
Long running queries are being canceled due to timeouts. Somewhat amazingly DataDog does not report a cost associated with the sort and distinct nodes. I find this hard to believe so I've removed the DISTINCT modifier by using EXISTS subqueries to keep the source dataset de-duplicated. The EXISTS subquery should be a lateral move in terms of performance because both query patterns use a 'Nested Loop Join'. Removing the DISTINCT modifier should be a net benefit all things considered.
Additional Notes:
There are multiple query patterns present in this endpoint. I've document two samples below. This change always lowers the minimum cost to execute the query while mostly not changing the maximum cost. However in some cases the maximum cost is higher. This is not as clear of a win as we saw in #95135. We'll let it ride for a while and monitor its performance. If we see noticeable outliers we may revert but it would have to be in excess of the outliers we see with the current query.
Previous Query Plan Sample (test database):
New Query Plan Sample (test database):