Commit graph

1 commit

Author SHA1 Message Date
550bd4a19c fix: saveBinaryBlob unique tmp suffix + ENOENT swallow on rename
FileSystemStorage.saveBinaryBlob used a bare ${filePath}.tmp suffix for its
atomic-write temp file. Two concurrent same-key calls computed the SAME temp
path; both writeFile'd, the first rename succeeded, the second rename fired
against a missing temp and threw ENOENT. The throw propagated up through
brain.flush() and broke any downstream job that called it.

Reproduced in production (Brainy 7.30 + Cortex 2.7 + mmap-filesystem):

  [job-queue] gcs-backup: failed - ENOENT: no such file or directory,
    rename '/data/brainy-data/.../_column_index/owner/DELETED.bin.tmp'
         -> '/data/brainy-data/.../_column_index/owner/DELETED.bin'

The race was two cron jobs (gcs-backup hourly + flush-brain every 15min) both
calling flush(), triggering column-store compaction over the same fields,
overlapping at the rename. Off-site backups had been failing continuously for
~48 hours.

PATCH

src/storage/adapters/fileSystemStorage.ts:992-999 — saveBinaryBlob now uses a
unique per-writer temp suffix matching the pattern at every other atomic-write
site in the same file (lines 336, 551, 744, 781, 1529, 2908):

  const tmpPath = `${filePath}.tmp.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2)}`

Adds defensive ENOENT swallow on rename: if the temp is gone, the work has
already landed (saveBinaryBlob is idempotent for a given key — all callers
persist the same logical bytes per key). Cleans up the temp on any other
rename failure to avoid orphan .tmp.* files.

SCOPE AUDIT

One bug site. Audit results:

- FileSystemStorage: six sibling atomic-write sites already used unique
  suffixes (lines 336/551/744/781/1529/2908); only saveBinaryBlob was the
  outlier. All six rechecked. Clean.
- OPFSStorage: WritableStream (no tmp+rename). Not affected.
- GCSStorage / R2Storage / AzureBlobStorage / S3CompatibleStorage: object-store
  PUT (atomic at the API). Not affected.
- MemoryStorage: in-memory. Not affected.
- HistoricalStorageAdapter: read-only. Not affected.
- COW / versioning / snapshot / HNSW / aggregation: all delegate to storage
  adapters via saveBinaryBlob / writeObjectToPath. They get the fix
  automatically by using the patched primitive.

The bare-`.tmp` pattern is now gone repo-wide.

BENEFICIARIES

Beyond the reported column-store-compaction race:

- HNSW connection persistence (src/hnsw/hnswIndex.ts:252 → saveBinaryBlob)
  was structurally susceptible to the same race. No production reports of
  HNSW failures (probably because HNSW writes are more naturally serialized
  by the index lock), but the fix removes the latent issue.
- Any future caller of saveBinaryBlob inherits the safer semantics.

TESTS

New tests/integration/savebinaryblob-concurrent-rename.test.ts (4 tests):
- 20 concurrent saveBinaryBlob(sameKey, ...) all resolve without throwing
- No orphan .tmp.* siblings remain in the blob directory afterwards
- Production-shape: two concurrent compactor passes over 10 column-index
  fields (owner, path, permissions, vfsType, modified, createdAt, accessed,
  updatedAt, mimeType, size). All 20 calls succeed; each field ends with
  valid bytes.
- Single-writer path still produces correct bytes (no regression on common
  case).

Verified the first three tests reproduce the production ENOENT error
verbatim on the pre-7.31.1 code path (stashed the fix, watched them fail
with the exact production error).

VERIFICATION

- npx tsc --noEmit: clean
- npm test: 1468 / 1468 unit
- New integration suite: 4/4
- npm run build: clean

CORTEX COMPATIBILITY

Zero changes. The Cortex mmap-filesystem adapter wraps FileSystemStorage
and inherits the patch automatically.

FORWARD-COMPAT

8.0's Db.persist() will route through the same patched primitive; no
additional work needed when the immutable Db API ships.
2026-06-09 10:36:02 -07:00