brainy/tests/integration/savebinaryblob-concurrent-rename.test.ts

120 lines
4.6 KiB
TypeScript
Raw Normal View History

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
/**
* @module tests/integration/savebinaryblob-concurrent-rename
* @description Regression test for the 7.31.1 fix to `FileSystemStorage.saveBinaryBlob`.
*
* Pre-7.31.1, `saveBinaryBlob` used a bare `${filePath}.tmp` suffix for its
* atomic-write temp file. Two concurrent same-key calls computed the same
* temp path; whichever rename ran second hit ENOENT because the first call
* had already consumed the temp.
*
* Reproduced in production: column-store compaction running alongside an
* explicit flush() repeatedly raced on `_column_index/<field>/DELETED.bin`,
* causing brain.flush() and the downstream GCS backup job to fail
* continuously.
*
* This test asserts:
* - 20 concurrent saveBinaryBlob(sameKey, ...) calls all resolve without throwing
* - The final file contents are valid (one of the inputs, last-write-wins)
* - No orphan `.tmp.*` siblings are left in the blob directory
* - The race also holds when interleaved across multiple keys (column-store
* compaction shape)
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
import { mkdtempSync, readdirSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { FileSystemStorage } from '../../src/storage/adapters/fileSystemStorage.js'
describe('7.31.1 — saveBinaryBlob concurrent rename safety', () => {
let dataDir: string
let storage: FileSystemStorage
beforeEach(async () => {
dataDir = mkdtempSync(join(tmpdir(), 'brainy-savebinaryblob-'))
storage = new FileSystemStorage(dataDir)
await storage.init()
})
afterEach(() => {
rmSync(dataDir, { recursive: true, force: true })
})
it('20 concurrent saveBinaryBlob calls for the same key resolve without ENOENT', async () => {
const key = '_column_index/owner/DELETED'
const writers = Array.from({ length: 20 }, (_, i) => {
// Each writer's payload is unique so we can verify which one wins.
const payload = Buffer.from(`writer-${i.toString().padStart(2, '0')}-payload`)
return storage.saveBinaryBlob(key, payload)
})
// No call should throw. Pre-7.31.1, ~19 of 20 throw ENOENT under contention.
await expect(Promise.all(writers)).resolves.not.toThrow()
// Final file should be readable and equal to one of the payloads.
const final = await storage.loadBinaryBlob(key)
expect(final).not.toBeNull()
expect(final!.toString()).toMatch(/^writer-\d{2}-payload$/)
})
it('leaves no orphan .tmp.* siblings after concurrent same-key writes', async () => {
const key = '_column_index/permissions/DELETED'
const writers = Array.from({ length: 20 }, (_, i) =>
storage.saveBinaryBlob(key, Buffer.from(`payload-${i}`))
)
await Promise.all(writers)
// Locate the directory containing the blob and list its children.
const blobPath = storage.getBinaryBlobPath(key)
expect(blobPath).not.toBeNull()
const parentDir = join(blobPath!, '..')
const siblings = readdirSync(parentDir)
const tmps = siblings.filter((name) => name.includes('.tmp'))
expect(tmps).toEqual([])
})
it('handles concurrent writes across the full column-index field set (production shape)', async () => {
// Mirror the per-field DELETED.bin pattern Brainy's column-store compactor
// emits. In production this is what races between flush() and gcs-backup.
const fields = [
'owner',
'path',
'permissions',
'vfsType',
'modified',
'createdAt',
'accessed',
'updatedAt',
'mimeType',
'size'
]
// Two concurrent compactor passes — one acting as the periodic flush, one
// as the explicit user flush — both touching every field.
const passA = fields.map((f) =>
storage.saveBinaryBlob(`_column_index/${f}/DELETED`, Buffer.from(`A-${f}`))
)
const passB = fields.map((f) =>
storage.saveBinaryBlob(`_column_index/${f}/DELETED`, Buffer.from(`B-${f}`))
)
await expect(Promise.all([...passA, ...passB])).resolves.not.toThrow()
// Every field should have a readable final blob.
for (const f of fields) {
const final = await storage.loadBinaryBlob(`_column_index/${f}/DELETED`)
expect(final).not.toBeNull()
expect(final!.toString()).toMatch(new RegExp(`^[AB]-${f}$`))
}
})
it('the existing single-writer path still produces correct bytes', async () => {
// Sanity: the patch must not regress the common single-writer case.
const key = '_blob/single-writer-test'
const payload = Buffer.from('hello')
await storage.saveBinaryBlob(key, payload)
const read = await storage.loadBinaryBlob(key)
expect(read?.toString()).toBe('hello')
})
})