Repository navigation
Retry home and commitfest views on serialization failures - #125
Merged
Merged
Conversation
The `home` and `commitfest` views run at REPEATABLE READ so all their queries see one snapshot. Rendering them can also trigger the lazy commitfest refresh in `CommitFest.relevant_commitfests()`, which locks rows with `SELECT ... FOR UPDATE`. When two requests race during a commitfest rollover, the one that loses can't lock the row that the other just updated, so PostgreSQL aborts it with "could not serialize access due to concurrent update" and the user gets a 500. These views are now wrapped in `retry_on_serialization_failure`, which sits outside `transaction.atomic` and reruns the view in a fresh transaction when it fails with a serialization failure (40001) or deadlock (40P01). On retry the winning request has already committed the refresh, so the up-to-date check short-circuits and nothing conflicts anymore. Attempts are capped at five so a persistent conflict still surfaces as an error.
JelteF
marked this pull request as ready for review
October 18, 2026 07:40
The four retry tests shared the same body and only differed in the SQLSTATE raised, how often it was raised, and the expected number of attempts, so they are now a single parametrized test. The non-retryable case uses 57014 (query_canceled) instead of 23505, because a unique violation surfaces as `IntegrityError` rather than `OperationalError` in Django, which made that case unrealistic.
The refresh only writes once per rollover, so once the request that won the race has committed, a retry has nothing left to conflict with. If that single retry still fails, something else is wrong and it's better to surface the error than to keep rerunning the whole view.
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.
When two requests to the
homeorcommitfestviews race during a commitfest rollover, the lazy commitfest refresh makes one of them fail with "could not serialize access due to concurrent update", because these views run at REPEATABLE READ. That resulted in a 500 for the user. Now these views are retried once in a fresh transaction on serialization failures and deadlocks. At that point the other request has already committed the refresh, so there's nothing left to conflict with.