diff --git a/src/indexes/columnStore/ColumnStore.ts b/src/indexes/columnStore/ColumnStore.ts index 2c07d433..d33c05c4 100644 --- a/src/indexes/columnStore/ColumnStore.ts +++ b/src/indexes/columnStore/ColumnStore.ts @@ -69,6 +69,34 @@ interface HeapEntry { * await store.flush() * const newest = await store.sortTopK('createdAt', 'desc', 10) // bigint[] */ +/** + * @description Thrown when a segment the manifest lists cannot be loaded — + * either it yields no bytes (missing/empty on disk) or its bytes are + * undecodable (corruption). Previously such a segment was silently skipped, + * dropping ALL of its entities from every `filter`/`rangeQuery`/`sortTopK` with + * no error — a manifest↔segments divergence that looked like a short result. + * Surfacing it loudly makes the divergence visible and repairable. (A genuine + * storage IO fault is a different class — it propagates as the underlying error, + * not wrapped in this.) + */ +export class ColumnSegmentLoadError extends Error { + /** The indexed field whose segment failed to load. */ + public readonly field: string + /** The manifest-listed segment id that could not be loaded. */ + public readonly segmentId: number | string + constructor(field: string, segmentId: number | string, reason: string) { + super( + `ColumnStore segment ${field}:${segmentId} is listed in the manifest but ` + + `could not be loaded (${reason}). The index is inconsistent with its ` + + `manifest — rebuild/repair the metadata index rather than trusting a ` + + `short query result.` + ) + this.name = 'ColumnSegmentLoadError' + this.field = field + this.segmentId = segmentId + } +} + export class ColumnStore implements ColumnStoreProvider { private storage!: StorageAdapter private idMapper!: EntityIdMapper @@ -594,14 +622,15 @@ export class ColumnStore implements ColumnStoreProvider { let cursor = this.segmentCache.get(cacheKey) if (!cursor) { - const loaded = await this.loadSegmentCursor(field, seg) - if (loaded) { - cursor = loaded - this.segmentCache.set(cacheKey, cursor) - } + // loadSegmentCursor either returns a cursor or THROWS — a corrupt / + // missing manifest-listed segment raises ColumnSegmentLoadError and a + // real storage fault propagates, so a listed segment is never silently + // dropped from the result set. + cursor = await this.loadSegmentCursor(field, seg) + this.segmentCache.set(cacheKey, cursor) } - if (cursor) cursors.push(cursor) + cursors.push(cursor) } return cursors @@ -615,39 +644,51 @@ export class ColumnStore implements ColumnStoreProvider { * `.cidx` object-path so indexes written before the format unification keep * loading correctly. Mirror of cortex's 2.3.1 read-side fallback. */ - private async loadSegmentCursor(field: string, seg: SegmentMeta): Promise { + private async loadSegmentCursor(field: string, seg: SegmentMeta): Promise { const manifest = this.manifests.get(field) - if (!manifest) return null + if (!manifest) { + // Defensive: getSegmentCursors guards on the manifest before calling. + throw new ColumnSegmentLoadError(field, seg.id, 'no manifest for field') + } - try { - let buf: Buffer | null = null + let buf: Buffer | null = null - const storage = this.storage as unknown as { - loadBinaryBlob?: (key: string) => Promise - readObjectFromPath: (path: string) => Promise - } + const storage = this.storage as unknown as { + loadBinaryBlob?: (key: string) => Promise + readObjectFromPath: (path: string) => Promise + } - // Preferred: raw blob at the cor-shared key. - if (typeof storage.loadBinaryBlob === 'function') { - const key = `${this.basePath}/${field}/L${seg.level}-${String(seg.id).padStart(6, '0')}` - const blob = await storage.loadBinaryBlob(key) - if (blob && blob.length > 0) buf = blob - } + // Preferred: raw blob at the cor-shared key. A real IO fault PROPAGATES — + // loadBinaryBlob throws on a fault and returns null only for genuine absence + // (a present-but-unreadable segment must not read as "missing"). + if (typeof storage.loadBinaryBlob === 'function') { + const key = `${this.basePath}/${field}/L${seg.level}-${String(seg.id).padStart(6, '0')}` + const blob = await storage.loadBinaryBlob(key) + if (blob && blob.length > 0) buf = blob + } - // Legacy fallback: `{ _binary, base64 }` envelope at the .cidx object-path. - if (!buf) { - const segPath = manifest.segmentPath(seg.level, seg.id) - const stored = await storage.readObjectFromPath(segPath) - if (!stored) return null + // Legacy fallback: `{ _binary, base64 }` envelope at the .cidx object-path. + if (!buf) { + const segPath = manifest.segmentPath(seg.level, seg.id) + const stored = await storage.readObjectFromPath(segPath) + if (stored) { if (stored._binary && stored.data) { buf = Buffer.from(stored.data, 'base64') } else if (Buffer.isBuffer(stored)) { buf = stored - } else { - return null } } + } + // A manifest-listed segment that yields NO loadable bytes is corruption, not + // benign absence: swallowing it silently dropped all of the segment's + // entities from every filter/rangeQuery/sortTopK with no error. Surface it + // loudly so the manifest↔segments divergence is visible (and repairable). + if (!buf) { + throw new ColumnSegmentLoadError(field, seg.id, 'manifest-listed segment has no loadable bytes') + } + + try { const parsed = readSegmentFromBuffer(buf) return new ColumnSegmentCursor( parsed.header, @@ -655,8 +696,13 @@ export class ColumnStore implements ColumnStoreProvider { parsed.entityIds, parsed.tombstones ) - } catch { - return null + } catch (err) { + // Undecodable bytes for a listed segment — corruption, not a short result. + throw new ColumnSegmentLoadError( + field, + seg.id, + `segment decode failed: ${(err as Error).message}` + ) } } diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index dd1d3fee..41ebeb45 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -29,6 +29,7 @@ import { getShardId } from './sharding.js' import { BlobStorage, type BlobStoreAdapter } from './blobStorage.js' import { unwrapBinaryData } from './binaryDataCodec.js' import { prodLog } from '../utils/logger.js' +import { isAbsentError } from '../utils/errorClassification.js' import { BrainyError } from '../errors/brainyError.js' import { MetadataWriteBuffer } from '../utils/metadataWriteBuffer.js' import { @@ -4064,8 +4065,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { return this.deserializeNoun(noun) } } catch (error) { - // Entity not found - return null + // A real storage/deserialize fault is NOT "entity not found": + // readCanonicalObject returns null for genuine absence and only throws on + // a real fault, so masking it here reports a present-but-unreadable entity + // as missing. Absence → null, fault → propagate loudly. + if (isAbsentError(error)) return null + throw error } return null @@ -4175,8 +4180,12 @@ export abstract class BaseStorage extends BaseStorageAdapter { return this.deserializeVerb(verb) } } catch (error) { - // Entity not found - return null + // A real storage/deserialize fault is NOT "relationship not found": + // readCanonicalObject returns null for genuine absence and only throws on + // a real fault, so masking it here reports a present-but-unreadable edge + // as missing. Absence → null, fault → propagate loudly. + if (isAbsentError(error)) return null + throw error } return null diff --git a/tests/unit/indexes/columnStore/segment-load-fault.test.ts b/tests/unit/indexes/columnStore/segment-load-fault.test.ts new file mode 100644 index 00000000..deb0868f --- /dev/null +++ b/tests/unit/indexes/columnStore/segment-load-fault.test.ts @@ -0,0 +1,128 @@ +/** + * @module tests/unit/indexes/columnStore/segment-load-fault + * @description Pattern-B acceptance for the ColumnStore (finding 4): a segment + * the manifest LISTS but that cannot be loaded must never be silently skipped — + * doing so dropped every entity in that segment out of `filter`/`rangeQuery`/ + * `sortTopK` with no error, so a corrupt index looked like a merely short result. + * + * The three failure classes and their required behaviour: + * - a real storage IO fault (EIO) PROPAGATES verbatim — a present-but-unreadable + * segment is not "absent", so it must not read as an empty result; + * - a manifest-listed segment with undecodable bytes throws `ColumnSegmentLoadError`; + * - a manifest-listed segment with NO bytes (gone on disk) throws `ColumnSegmentLoadError`. + * Only genuine absence stays benign: querying a field that has no manifest at all + * returns empty (nothing was ever written for it) — that is not a fault. + */ +import { describe, it, expect, beforeEach } from 'vitest' +import { + ColumnStore, + ColumnSegmentLoadError +} from '../../../../src/indexes/columnStore/ColumnStore.js' +import { MemoryStorage } from '../../../../src/storage/adapters/memoryStorage.js' +import { EntityIdMapper } from '../../../../src/utils/entityIdMapper.js' + +type FaultMode = 'none' | 'io' | 'corrupt' | 'missing' + +/** + * A MemoryStorage that can fault reads of persisted column SEGMENTS only + * (keys/paths containing the `L0-` segment marker). Manifest and DELETED-bitmap + * reads pass through untouched so a fresh store still initialises normally — the + * fault is isolated to the exact seam finding 4 hardened. + */ +class FaultInjectingStorage extends MemoryStorage { + public faultMode: FaultMode = 'none' + + private eio(): Error { + const e = new Error('simulated disk read fault') as Error & { code: string } + e.code = 'EIO' + return e + } + + public async loadBinaryBlob(key: string): Promise { + if (this.faultMode !== 'none' && key.includes('/L0-')) { + if (this.faultMode === 'io') throw this.eio() + // Too small to hold even a header → readSegmentFromBuffer throws → wrapped. + if (this.faultMode === 'corrupt') return Buffer.from([1, 2, 3, 4, 5]) + if (this.faultMode === 'missing') return null + } + return super.loadBinaryBlob(key) + } +} + +describe('ColumnStore segment-load faults surface loudly, absence stays benign (finding 4)', () => { + let storage: FaultInjectingStorage + let idMapper: EntityIdMapper + + beforeEach(async () => { + storage = new FaultInjectingStorage() + await storage.init() + idMapper = new EntityIdMapper({ storage, storageKey: 'test:idMapper' }) + await idMapper.init() + + // Write one persisted L0 segment for `createdAt`, then close the writer. + const writer = new ColumnStore({ flushThreshold: 10 }) + await writer.init(storage, idMapper) + for (let i = 0; i < 5; i++) { + writer.addEntity(BigInt(idMapper.getOrAssign(`e${i}`)), { + createdAt: (i + 1) * 100 + }) + } + await writer.flush() + await writer.close() + storage.faultMode = 'none' + }) + + // Fresh reader over the same storage: empty segment cache, so every query is + // forced to actually load the persisted segment (that is the seam under test). + const reopen = async (): Promise => { + const s = new ColumnStore({ flushThreshold: 10 }) + await s.init(storage, idMapper) + return s + } + + it('propagates a storage IO fault verbatim — not [] and not a ColumnSegmentLoadError', async () => { + storage.faultMode = 'io' + const store = await reopen() + await expect(store.filter('createdAt', 300)).rejects.toMatchObject({ + code: 'EIO' + }) + await store.close() + }) + + it('throws ColumnSegmentLoadError when a manifest-listed segment is undecodable', async () => { + storage.faultMode = 'corrupt' + const store = await reopen() + await expect( + store.sortTopK('createdAt', 'desc', 10) + ).rejects.toBeInstanceOf(ColumnSegmentLoadError) + await store.close() + }) + + it('throws ColumnSegmentLoadError when a manifest-listed segment has no loadable bytes', async () => { + storage.faultMode = 'missing' + const store = await reopen() + await expect( + store.rangeQuery('createdAt', 100, 500) + ).rejects.toBeInstanceOf(ColumnSegmentLoadError) + await store.close() + }) + + it('a field with no manifest is genuine absence — returns empty, never throws', async () => { + storage.faultMode = 'none' + const store = await reopen() + const bitmap = await store.filter('no_such_field', 'x') + expect(bitmap.size).toBe(0) + const sorted = await store.sortTopK('no_such_field', 'asc', 10) + expect(sorted).toEqual([]) + await store.close() + }) + + it('with no fault, the persisted segment still loads and answers queries', async () => { + storage.faultMode = 'none' + const store = await reopen() + const sorted = await store.sortTopK('createdAt', 'desc', 10) + const uuids = sorted.map((id) => idMapper.getUuid(Number(id))) + expect(uuids).toEqual(['e4', 'e3', 'e2', 'e1', 'e0']) + await store.close() + }) +})