Retry a colliding node_id instead of 500ing - #302
Merged
Merged
Conversation
node_id is the first 8 characters of a uuid4 — 32 bits — and the column is
unique across EVERY org, not per-org. So a collision is not a per-tenant
curiosity: it is one customer's new node landing on an id another customer
already holds. The code drew once and committed, so the unique constraint
surfaced as an unhandled IntegrityError, i.e. a 500 at the worst possible
moment — someone adding their first node.
Odds of a single creation colliding, by fleet size:
1k nodes 1 in 4,294,967
10k nodes 1 in 429,496
100k nodes 1 in 42,949
Rare per request, certain enough in aggregate, and free to handle.
Retries rather than widening the id, because 8 hex characters is a
deliberate UX choice: the operator types it into the installer. Three
draws, then a clean 503 — three collisions in a row is not bad luck at any
plausible fleet size, it means something else is wrong (a duplicated uuid
source, or a constraint firing on a different column), and a 503 says
"try again" where a 500 says "we crashed".
Lets the unique constraint be the arbiter rather than a pre-check SELECT,
which would race two concurrent creates.
Two tests, both verified to fail without the retry: one forces a single
collision and asserts the second draw succeeds with a different id, the
other collides forever and asserts 503 rather than an unhandled error.
The uuid shim in those tests replaces nodes.py's OWN module reference
rather than mutating the real uuid module — request_context.py also calls
uuid4() and wants a genuine UUID with .hex, which is how the first version
of the test broke.
874 backend tests pass, ruff clean.
Co-Authored-By: Claude Opus 5 <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
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.
node_idis the first 8 characters of auuid4— 32 bits — and the columnis
unique=Trueacross every org, not per-org. So a collision isn't aper-tenant curiosity: it's one customer's new node landing on an id another
customer already holds.
The code drew once and committed, so the unique constraint surfaced as an
unhandled
IntegrityError→ 500, at the worst possible moment: someoneadding their first node.
Odds
Rare per request, certain enough in aggregate, and free to handle.
Why retry rather than widen the id
8 hex characters is a deliberate UX choice — the operator types it into the
installer, and CameraNode validates exactly that shape
(
setup/mod.rs:199: 8 chars, all hex). Widening it would push that cost ontoevery install to avoid a once-in-a-fleet event.
Three draws, then a clean 503. Three collisions in a row isn't bad luck at
any plausible fleet size — it means something else is wrong (a duplicated uuid
source, a constraint firing on a different column) — and a 503 says "try
again" where a 500 says "we crashed".
The unique constraint stays the arbiter rather than a pre-check
SELECT, whichwould race two concurrent creates.
Tests
Two, both verified to fail without the retry:
One forces a single collision and asserts the second draw succeeds with a
different id; the other collides forever and asserts 503 rather than an
unhandled error.
The uuid shim replaces
nodes.py's own module reference rather thanmutating the real
uuidmodule —request_context.pyalso callsuuid4()andwants a genuine UUID with
.hex, which is how my first version of the testbroke.
Verification
ruffclean🤖 Generated with Claude Code