Bug 2035204 - Prevent creation of duplicate AlertSummary rows for the same push - #9754
Open
junngo wants to merge 1 commit into
Open
Bug 2035204 - Prevent creation of duplicate AlertSummary rows for the same push#9754junngo wants to merge 1 commit into
junngo wants to merge 1 commit into
Conversation
✅ Deploy Preview for treeherder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
junngo
commented
Aug 5, 2026
| _generate_performance_data(test_repository, signature, base_time, 1, 0.5, before) | ||
| _generate_performance_data( | ||
| test_repository, signature, base_time, int(interval / 2) + 1, 1.0, int(interval / 2) | ||
| ) |
Collaborator
Author
There was a problem hiding this comment.
Example: when gap=True
t: 1 2 ... 13 14 | 15 | 16 17 ... 30
value: 0.5 0.5 ... 0.5 0.5 | (missing) | 1.0 1.0 ... 1.0
↑
no PerformanceDatum exists for t=15
| sheriffed=not signature.monitor, | ||
| ) | ||
| .order_by("created", "id") | ||
| .first() |
Collaborator
Author
There was a problem hiding this comment.
The lead columns for the index are repository and framework, so those are still index-scoped; push and sheriffed are filtered after that. Total AlertSummary row count is about 5,300, so this shouldn't be a performance concern for now.
| existing_prev_timestamp = time.mktime(summary.prev_push.time.timetuple()) | ||
| if prev.push_timestamp < existing_prev_timestamp: | ||
| summary.prev_push_id = prev.push_id | ||
| summary.save(update_fields=["prev_push_id"]) |
Collaborator
Author
There was a problem hiding this comment.
I chose the widened prev_push over the narrower one. Narrowing favors the comparison for the alert that was just merged, while widening avoids losing comparison range for any of the merged alerts. This felt like a policy call, so happy to hear your opinion.
junngo
force-pushed
the
prevent-dup-alert-summary
branch
from
August 6, 2026 08:28
918fbcb to
349fd77
Compare
Comment on lines
+202
to
+209
| PerformanceAlertSummary.objects.filter( | ||
| repository=signature.repository, | ||
| framework=signature.framework, | ||
| push_id=cur.push_id, | ||
| sheriffed=not signature.monitor, | ||
| ) | ||
| .order_by("created", "id") | ||
| .first() |
Collaborator
There was a problem hiding this comment.
Add select_related so prev_push is fetched in the same query
Suggested change
| PerformanceAlertSummary.objects.filter( | |
| repository=signature.repository, | |
| framework=signature.framework, | |
| push_id=cur.push_id, | |
| sheriffed=not signature.monitor, | |
| ) | |
| .order_by("created", "id") | |
| .first() | |
| PerformanceAlertSummary.objects.select_related("prev_push") | |
| .filter( | |
| repository=signature.repository, | |
| framework=signature.framework, | |
| push_id=cur.push_id, | |
| sheriffed=not signature.monitor, | |
| ) | |
| .order_by("created", "id") | |
| .first() |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=2035204
This PR prevents duplicate AlertSummary rows from being created for the same push.
When a new alert is merged into an existing AlertSummary,
prev_push_idis widened to the earliest prev_push seen across the merged alerts, but never narrowed. This is because prev_push_id drives the PerfCompare link, so narrowing it could risk breaking existing references. Manually created summaries (with a sheriff-set prev_push) are left untouched.Happy to hear any thoughts or concerns on this approach.