fix(db): report lock contention instead of throwing (!32) · Merge requests · GlitchTip / Django VTasks

Summary

DatabaseTaskBackend.acquire_lock throws instead of reporting contention, on the ordinary path, with no fault involved.

AI disclosure: Claude Code — found it via the gauntlet, wrote the fix and the test, and drafted this description.

The bug

except VTaskMetadata.DoesNotExist:
    VTaskMetadata.objects.create(...)      # ← IntegrityError escapes from here
    return True
except DatabaseError:
    # This can happen if another process created the lock row concurrently.
    return False

The comment names exactly this race, and IntegrityError is a DatabaseError — but a sibling except clause cannot catch anything raised from inside another handler, so it escapes regardless.

What makes it the ordinary path rather than an edge case is skip_locked: while a sibling holds the row, the SELECT skips it and .get() raises DoesNotExist even though the row plainly exists. So under normal two-worker contention the loser throws every time.

The same lock gates the periodic rescue sweep, the scheduler and the delayed-task promoter, so a lost race silently costs that tick of all three.

How it was found

The gauntlet's db-baseline scenario — no faults at all, two workers:

Periodic rescue sweep failed
Traceback (most recent call last):
  File "django_vtasks/backends/db.py", line 231, in acquire_lock
    lock = VTaskMetadata.objects.select_for_update(**self._get_lock_params()).get(key=key)
django_vtasks.db.models.VTaskMetadata.DoesNotExist: ...

During handling of the above exception, another exception occurred:
django.db.utils.IntegrityError: duplicate key value violates unique constraint
"db_vtaskmetadata_pkey"
DETAIL:  Key (key)=(vtasks_rescue_lock) already exists.

It was on the harness's ranked list of things found by inspection; this is the first time it has been demonstrated running.

The fix

The insert moves into a savepoint. That is not incidental: catching a database error inside atomic() without one poisons the transaction, which would move the failure from acquire_lock to COMMIT — a fix that only relocates the crash.

The except DatabaseError clause stays for genuine failures in the SELECT or the refresh, with its comment corrected, since the case it described can no longer reach it.

Test

test_losing_the_insert_race_is_contention_not_an_error reproduces the production path rather than simulating it: a holder thread takes a blocking select_for_update on the row, so the skip_locked reader genuinely misses a row that exists, and the insert genuinely hits the primary key. Verified red against the unfixed handler with the real IntegrityError, green with the fix. It also asserts a query after the lost race, which is what pins the savepoint — without it the transaction is left unusable.

PostgreSQL-only (it needs skip_locked to make the SELECT miss); skips elsewhere.

Verification

204 tests OK on Postgres 18 + Valkey 9; 185 OK on SQLite (12 skipped)
ruff check / ruff format --check clean

fix(db): report lock contention instead of throwing