Fix persistent 500 by resyncing cards_count after failed card insert#10
Merged
Conversation
…action The original code called account.increment!(:cards_count) which does an atomic UPDATE followed by a separate SELECT (reload_attribute!). Under concurrent load two threads could each run the UPDATE before either ran the SELECT, causing both to read the same final counter value and then both try to INSERT a card with that number, hitting the unique index on (account_id, number). Wrapping increment! in Account.transaction ensures the UPDATE and the subsequent SELECT happen within the same SQLite write transaction. SQLite serialises writers, so no other thread can increment the counter between our UPDATE and our SELECT — each card creation gets a distinct number. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
When a card INSERT fails with RecordNotUnique, the entire Card.transaction rolls back — including the accounts.cards_count increment inside it. This leaves the counter behind the actual max card number. Every retry then tries the same (already-taken) number, rolls back again, and all three retries are exhausted, causing a persistent 500. On RecordNotUnique, before retrying, reload the account and advance cards_count to MAX(cards.number) if it has fallen behind. This update runs outside the failed transaction so it persists. The next retry then increments past the conflicting number and succeeds. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Root cause of the persistent 500
The previous fixes missed a second failure mode. When
card.update!raisesRecordNotUnique:Card.transactionrolls back — including theaccounts.cards_count += 1that happened insideassign_number.increment!from the same stale counter value, gets the same already-taken number, rolls back again, and exhausts all three retries → persistent 500.This explains why only a single request was failing (no concurrency needed) and why the retries didn't help.
Fix
In the
RecordNotUniquerescue block, before retrying, reload the account and advanceaccounts.cards_counttoMAX(cards.number)if it has fallen behind. This update runs outside the rolled-back transaction so it persists. The next retry then increments past the conflicting number and succeeds.The
Account.transactionwrapping inassign_numberis kept — it serialises concurrent writers so two threads can't get the same counter value in the first place.Test plan
bin/rails test test/models/card_test.rb test/controllers/cards/— 107 tests, all pass🤖 Generated with Claude Code