cherry-pick: replay conflict retry on a fresh transaction after ERR_TRY_AGAIN (#1696) (→ v5.1) - #1852
Conversation
…TRY_AGAIN (#1696) * fix(storage): replay conflict retry on a fresh transaction after ERR_TRY_AGAIN Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): keep retries from deduping against their own audit entry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: log the swallowed abort error (review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): per-write sticky own-audit-entry marker for retry dedup (review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): abort before the MAX_RETRIES throw, pin change-feed entries in tests (review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Kris Zyp <kriszyp@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to replay writes onto a fresh transaction when a commit fails with ERR_TRY_AGAIN (e.g., due to a memtable flush stranding the snapshot), preventing infinite retry loops. It also tracks whether a write has already appended its audit entry in a prior attempt to avoid incorrect deduplication during replay. The feedback suggests using optional chaining (?.) when accessing store.store on the found write operation in DatabaseTransaction.ts to prevent potential runtime TypeError exceptions.
| // transaction object, which abort() leaves intact); isRetry in save() keeps the log | ||
| // entries themselves from being re-added. | ||
| const retryTransaction: RocksTransactionWithRetry = new RocksTransaction( | ||
| (this.writes.find((write) => write)?.store.store ?? this.db.store) as RocksStore |
There was a problem hiding this comment.
To prevent potential runtime TypeError exceptions if write.store is undefined or null, use optional chaining (?.) when accessing store.store on the found write operation. This aligns with defensive programming best practices.
| (this.writes.find((write) => write)?.store.store ?? this.db.store) as RocksStore | |
| (this.writes.find((write) => write)?.store?.store ?? this.db.store) as RocksStore |
References
- When accessing properties of a result from a method that might return nullish, guard against nullish values to prevent runtime errors.
2 participants
Cherry-pick of #1696 — "fix(storage): replay conflict retry on a fresh transaction after ERR_TRY_AGAIN" — onto the 5.1.x patch line, per Kris's request.
Apply
f10eba31). 3 files, +295 / −2, includes thesourceApplyConflictRetry.test.jsunit test.Before merge
v5.1(validates the change against the 5.1.x base).v5.1is unprotected — hold for Kris to merge, then cut 5.1.22.🤖 Cherry-pick prepared by Claude (Opus 4.8) on Kris's behalf.