Skip to content

fix(postgres): lock missing items that TransactWriteItems checks or deletes - #387

Open
yesyayen wants to merge 14 commits into
ExtendDB:mainfrom
yesyayen:fix/twi-absent-key-skew
Open

yesyayen wants to merge 14 commits into
ExtendDB:mainfrom
yesyayen:fix/twi-absent-key-skew

Conversation

@yesyayen

@yesyayen yesyayen commented Oct 5, 2026

Copy link
Copy Markdown
Member

What

On PostgreSQL, two TransactWriteItems that each check or delete an item the other one creates could both commit. A ConditionCheck or Delete that found no row took no lock, so nothing stopped a concurrent create while the transaction still relied on the item being absent.

  • reserve_missing_item() in crates/storage-postgres/src/data/transactions.rs runs when the ConditionCheck or Delete arm's SELECT ... FOR UPDATE finds no row. It inserts a key-only row with ON CONFLICT DO NOTHING and deletes it again in the same transaction. The key's unique index entry stays with the open transaction, so any insert of the key waits for it to end: TransactWriteItems, PutItem, UpdateItem and BatchWriteItem alike. No other transaction ever sees the row, and no stream record or index row comes from it.
  • If the insert loses to a concurrent create, the op re-reads FOR UPDATE and uses the committed item: the check is evaluated against it, and the Delete removes it.
  • If the placeholder delete removes anything other than one row, the transaction fails with an internal error instead of committing a key-only item.
  • Put and Update need nothing new: their own insert already holds the key.
  • Docs: the storage extension guide no longer says the PostgreSQL backend uses SERIALIZABLE. It, differences-from-dynamodb.md, the architecture guide, and the storage design now describe the key reservation.

This branch is stacked on #385: the first 6 commits are #385, and this PR adds the last 7.

SQLite runs one writer at a time, so it does not have this bug. MongoDB does, and a separate PR fixes it there.

Why

Found while checking TransactWriteItems isolation on PostgreSQL. Measured on 2026-10-05 on the head of #385: 100 rounds of the first shape and 40 of the others on PostgreSQL, 60 rounds per shape on Amazon DynamoDB.

Race Amazon DynamoDB PostgreSQL before this PR
[CC y absent, Put x] vs [CC x absent, Put y] never both commit both commit, 100 of 100
[Delete y if absent, Put x] vs [Delete x if absent, Put y] never both commit both commit, 40 of 40
[Delete y, Put x] vs [Delete x, Put y] both commit only when one item remains both commit and both items remain, 40 of 40
[Update x if absent, Put y] vs [Update y if absent, Put x] never both commit never both commit

The loser on Amazon DynamoDB gets TransactionConflict ("Transaction is ongoing for the item") or ConditionalCheckFailed.

Fixes: n/a, found by a concurrency probe, no issue filed

Result

Testing done

  • tests/test_transact_absent_item_isolation.py (new, dual-target): two transactions that each read an item the other one creates run at the same time, 40 rounds per shape: ConditionCheck on a hash table and on a table with a number sort key, conditional Delete, unconditional Delete, and conditional Update of a missing item. Every outcome is checked against the two serial orders, every failure must be a cancellation, and a run may see at most 5 HTTP 500s.
  • crates/storage-postgres/tests/twi_conflict.rs (4 new tests, need EXTENDDB_TEST_PG_CONNECTION_STRING): an outside transaction holds an uncommitted insert of the item. A ConditionCheck must wait for it and then fail, and a Delete must wait and then remove the new item. A non-transactional PutItem of a key that a transaction checked as missing must wait for that transaction. A Delete that loses its reservation must remove the winner from the table and its LSI, and write one REMOVE stream record. The tests wait on lock-waiter counts, not on sleeps.
  • Transaction, condition, batch, stream and GSI pytest suites (14 files) against a server built from this branch: no new failures. 2 GSI propagation-delay and 2 idempotency-scope tests also fail on fix(postgres): lock TransactWriteItems items in key order, cancel on deadlock #385.
cargo fmt --all -- --check
cargo clippy --all-targets -- -D warnings
cargo test --release --workspace                                        # 1,234 passed
cargo test --release -p extenddb-storage-postgres --test twi_conflict   # 8 of 8 in 10 of 10 runs, live PostgreSQL

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: n/a. Isolation inside one backend's write transaction; no wire, trait, auth, on-disk, or CLI change.

Merge order: after #385, which this branch is stacked on, and after the MongoDB fix for the same race, without which the new test fails on MongoDB.

Breaking changes

None.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

…g them

Two TransactWriteItems that name the same items in different orders could deadlock in PostgreSQL. The victim came back as HTTP 500, and each deadlock held its row locks for deadlock_timeout first.

Run the ops in table and key order, so every write transaction takes its row locks in one order and two of them cannot deadlock on each other. The results keep their request positions.

When PostgreSQL still aborts the transaction with deadlock_detected (40P01) or serialization_failure (40001), cancel it with a TransactionConflict reason on the item that hit the abort, the shape Amazon DynamoDB returns. An abort outside the per-item work names every item. To read the SQLSTATE, the transaction helpers now map database errors through db_error.

Assisted-by: pi claude-opus-5-5
… known

With the ops running in key order, a request whose earliest invalid op is known no longer runs and locks the rest. A storage test pins that the earliest invalid op in request order is still the one reported, and CI now runs the conflict storage tests against its PostgreSQL.

Assisted-by: pi claude-opus-5-5
Bound the tolerated InternalServerError count to a fixed 5 per run instead of a share of the cancellations, so a server that cancels a lot cannot hide a 5xx rate far above the service's.

Assisted-by: pi claude-opus-5-5
Add the contention difference to differences-from-dynamodb.md and the lock order rule for blocking-lock backends to the storage extension guide.

Assisted-by: pi claude-opus-5-5
…ocks

A storage test holds the next op's row from outside the transaction and expects the ValidationException within 5 s. The pytest docstring now says which test can deadlock, and the file ends with one newline.

Assisted-by: pi claude-opus-5-5
Two transactions that each read an item the other creates must not both commit, as on Amazon DynamoDB. The tests cover ConditionCheck, conditional and unconditional Delete, and conditional Update on missing items.

Assisted-by: pi claude-opus-5-5
A ConditionCheck or Delete that found no row took no lock, so a concurrent create could commit while the transaction still relied on the item being absent. Two transactions that each checked an item the other created both committed (write skew).

The op now inserts the missing key and deletes it again in the same transaction. The key's unique index entry stays with the open transaction, so any insert of the key, transactional or not, waits for it to end. No other transaction ever sees the row.

Assisted-by: pi claude-opus-5-5
Replace the claim that the PostgreSQL backend uses SERIALIZABLE with what it does, and add the missing-item rule for backend authors.

Assisted-by: pi claude-opus-5-5
…leted

If the delete of the placeholder row removed no row, the transaction would commit a key-only item. It cannot happen today, because both statements bind the same key, so the op now returns an internal error instead of a silent ghost item. The comments cite the PostgreSQL page on unique-index waits and say when the create-race bound applies.

Assisted-by: pi claude-opus-5-5
The pytest races ConditionChecks of missing items on a table with a number sort key. A storage test holds a create in flight on an LSI and stream table, and checks that a Delete of the missing item waits for it, then removes the winner with one REMOVE record and no index row left.

Assisted-by: pi claude-opus-5-5
The differences page says that a single-item write waits for a write transaction that holds the item, including a missing item it checks or deletes, and that contention on one missing key is less fair than on an existing row. The architecture guide and the storage design name the key reservation.

Assisted-by: pi claude-opus-5-5
The differences page and the storage design said that a write transaction holds a missing item's key until it commits. A rollback releases the key too.

Assisted-by: pi claude-opus-5-5
MongoDB has the same write skew, and the MongoDB write-race fix repairs it. That fix lands as its own PR. Until it is on main, the four affected tests are marked xfail when the MongoDB test runner runs them. The marker is not strict, so the tests also pass once the fix is in. Remove the marker then.

Assisted-by: pi claude-opus-5-5
@yesyayen
yesyayen marked this pull request as ready for review October 5, 2026 22:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant