Skip to content

fix(dedupe): only count import locks held in the current database - #16397

Merged
Maffooch merged 1 commit into
devfrom
fix/dupe-delete-import-lock-current-db
Oct 17, 2026
Merged

Maffooch merged 1 commit into
devfrom
fix/dupe-delete-import-lock-current-db

Conversation

@Maffooch

@Maffooch Maffooch commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Shortcut: sc-16686

Description

test_duplicate_of_a_test_just_reimported_is_deleted_once_the_import_has_ended (added in #16379) failed in 4 of the last 5 failed merge-queue runs, on amd64 and arm64, with Locations on and off. The flake comes from a real bug.

The excess-duplicate delete skips tests whose import lock is held. To find those tests, _import_lock_holders reads pg_locks. But pg_locks lists advisory locks for every database on the server, and an advisory lock only conflicts inside its own database. So a test was left out of the delete whenever a test with the same id was being imported in a different database:

  • in CI, another --parallel worker, since each worker has its own test_defectdojo_N database;
  • in production, another DefectDojo database on the same Postgres server. One instance's imports could hold back another instance's duplicate cleanup.

The per-chunk try-lock (tests_under_import_now) was already scoped to the database. Only the selection-time read and the abandoned-lock check used the unfiltered view.

I confirmed this on a scratch Postgres: with the lock held in db_a, the current holder query run from db_b reports the test as locked, while pg_try_advisory_xact_lock from db_b succeeds.

The fix filters the holder query to current_database().

Test results

New regression test test_import_lock_in_another_database_does_not_hold_back_the_delete holds the lock from the postgres database and checks that the duplicate is still deleted.

  • Without the fix it fails: an import lock held in another database kept this database's excess duplicate.
  • With the fix, all 50 tests in unittests.test_dedupe_delete_commit_race pass locally.
  • ruff check (0.16.10) is clean.

Documentation

None needed.

🤖 Generated with Claude Code

The excess-duplicate delete read import-lock holders from pg_locks, which
lists advisory locks for every database on the server. Advisory locks only
conflict within their own database, so a test was skipped whenever another
database's test with the same id was being imported: another DefectDojo
instance on a shared cluster, or another --parallel test worker. That made
test_duplicate_of_a_test_just_reimported_is_deleted_once_the_import_has_ended
flaky in the merge queue.

Filter the holder query to the current database, and add a regression test
that holds the lock from another database.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch
Maffooch requested a review from blakeaowens as a code owner October 17, 2026 17:00
@Maffooch Maffooch added this to the 3.4.200 milestone Oct 17, 2026
@Maffooch
Maffooch enabled auto-merge October 17, 2026 17:02
@Maffooch
Maffooch requested a review from devGregA October 17, 2026 17:02
@Maffooch
Maffooch added this pull request to the merge queue Oct 17, 2026
Merged via the queue into dev with commit 6080616 Oct 17, 2026
48 checks passed
@Maffooch
Maffooch deleted the fix/dupe-delete-import-lock-current-db branch October 17, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants