Skip to content

Fix uncertain idempotency on transaction should retry without backoff - #127

Open
benh wants to merge 12 commits into
mainfrom
fix-uncertain-idempotency-on-transaction-should-retry-without-backoff
Open

Fix uncertain idempotency on transaction should retry without backoff#127
benh wants to merge 12 commits into
mainfrom
fix-uncertain-idempotency-on-transaction-should-retry-without-backoff

Conversation

@benh

@benh benh commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Bugs reported from @mtrifiro.

See each commit.

@benh
benh requested a review from onelxj August 14, 2026 06:46
@aviator-app

aviator-app Bot commented Aug 14, 2026

Copy link
Copy Markdown

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This pull request is currently open (not queued).

How to merge

To merge this PR, comment /aviator merge or add the mergequeue-ready label.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@benh benh self-assigned this Aug 14, 2026
Comment thread tests/reboot/pydantic/methods/test.py
onelxj added 2 commits August 14, 2026 13:57
There is a race condition in database between the
transaction lookup and the transaction creation, so
if 2 threads running shared transactions will both
pass the lookup and have no result, then they'll
try to create and only one will win and second
will crash the DB server with `CHECK(inserted)`.
Comment thread reboot/aio/contexts.py Outdated
Comment thread reboot/std/collections/ordered_map/v1/ordered_map.py Outdated
Comment thread reboot/std/collections/ordered_map/v1/ordered_map.py Outdated
Comment thread reboot/std/collections/ordered_map/v1/ordered_map.py Outdated
Comment thread reboot/std/collections/ordered_map/v1/ordered_map.py Outdated
@benh
benh force-pushed the fix-uncertain-idempotency-on-transaction-should-retry-without-backoff branch from 8140285 to d7b2354 Compare August 14, 2026 21:15
@github-actions

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@aviator-app

aviator-app Bot commented Aug 14, 2026

Copy link
Copy Markdown

This pull request failed to merge: some required checks failed. After you have resolved the problem, you should remove the blocked pull request label from this PR and then try to re-queue the PR.

Failed checks: Build public repo for Linux x86_64 / GitHub-hosted Runner

benh added 10 commits August 15, 2026 00:53
Don't treat `TransactionShouldRetryWithoutBackoff` as an uncertain
mutation within an `IdempotencyManager` since it is not uncertain and
it may cause the entire transaction to later abort due to that
uncertainty rather than just letting
`TransactionShouldRetryWithoutBackoff` propagate.
If a developer caught an error that is unrecoverable don't let them
make more Reboot method calls which is just wasting resources since
the whole transaction needs to abort anyway.
We need to correctly handle multiple calls to
`IdempotencyManager.idempotently()` which may create uncertain
mutations that we want to properly handle.
@benh
benh force-pushed the fix-uncertain-idempotency-on-transaction-should-retry-without-backoff branch from d7b2354 to bfd65ea Compare August 15, 2026 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants