[#808] fix MS SQL MERGE upsert conversion deadlock with WITH (HOLDLOCK, UPDLOCK) - #810
Merged
vharseko merged 1 commit intoAug 3, 2026
Conversation
…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
maximthomas
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The MS SQL upsert (
Storage.java:392) is aMERGE ... WITH (HOLDLOCK), added in #747/#750 to stop two concurrentWHEN NOT MATCHEDinserts of the same key racing into aPRIMARY KEYviolation.HOLDLOCKisSERIALIZABLE: the search phase of theMERGEtakes a shared lock and, unlike underREAD COMMITTED, holds it until the transaction ends. TheWHEN MATCHEDbranch 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 whattest_issue_496_2hit in run 30627712217 (8 threads × 1023 write transactions, all on the same key"key"; the MS SQL DDL isprimary key(h), so every writer contends for one row).Adding
UPDLOCKmakes 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.HOLDLOCKstays, 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 / SQLState40001, PostgreSQL40P01), even though the transaction is already rolled back at that point andPDBStorage.write()already retriesRollbackExceptionthe same way.UPDLOCKremoves 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 sameoldValueand both write — a lost update, against theWriteableTransaction.update()contract ("Atomically adds, deletes, or replaces a record").cassandra/Storage.javahas the same defect.Fixes #808