Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from django.db import OperationalError

import pytest

from pgcommitfest.commitfest.views import retry_on_serialization_failure


def _pg_error(pgcode):
# Django re-raises psycopg2 errors with ``from``, so the SQLSTATE is only
# reachable through ``__cause__.pgcode``, which is what the decorator reads.
cause = Exception("underlying psycopg2 error")
cause.pgcode = pgcode
error = OperationalError("could not serialize access due to concurrent update")
error.__cause__ = cause
return error


@pytest.mark.parametrize(
"pgcode,failures,expected_calls",
[
("40001", 1, 2), # serialization failure, recovers
("40P01", 1, 2), # deadlock, recovers
("40001", 99, 2), # persistent failure, gives up after one retry
("57014", 99, 1), # query_canceled is not transient, no retry
],
)
def test_retry_on_serialization_failure(pgcode, failures, expected_calls):
calls = []

@retry_on_serialization_failure
def view():
calls.append(1)
if len(calls) <= failures:
raise _pg_error(pgcode)
return "ok"

if failures >= expected_calls:
with pytest.raises(OperationalError):
view()
else:
assert view() == "ok"
assert len(calls) == expected_calls
46 changes: 45 additions & 1 deletion pgcommitfest/commitfest/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import User
from django.db import connection, transaction
from django.db import OperationalError, connection, transaction
from django.db.models import Count, Q
from django.http import (
Http404,
Expand All @@ -15,6 +15,7 @@
from django.views.decorators.csrf import csrf_exempt

import collections
import functools
import hmac
import json
import urllib
Expand Down Expand Up @@ -48,6 +49,47 @@
UserInputError,
)

# PostgreSQL SQLSTATEs for transient conflicts that are safe to retry once the
# aborted transaction has been rolled back: serialization_failure and
# deadlock_detected.
RETRYABLE_PGCODES = {"40001", "40P01"}


def retry_on_serialization_failure(view_func):
"""Re-run a view when its transaction hits a serialization failure.

A few views (home, commitfest) run the whole request at REPEATABLE READ so
that all of their queries observe a single consistent snapshot. Rendering
also triggers the lazy, roughly-once-a-month commitfest refresh in
CommitFest.relevant_commitfests(), which locks and updates rows with
SELECT ... FOR UPDATE. When two such requests race during a rollover, the
one that loses the race cannot re-read the newer row under REPEATABLE READ,
so PostgreSQL aborts it with "could not serialize access due to concurrent
update" (SQLSTATE 40001) instead of blocking and continuing.

Once that happens the transaction is dead, so the only way to recover is to
roll it back and run the view again from scratch. That is why this decorator
has to sit *outside* transaction.atomic: each attempt then runs in a brand
new transaction with a fresh snapshot. By the time we retry, the request
that won the race has already committed the refresh, so the cheap
up-to-date check short-circuits and the retry neither writes nor conflicts.
"""

@functools.wraps(view_func)
def wrapper(*args, **kwargs):
try:
return view_func(*args, **kwargs)
except OperationalError as e:
if getattr(e.__cause__, "pgcode", None) not in RETRYABLE_PGCODES:
raise
# A single retry is enough: the refresh only writes once per rollover,
# so after the winning request commits there is nothing left to
# conflict with. If the retry fails anyway, something else is going on
# and we'd rather surface the error than keep hammering the database.
return view_func(*args, **kwargs)

return wrapper


def get_tags_data():
"""Generate JSON data for enhanced selectize dropdown with tag colors and descriptions."""
Expand All @@ -64,6 +106,7 @@ def get_tags_data():
)


@retry_on_serialization_failure
@transaction.atomic
def home(request):
curs = connection.cursor()
Expand Down Expand Up @@ -607,6 +650,7 @@ def patchlist(request, cf, personalized=False):
)


@retry_on_serialization_failure
@transaction.atomic
def commitfest(request, cfid):
curs = connection.cursor()
Expand Down
Loading