fix(transaction): retry ERR_TRY_AGAIN commits instead of dropping them silently - #1223
Merged
Conversation
…m silently RocksDB returns kTryAgain (rocksdb-js code ERR_TRY_AGAIN) when a transaction's snapshot sequence falls outside the memtable conflict-check window (max_write_buffer_size_to_maintain). This happens under bulk-ingest bursts — e.g. a v4→v5 migration full-table copy into a RocksDB receiver. The commit error handler only retried ERR_BUSY; ERR_TRY_AGAIN fell through to `throw`, and because the commit is driven from an unawaited onCommit().then() the rejection became an unhandled promise rejection in the worker — logged but the write was silently dropped, so copied tables froze short of their source counts with no error surfaced (harper-pro#308). Treat ERR_TRY_AGAIN like ERR_BUSY: retry with the existing backoff and, after MAX_RETRIES, throw a ServerError (loud) rather than losing the write. Both are transient, retryable conflict conditions. Refs harper-pro#308. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
|
Reviewed; no blockers found. |
cb1kenobi
approved these changes
Jun 10, 2026
heskew
approved these changes
Jun 10, 2026
kriszyp
added a commit
to HarperFast/harper-pro
that referenced
this pull request
Jun 10, 2026
Pulls in core fix (HarperFast/harper#1223): the RocksDB receiver was silently dropping records during bulk full-table copy because ERR_TRY_AGAIN (memtable conflict-check window overflow under bulk ingest) fell through the commit retry as an unhandled rejection. Now retried like ERR_BUSY. Refs #308. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
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.
Summary
Under bulk-ingest bursts (notably a v4→v5 migration full-table copy into a RocksDB receiver), RocksDB returns
kTryAgain(rocksdb-js codeERR_TRY_AGAIN) when a transaction's snapshot sequence falls outside the memtable conflict-check window (max_write_buffer_size_to_maintain). The commit error handler inDatabaseTransaction.tsonly retriedERR_BUSY;ERR_TRY_AGAINfell through tothrow. Because the commit is driven from an unawaitedonCommit().then(), that rejection became an unhandled promise rejection in the worker — logged but the write was silently dropped. Copied tables froze short of their source counts with no error surfaced anywhere.This is the data-loss mechanism behind harper-pro#308, reproduced deterministically twice in our internal migration test bench (the internal migration-bench repro).
Change
One clause: treat
ERR_TRY_AGAINlikeERR_BUSYin the commit retry — retry with the existing backoff, and afterMAX_RETRIESthrow aServerError(loud) rather than losing the write. Both are transient, retryable conflict conditions. Reuses the already-exercisedERR_BUSYretry/backoff path; no new control flow.What to check
resources/DatabaseTransaction.ts— the retry condition + comment.ERR_TRY_AGAINcode string is confirmed against rocksdb-js (src/binding/napi/helpers.cpp:264/300,kTryAgain → "ERR_TRY_AGAIN").^2.0.0(retuned memtable defaults —maxWriteBufferSizeToMaintain: -1,dbWriteBufferSize: 0) which reduces the frequency of this conflict; this PR handles the residual transient occurrences so they retry instead of silently dropping. The two are complementary.Validation
Core
tscbuild clean. End-to-end acceptance is the internal migration-bench repro re-run (it deterministically reproduced the silent drop; per-table count parity is the signal). A focused unit test for this internal retry path needs significant transaction-harness mocking and mirrors the already-testedERR_BUSYbranch — happy to add one if reviewers want it.🤖 Generated with Claude Opus 4.7.