Skip to content

[#808] fix MS SQL MERGE upsert conversion deadlock with WITH (HOLDLOCK, UPDLOCK) - #810

Merged
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/808-mssql-merge-updlock
Aug 3, 2026
Merged

[#808] fix MS SQL MERGE upsert conversion deadlock with WITH (HOLDLOCK, UPDLOCK)#810
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/808-mssql-merge-updlock

Conversation

@vharseko

Copy link
Copy Markdown
Member

The MS SQL upsert (Storage.java:392) is a MERGE ... WITH (HOLDLOCK), added in #747/#750 to stop two concurrent WHEN NOT MATCHED inserts of the same key racing into a PRIMARY KEY violation.

HOLDLOCK is SERIALIZABLE: the search phase of the MERGE takes a shared lock and, unlike under READ COMMITTED, holds it until the transaction ends. The WHEN MATCHED branch then has to convert that lock to an exclusive one. Two transactions that both hold S on the same key and both want X form a lock cycle, and SQL Server kills one of them with error 1205 — which is what test_issue_496_2 hit in run 30627712217 (8 threads × 1023 write transactions, all on the same key "key"; the MS SQL DDL is primary key(h), so every writer contends for one row).

Adding UPDLOCK makes the search phase take an update lock instead. Two update locks are incompatible with each other, so the second transaction waits rather than forming a cycle, and no S→X conversion is needed. HOLDLOCK stays, so the atomicity that #747 needed is preserved. MERGE ... WITH (HOLDLOCK, UPDLOCK) is the canonical SQL Server upsert idiom.

Only the MS SQL branch is affected; PostgreSQL (ON CONFLICT), MySQL (ON DUPLICATE KEY UPDATE) and Oracle (MERGE) are untouched.

Not fixed here, tracked separately in #808:

  • Storage.write() does not retry retriable serialization failures (MS SQL 1205 / SQLState 40001, PostgreSQL 40P01), even though the transaction is already rolled back at that point and PDBStorage.write() already retries RollbackException the same way. UPDLOCK removes the self-inflicted deadlock on a single key, but transactions touching several keys can still deadlock.
  • WriteableTransactionTransactionImpl.update() reads without a lock before writing, so two transactions can read the same oldValue and both write — a lost update, against the WriteableTransaction.update() contract ("Atomically adds, deletes, or replaces a record"). cassandra/Storage.java has the same defect.

Fixes #808

…k with WITH (HOLDLOCK, UPDLOCK)

HOLDLOCK alone makes the MERGE search phase take a shared lock that is held
to the end of the transaction; the WHEN MATCHED branch then has to convert it
to an exclusive lock, so two concurrent upserts of the same key deadlock on the
conversion and SQL Server kills one of them with error 1205. UPDLOCK takes an
update lock right away - two update locks are incompatible, so the second
transaction waits instead of forming a lock cycle - while keeping the atomicity
that WITH (HOLDLOCK) was added for in OpenIdentityPlatform#747.

Fixes OpenIdentityPlatform#808
@vharseko
vharseko requested a review from maximthomas July 31, 2026 16:37
@vharseko vharseko added bug concurrency Thread-safety / race-condition bugs jdbc java Pull requests that update java code tests Test suites: fixing, enabling, un-disabling labels Jul 31, 2026
@vharseko
vharseko merged commit 77bf727 into OpenIdentityPlatform:master Aug 3, 2026
17 checks passed
@vharseko
vharseko deleted the issues/808-mssql-merge-updlock branch August 3, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug concurrency Thread-safety / race-condition bugs java Pull requests that update java code jdbc tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: test_issue_496_2 deadlocks on MS SQL (MERGE WITH (HOLDLOCK) conversion deadlock)

2 participants