Skip to content

fix(mongodb): retry an unconditional PutItem that loses the create race - #431

Open
LeeroyHannigan wants to merge 8 commits into
mainfrom
fix/mongodb-upsert-duplicate-key
Open

fix(mongodb): retry an unconditional PutItem that loses the create race#431
LeeroyHannigan wants to merge 8 commits into
mainfrom
fix/mongodb-upsert-duplicate-key

Conversation

@LeeroyHannigan

@LeeroyHannigan LeeroyHannigan commented Oct 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

On the MongoDB backend, two unconditional PutItem calls that create the same item at the same time could both find no document and both insert it. The second insert failed on the _id_ unique index with a duplicate-key error, and ExtendDB answered that put with HTTP 500 InternalServerError. The sessionless PutItem path now retries a lost create race under the existing 50-attempt transient retry budget with backoff, and the next attempt replaces the item the other put created. Inside a snapshot transaction MongoDB reports the same collision as a write conflict, which the transaction path already retried.

Closes #356.

What the service does

Measured on 2026-10-21 against Amazon DynamoDB in us-east-1, client retries off, 32 writers x 63 rounds (2,016 calls) per cell unless stated:

  • Racing unconditional PutItem on a new key per round, hash, GSI and stream tables: 2,016 of 2,016 answered 200 in each cell. On one key for 63 rounds, GSI and stream tables: 2,016 of 2,016.
  • PutItem racing DeleteItem, UpdateItem and BatchWriteItem on one key: every call 200. One putter per three deleters, 64 writers x 60 rounds: 960 puts and 2,880 deletes, all 200.
  • One-key churn, half the writers PutItem and half DeleteItem: seven runs of 64 x 100 on a hash table and three runs of 32 x 200 on a GSI table, 32,000 puts and 32,000 deletes, 0 errors.
  • In every cell the ALL_OLD images, the final item, the GSI rows and the stream records form one serial history.
  • UpdateItem without ReturnValues creating 30 items, then PutItem on each: 0 items without the key attribute, 30 puts answered 200.

What ExtendDB did on MongoDB

The data collection has two unique indexes, _id_ and one on the key fields. MongoDB retries an upsert that hits a duplicate key only when the filter is an equality on the fields of the index that raised the error; the put filters on pk (and the sort key field) and the error names _id_, so the server did not retry. The sessionless path mapped the error to an internal error. On a 16-core host tests/test_put_item_create_race.py failed 2 or 3 of 9 cases per run with 1 of 8 puts failed: [('error', 'InternalServerError')]; 32 writers on a new key per round got 3 InternalServerError in 2,016 puts; one-key put and delete churn from 8 and 32 threads got 66 and 45 in 2,016 puts.

On the transaction path, which GSI and stream tables use, a race between two puts doesn't raise the duplicate-key error. Inside a snapshot transaction MongoDB 7.0 reports the collision as WriteConflict (code 112, WiredTigerIdIndex::_insert: index: _id_) with the TransientTransactionError label, whether the other put's transaction is still open or committed after this one's snapshot, and TxErr::from already retried it.

Changes

  • crates/storage-mongodb/src/data_engine.rs: the sessionless unconditional PutItem retries a duplicate-key error for up to TRANSIENT_RETRY_ATTEMPTS (50) attempts with backoff_sleep between them. On the transaction path a duplicate-key error returns TxErr::Transient, so it shares the 50-attempt budget and backoff of a write conflict; that arm is a defence, reached only when a document in the transaction's snapshot holds the item's _id without matching the key filter. A put whose 50 attempts all lost the race returns the internal error the PostgreSQL backend returns in that case, with the attempt count in the message; is_duplicate_key matches the command form findAndModify raises as well as the write form; the fast-path comment describes both unique indexes and the measured index.
  • crates/storage-mongodb/src/data_engine_live_tests_put_race.rs, registered in data_engine_live_tests.rs: two live storage crate tests (sessionless and transaction path each). In one, every attempt loses the race and the put gives up after 50 attempts. In the other, one attempt loses the race, then the obstruction is replaced by another put's item or deleted, and the put succeeds with the right old image and stored item. They are part of main's data_engine::live_tests module, are #[ignore]d, read EXTENDDB_TEST_MONGODB_URI through its uri(), and run in the mongodb-rust job through devtools/run-mongodb-tests --rust-live.
  • tests/test_put_item_create_race.py: the MongoDB xfail marker on the three plain-table cases is removed, and each writer thread has its own client.

Verification

The table and the negative controls below are from the first round, before the merge with main. They ran on MongoDB 7.0 as a single-node replica set; fix is this branch before the merge and main is ec81e9d.

check service MongoDB main MongoDB fix
tests/test_put_item_create_race.py, 9 cases passes (dual-target file) 2 or 3 of 9 fail per run (--runxfail) 9 passed in each of 16 runs
racing puts, new key per round, hash / GSI / stream, 2,016 each 2016 / 2016 / 2016 ok hash 3 InternalServerError 2016 / 2016 / 2016 ok, 0 violations
one-key put and delete churn, hash 22,400 puts, 22,400 deletes, 0 errors 66 and 45 InternalServerError per 2,016 puts 44,800 puts, 44,800 deletes, 0 errors; 4,663 lost races retried
put-delete, put-update, batch, put-delete3 cells on hash tables all 200, 0 violations not run all 200, 0 violations
one-key cells on GSI and stream tables all 200 TransactionConflictException on 11 to 23 percent of puts 5 to 284 TransactionConflictException per cell, 0 InternalServerError, 0 violations (issue 426, unchanged)
storage crate: unconditional_put_retries_a_lost_create_race_and_then_gives_up n/a fails (raw E11000 message) passes
storage crate: unconditional_put_that_lost_a_create_race_succeeds_on_a_later_attempt n/a fails (InternalServerError) passes
regression files on the fix server: test_put_item_create_race, test_item_operations, test_batch_operations, test_transaction_operations, test_conditional_operator n/a n/a 243 passed
full wire suite on the fix branch (first round) n/a n/a 1024 passed; 32 non-passing cases are environmental and fail the same way on main

Negative controls, each restored with git checkout afterwards:

mutation result
sessionless retry arm disabled race test 3 of 9 fail with InternalServerError (twice); Rust give-up test fails
transaction retry arm disabled Rust give-up test fails at the transaction table
is_duplicate_key command form disabled both Rust tests fail
backoff_sleep removed from the sessionless retry lost-race-then-success test fails: could not create the item after 50 attempts (3 of 3 runs)
sessionless budget set to 5 lost-race-then-success test fails
cap of 5 lost races put back on the transaction arm both Rust tests fail
transaction give-up error disabled give-up test fails with TransactionConflictException
transaction arm gives up at the first lost race both Rust tests fail

After the merge with main at 5ff323b:

  • cargo +1.97.0 fmt --all -- --check is clean, and cargo +1.97.0 clippy -p extenddb-storage-mongodb --all-targets -- -D warnings is clean with and without --features test-hooks.
  • cargo +1.88.0 test --locked -p extenddb-storage-mongodb: 243 passed, 46 ignored.
  • Against MongoDB 7.0.45, the 18 tests in data_engine::live_tests pass, and tests/test_put_item_create_race.py passed 9 of 9 in each of 3 runs.
  • With the transaction-path or the sessionless retry arm removed, both create-race live tests fail.

No dependency change.

Not in this change

Measured on main ec81e9d and the service on 2026-10-21, except where an item names another commit:

  1. UpdateItem without ReturnValues that created an item on a plain table stored it without its key attribute (GetItem returned {"w": {"S": "upd"}}), and every later PutItem on that key answered 500 (ImmutableField on _id). The review of this PR found that conditional PutItem, UpdateItem with ALL_OLD, TransactWriteItems Put and BatchWriteItem on that item answered 500 as well. Service: 0 items without the key attribute, 30 later puts 200. Fixed on main by fix(mongodb): store the key when UpdateItem creates an item #473 (MongoDB: UpdateItem that creates an item stores it without its key attribute, and every later PutItem on that key answers 500 #436), which the merge brings into this branch. On main 5ff323b, after an UpdateItem without ReturnValues creates the item, GetItem returns the key and all five of those writes answer 200.
  2. Stream tables: a write conflict on the stream sequence counter answers 500. BatchWriteItem (raced key plus a side key) racing PutItem, 16 writers x 30 rounds, on main: 40 of 240 BatchWriteItem and 5 of 240 PutItem 500, 45 WriteConflict (code 112) log lines; 16 puts returned as old image the item of a batch that answered 500. Fix server: 46 and 7 of 240. stream_engine::next_sequence_number_in_session.
  3. UpdateItem with ALL_OLD under one-key contention answers TransactionConflictException (fix server: hash 3, GSI 168 to 193, stream 221 to 306 of 1,008); the service answers 200. Issue 426 names PutItem only.
  4. Stream sequence numbers repeat across the shards of one stream (each value once per shard). Not compared with the service.
  5. A conditional UpdateItem that creates the item on a table without GSIs or streams (update_item_pushdown) finds no item, then writes it with an unconditional replace_one upsert outside a session. Racing attribute_not_exists(pk) creates on one key can each answer 200 with an empty ALL_OLD while all but one write is replaced, or answer 500 on the _id_ duplicate. On main 5ff323b, 32 writers x 50 rounds on fresh keys: 17 rounds with more than one 200, and 5 InternalServerError. Filed as MongoDB: concurrent conditional UpdateItem creates on one key can each answer 200, and all but one write is lost #790.

Refs #577: the fifty-writer test_all_writers_succeed failure on the MongoDB job is probably this create race. Close #577 only after that test passes there with this merged.

@LeeroyHannigan
fix(mongodb): retry an unconditional PutItem that loses the create race
c3f8bc8 
Amazon DynamoDB applies concurrent PutItem calls on one key one after
another. A call without a condition succeeds, the last writer's item is
stored, and with ReturnValues ALL_OLD each call returns the item it
replaced. Measured on 2026-10-21 against the service in us-east-1 with
client retries off: 32 writers x 63 rounds of racing unconditional puts
on a new key per round answered 2,016 of 2,016 with 200 on hash, GSI and
stream tables; 32 writers on one key for 63 rounds answered 2,016 of
2,016 on GSI and stream tables; one key with half the writers sending
PutItem and half DeleteItem answered 22,400 puts and 22,400 deletes with
200 and no error (seven runs of 64 writers x 100 operations on a hash
table, three runs of 32 x 200 on a GSI table); 32 writers racing PutItem
against DeleteItem, UpdateItem and BatchWriteItem on one key answered
every call with 200; and 64 writers with one putter per three deleters on
one key answered 960 puts and 2,880 deletes with 200. In every cell the
ALL_OLD images, the final item, the index and the stream records formed
one serial history. tests/test_put_item_create_race.py runs against the
service as well as against ExtendDB and checks the race on 8 clients.

On MongoDB, two puts that create the same item could each find no
document and each insert it. The second insert fails with a duplicate-key
error (E11000), which findAndModify reports as a command error. The data
collection has two unique indexes: _id_, and one on the key fields (pk,
plus the sort key field on a table with a sort key). MongoDB retries such
an upsert itself only when the filter is an equality on the fields of the
index that raised the error. The put filters on pk and, on a table with a
sort key, the sort key field, and every measured duplicate-key error names
_id_, so the server does not retry it. The sessionless path (no condition,
no index, no stream) returned the error as an internal error (HTTP 500),
and the transaction path (a table with an index or a stream) classified it
as fatal, which also returned an internal error. On a 16-core host the
race test failed 2 or 3 of its 9 cases per run, each failure a racing put
answering InternalServerError; 32 writers creating a new key per round
got 3 InternalServerError responses in 2,016 puts; and one-key put and
delete churn from 8 and 32 threads got 66 and 45 in 2,016 puts.

Both paths now retry the put under the transient retry budget of 50
attempts with the same backoff the write-conflict retry uses. The next
attempt finds the item the other put created, replaces it and returns it
as the old image, so the ALL_OLD images form one chain; if a delete
removed that item first, the next attempt inserts. On the transaction
path the index update and the stream record take that item as the old
image too. A put whose 50 attempts all lost the create race returns the
internal error the PostgreSQL backend returns when its own retries of
this race run out, with the attempt count in the message.
is_duplicate_key now recognizes the command form as well as the write
form that insert_one returns.

The MongoDB xfail marker on the plain-table race tests is removed.
devtools/run-mongodb-tests exports EXTENDDB_TEST_MONGODB_URI, and two
storage crate tests use it, on the sessionless path and on the
transaction path each: one forces every attempt to lose the race with a
document that holds the item's _id under another pk and checks the
give-up error after 50 attempts and that the document is unchanged; the
other replaces or deletes that document 100 ms after the put starts and
checks that the put succeeds, returns the other put's item or no old
image, and stores its own item. Without the URI the tests print a SKIP
line; with EXTENDDB_TEST_MONGODB_CONTAINER set and no URI they fail. The
scratch databases are dropped when a test fails.

Verified against MongoDB 7.0 as a single-node replica set. The race test
passes 9 of 9 cases in each of 16 runs. The same cells as the service
measurement answered every unconditional PutItem, DeleteItem and
BatchWriteItem with 200 and no invariant violation; one-key put and
delete churn answered 44,800 puts and 44,800 deletes with 200 (ten runs
of 16 x 400 and four of 64 x 100) while 4,663 puts lost a create race
and succeeded on a later attempt. With the backoff removed from the
sessionless retry the lost-race-then-success test fails after 50 attempts;
with a cap of 5 lost races put back on either path, with the transaction
give-up error disabled, or with the transaction arm giving up at the first
lost race, the storage crate tests fail; with the sessionless retry
disabled the race test fails its 3 plain-table cases. One-key contention
on GSI and stream tables still answers TransactionConflictException for
some puts (issue 426), unchanged here.

Closes #356
This was referenced Oct 22, 2026
Open
Merged
Open
Open
Merged
Open
Open
robinnsc
robinnsc reviewed Oct 31, 2026
View reviewed changes

@robinnsc robinnsc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix does what it says for the fast path. Running against main's build on a local MongoDB 7, racing unconditional PutItem answered InternalServerError: 30 of 1,008 and 37 of 1,600 puts in one-key put/delete churn, with every duplicate key naming _id_. Head answered 0 across those cells and 8-64-writer variants on plain, range, GSI and stream tables. ALL_OLD chains, final items, GSI entries and stream histories were consistent, and conditional puts were never retried into a wrong 200. BatchWriteItem is covered too, since it goes through PutItem. test_put_item_create_race.py failed the same 3 of 9 on main in every run and passed 9 of 9 here. Uncontended latency is unchanged.

The merge with current main is the thing to fix before this goes in (first inline comment). Otherwise it's comments and notes.

Notes:

  • Pre-existing, not this PR: a conditional UpdateItem that creates the item (update_item_pushdown, data_engine.rs:2959) reads, then runs an unconditional replace_one upsert outside a session. With 32 writers sending attribute_not_exists(pk) creates on one fresh key, two of them both answered 200 with an empty ALL_OLD, and one write was silently replaced. Some rounds also answered 500 on the _id_ duplicate. That happens on main and on this branch, and I couldn't find an issue for it. Probably worth its own, since attribute_not_exists creates are how people enforce uniqueness. The description's "Not in this change" item 1 could also say that the item the UpdateItem fast path creates breaks more than PutItem: conditional PutItem, UpdateItem with ALL_OLD, TransactWriteItems Put and BatchWriteItem all answered 500 on it.
  • With the URI exported, main's snapshot_engine live tests also run in the mongodb-rust job (about a minute, and they pass). The comment at integration-mongodb.yml:56 saying mongodb-snapshot is the job that exports it would then be out of date. Passing the URI inline to these tests, like main does for records_live, would avoid both.
  • test_put_item_create_race.py shares one boto3 client across its 8 threads. The xfail used to absorb a TLS hiccup from that, and now it would fail the job.
  • Small: is_duplicate_key here now matches both forms, management_store.rs:18 only the write form, and a couple of other places string-match E11000. That's fine today, just more than one copy of the rule.

Comment thread crates/storage-mongodb/src/data_engine.rs Resolved
Comment thread crates/storage-mongodb/src/data_engine.rs Resolved
@navareamol navareamol added this to the 1.0 milestone Nov 2, 2026
@navareamol navareamol added the v1 Needed for the 1.0 release label Nov 3, 2026
@LeeroyHannigan
Merge branch 'main' into fix/mongodb-upsert-duplicate-key
2b799c5
@LeeroyHannigan
test(mongodb): run the create-race tests as live data engine tests
09d6bf9 
Main added a live test module for the data engine in #473: its tests are
#[ignore]d, read EXTENDDB_TEST_MONGODB_URI through one uri() helper, fail
rather than skip when a run passes --ignored without it, and run in the
mongodb-rust job through devtools/run-mongodb-tests --rust-live. Its
scratch_engine helper builds the engine with MongoEngine::new and struct
update syntax, so a field added to MongoEngine needs no change there.

The two create-race storage tests move into that module as
data_engine_live_tests_put_race.rs. They used their own URI gate, which
printed SKIP without the variable, and a with_scratch_engine helper that
built MongoEngine from a struct literal. Against main that literal missed
the four fields #384 added, and the test build failed with E0063. Both
helpers are gone, and the tests check what they checked before.

devtools/run-mongodb-tests exported EXTENDDB_TEST_MONGODB_URI a second
time for these tests. Main exports it since #473, so the line is removed.
@LeeroyHannigan
docs(mongodb): say a create race in a transaction is a write conflict
0d3f76d 
The comment on the duplicate-key arm of the transaction path said the arm
handles a put that lost the create race to another put. It does not. On
MongoDB 7.0, two snapshot transactions that upsert the same _id fail the
second upsert with WriteConflict (code 112, "WiredTigerIdIndex::_insert:
index: _id_") and the TransientTransactionError label, both while the
first transaction is open and after it committed past the second one's
snapshot. TxErr::from already retried that error before this branch.

The comment now says the arm is a defence: it is reached only when a
document in the transaction's snapshot holds the item's _id without
matching the key filter, which is the document the storage tests insert.
@LeeroyHannigan
test(put-item): give each create race writer its own client
08c4995 
test_put_item_create_race.py shared one boto3 client across its 8 writer
threads. A botocore client used from several threads at once can fail a
request with a TLS error, and with retries off and the MongoDB xfail marker
gone, that error fails the test. Each writer now has its own client.
@LeeroyHannigan
docs(mongodb): describe the duplicate-key arm the same way everywhere
b7c9258 
0d3f76d rewrote the comment on the transaction path's duplicate-key arm:
a put that loses the create race inside a snapshot transaction gets
WriteConflict 112, so the arm only sees a document that holds the item's
_id without matching the key filter. Two comments in the same function and
the doc of create_race_exhausted still described the arm as the create
race, and now say what it is.

The create race test's client fixture no longer explains why each writer
has its own client; the docstring states what the fixture gives.
@LeeroyHannigan LeeroyHannigan mentioned this pull request Nov 3, 2026
Open
@LeeroyHannigan

LeeroyHannigan commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

I merged main at 5ff323b in 2b799c5. The merge had no conflicts. 09d6bf9 fixes the E0063 in the test build from the first inline comment by moving the create-race tests into main's live test module.

On the merged branch, fmt is clean, clippy with --all-targets -D warnings is clean on extenddb-storage-mongodb with and without test-hooks, and its unit tests pass (243 passed, 46 ignored). The 18 tests in data_engine::live_tests pass against MongoDB 7.0.45, and with the transaction-path or the sessionless retry arm removed, both create-race live tests fail.

@LeeroyHannigan LeeroyHannigan mentioned this pull request Nov 3, 2026
Open
LeeroyHannigan added a commit that referenced this pull request Nov 3, 2026
@LeeroyHannigan
test(mongodb): build every test engine through one constructor
9212328 
Merged with main, the storage-mongodb test build failed with E0063: the
MongoEngine literals in stream_records_live_tests.rs (from #543) and in
snapshot_engine.rs (from #395) do not set contention_deadline. Each test
that needed an engine listed every field, so each field added to
MongoEngine broke every such literal.

MongoEngine::new now builds the engine in from_parts, and a test-only
MongoEngine::for_test calls the same function with the driver's default
pool of 10 connections. The test literals in backup_live_tests.rs,
stream_records_live_tests.rs, snapshot_engine.rs and the restore backfill
test now call for_test and set only the fields they change with struct
update syntax. A new field is set in from_parts and nowhere else.

The four contention storage tests move from the data_engine test module
into data_engine_live_tests_contention.rs, next to the create-race tests
#431 moved. They build the engine through the live test module's
scratch_engine, are #[ignore]d like the other live tests and run under
devtools/run-mongodb-tests --rust-live, and set the deadline with struct
update syntax. The with_scratch_engine helpers and the second URI gate go
with them. What each test checks is unchanged.
LeeroyHannigan added a commit that referenced this pull request Nov 3, 2026
@LeeroyHannigan
Merge branch 'fix/mongodb-upsert-duplicate-key' into fix/mongodb-puti…
63df25e 
…tem-gsi-conflict

Brings #431's b7c9258, which describes the transaction path's
duplicate-key arm as a document that holds the item's _id rather than as
the create race.

The one conflict is the PutItem transaction path's give-up after its loop.
This branch keeps its own version: the loop gives up with the create race
error once CREATE_RACE_ATTEMPTS attempts have met such a document, and a
spent contention budget answers the transient error. The comment on that
check and the doc of create_race_exhausted now use #431's wording, and the
doc says the transaction path gives up after CREATE_RACE_ATTEMPTS attempts
rather than after every attempt.
@LeeroyHannigan LeeroyHannigan mentioned this pull request Nov 3, 2026
Open
1 of 3 tasks
@LeeroyHannigan LeeroyHannigan mentioned this pull request Nov 3, 2026
Open

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

Reviewers

@amrith amrith Awaiting requested review from amrith amrith is a code owner
@c33howard c33howard Awaiting requested review from c33howard c33howard is a code owner
@jcshepherd jcshepherd Awaiting requested review from jcshepherd jcshepherd is a code owner
@pdf-amzn pdf-amzn Awaiting requested review from pdf-amzn pdf-amzn is a code owner
@yesyayen yesyayen Awaiting requested review from yesyayen
@eliseong eliseong Awaiting requested review from eliseong eliseong is a code owner
@hitesh96db hitesh96db Awaiting requested review from hitesh96db hitesh96db is a code owner
@navareamol navareamol Awaiting requested review from navareamol navareamol is a code owner
@robinnsc robinnsc Awaiting requested review from robinnsc robinnsc is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

v1 Needed for the 1.0 release

Projects

None yet

Milestone

1.0

3 participants

@LeeroyHannigan @robinnsc @navareamol