cherry-pick: fix(blob): clean up orphaned blob files on aborted/superseded writes → v5.0 (conflict)#479
Merged
Merged
Conversation
Blobs flagged with `saveBeforeCommit` (or `saveInRecord`) are written to
disk in the `beforeIntermediate` phase of a transaction write, before the
underlying record write commits. Several paths could leave the file on
disk with no record referencing it:
- Multi-write transaction where one `beforeIntermediate` rejected after
others wrote files (only the failing blob was cleaned up by
`deleteOnFailure`; succeeded peers were leaked).
- Commit-handler early-returns: out-of-order replicated update treated
as duplicate, superseded by a newer put/delete, no-audit fullUpdate
loses to existing version, cache-resolve version-changed.
- `DatabaseTransaction.commit` rejecting on `Promise.all(completions)`
failure (e.g. blob save error) without aborting — leaked the
underlying RocksDB transaction *and* the blob files.
- Mid-record decode failure in the replication ingest path.
Approach:
- `startPreCommitBlobsForRecord` now returns `{ blobs, complete }` so
each `TransactionWrite` can attach `savedBlobs` for later cleanup.
- New `cleanupUnusedBlobs(blobs)` waits for any in-flight save to
settle, then unlinks. Idempotent — clears the list after scheduling.
- Commit handlers in `Table.ts` set `write.skipped = true` (resetting
to `false` on each invocation) on early-return paths that don't end
up writing a record or audit reference. The transaction commit
success paths walk writes and clean up `skipped` ones. Cleanup is
deferred to post-commit because the same handler runs again on
optimistic-lock retries and a retry can flip a previously-skipped
write into a successful one (e.g. a concurrent delete makes our
older replicated update suddenly win).
- `LMDBTransaction.abort` and `DatabaseTransaction.abort` walk all
writes' `savedBlobs` and clean unconditionally.
- `DatabaseTransaction.commit` adds an explicit reject handler so a
`Promise.all` failure aborts instead of leaking.
Tests: new `Blob test > multi-write transaction with one failing blob
cleans up succeeded blobs` and `cleanupUnusedBlobs is a no-op for
unsaved blobs`. The post-suite `cleanupOrphans` test acts as a safety
net across all blob tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…KNOWN_SIZE When all blob content is read and the reader gets bytesRead=0, it re-reads the header (still UNKNOWN_SIZE) and sets up an fs.watch watcher. If the writer finishes and updates the header between that header read and the watcher being created, no further writes occur and the watcher never fires, causing a 60s timeout with 'size is supposed to be 281474976710655 bytes'. Fix: after readSync at `position` returns 0, immediately re-read the header from position 0. If the size is no longer UNKNOWN_SIZE, bypass the watcher and retry directly. Apply the same check in the timeout callback as a second line of defense. Also eliminates the shared module-level HEADER/headerView buffers in favor of local DataView instances, avoiding any shared-state risk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…riterFinished Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lect phase When a join condition (e.g. relatedByName) becomes the primary iterator due to a lower estimated count, the select phase uses filterMap.hasMappings=true and calls targetTable.transformEntryForSelect with the outer table's readTxn. That readTxn is a RocksTransaction scoped to the outer table's RocksDB column family and cannot read from a different table's column family, so getEntry returns null, producing [undefined] in the joined attribute. Fix: compute targetReadTxn = targetTable._readTxnForContext(context) when the target table differs from the current table, ensuring each joined table's records are loaded with a transaction scoped to that table's own store. Co-Authored-By: Claude Sonnet 4.6 <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.
Resolve cherry-pick conflicts
PR #462 (fix(blob): clean up orphaned blob files on aborted/superseded writes) conflicted when cherry-picking onto
v5.0.Conflict markers (
<<<<<<</=======/>>>>>>>) have been committed to this branch. Please check out the branch, resolve all conflicts, push, and merge.Source PR: #462
Commit(s):
88cddb1cdc910395acb359e6580e2f5c17acf28a 46166414647598c65fb5cd55a45b5706aec1fe36 dd03850a036f7791e8146d4849b27fd9c7fa26f9 a320330a41d734e4fbe7eb81bcbc6ee34dfbb164 7b6d90959bfe7699b56b70175cc8714d41109810 f85b5b75b504d677a8ba024ca51e79aad582fe84