test(finding): accept either deadlock victim in the bulk-delete vs tag writer race

[Maffooch]

[sc-15987]

Description

TestBulkDeleteRacesConcurrentM2MWriter.test_tag_writer_holding_a_tag_row_does_not_fail_the_chunk (added in #16091) is flaky. Its failure is OperationalError: deadlock detected, raised in the concurrent tag writer thread.

The test forces a real deadlock. bulk_add_tags_to_instances holds a tag's count row, and the bulk-delete chunk holds the finding rows and waits on that tag row. The writer then commits, and its deferred foreign-key check waits on the chunk's finding lock. The test assumed Postgres always aborts the chunk, whose conflict retry then finishes the delete.

Postgres does not guarantee which side it aborts. A waiting backend runs the deadlock check once, deadlock_timeout after it started waiting, and aborts itself if it finds a cycle. The chunk starts waiting first, so it is the victim only when the writer reaches its COMMIT within deadlock_timeout. On a slow or loaded machine the writer gets there later: the chunk's one check has already run and found no cycle, so the writer's check finds the cycle and the writer is aborted.

Both orders meet the guarantee #16091 describes: the chunk deletes, no tag through row survives, and the tag count stays consistent. The product code needs no change. When the writer is the victim its tag add rolls back and the chunk commits on its first attempt, which is also a consistent result.

Changes, all in unittests/test_dedupe_delete_commit_race.py:

• A shared helper sets up the cycle. It then asserts the delete outcome for both orders, and that exactly one side was the deadlock victim. It counts the chunk's attempts through lock_findings_for_delete and checks the writer for a transient conflict error. Any other writer error still fails the test.
• The existing test runs the helper with no timing control and accepts either victim.
• A new test, test_tag_writer_aborted_as_the_deadlock_victim_leaves_the_chunk_to_finish, forces the writer-victim order. The writer holds its COMMIT for twice the server's deadlock_timeout, which is read at run time. The test then asserts the writer was aborted and the chunk did not retry. So the order that used to be the flake is covered on purpose.

The follow-up that #16091 mentions still applies: bulk_add_tags_to_instances could take FOR KEY SHARE on its findings before the tag UPDATE and remove this cycle entirely.

Test results

These ran in the OSS unit-test compose (ossfix-unittests image) with --keepdb:

• Before, 50 runs of the unchanged test: 47 passed, 3 failed with OperationalError: deadlock detected in the writer.
• After, 50 runs of both tag-writer tests: 50 passed, 0 failed.
• unittests.test_dedupe_delete_commit_race: 9 tests OK, with DD_V3_FEATURE_LOCATIONS both off and on.
• ruff check --config ruff.toml with ruff 0.16.5 (the pinned version) passes.

Documentation

Test only, no user-facing change.

🤖 Generated with Claude Code