fix(close): a read-only brain writes nothing under _system/
`readonly-close-no-marker` closed the clean-shutdown-marker half of this law and named the rest as a known residual. This is that residual, closed. MEASURED on the base: a read-only open → read → close rewrote FOUR files — `_system/__metadata_field_registry__.json.gz`, `type-statistics.json.gz`, `subtype-statistics.json.gz` and `verb-subtype-statistics.json.gz`. An IDLE reader that only opened and closed rewrote all four as well. The cause was not the closes the marker fix guarded. It was Phase 1 of closeDurableSteps, where every component flush ran unconditionally. A flush is a write by definition: MetadataIndexManager#flush() saves the field registry "even with no dirty fields" (its own comment), and the storage adapter's count flush re-stamps the three statistics files. A session that committed nothing re-stamped all four. Phase 2's closes were ungated too — the graph index's close drains both LSM MemTables to SSTables and stamps its watermark, and the optional vector/metadata `close` hooks (unimplemented in the reference engine, filled in by a native provider) persist buffered state. Every one of those calls now carries the same `!isReadOnly` guard the generation store already had. A reader still RELEASES what it holds, so Phase 2 is a branch rather than a skip: GraphAdjacencyIndex gains `stopBackgroundFlush()`, the non-writing half of its close, which clears the auto-flush interval that would otherwise outlive the session. `close()` now calls it too, so there is one place that owns the timer. Why this matters beyond tidiness: `_system/` is where a store keeps its evidence about itself — what the writer committed, what the projections have seen. A reader that rewrites any of it vouches for a state it only observed, and on shared or snapshot storage it mutates bytes another process owns. The pin hashes every file under `_system/` (and, in one case, the whole store) across a reader's open → read → close, names the four paths that used to move so a regression says which subsystem did it, and asserts the asymmetry holds in the other direction — a WRITER's close still persists.
This commit is contained in:
parent
0d5ab6077d
commit
f27a777615
4 changed files with 322 additions and 16 deletions
|
|
@ -20834,34 +20834,45 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
|
||||
// Phase 1: Flush ALL components in parallel to persist buffered data
|
||||
// This is critical when cor native providers buffer data in Rust memory
|
||||
//
|
||||
// READ-ONLY GUARD, applied to EVERY flush here. A flush is a write by
|
||||
// definition, and a reader has nothing of its own to persist — but these
|
||||
// calls were not conditional, so a read-only open → read → close REWROTE
|
||||
// four files under `_system/`: the metadata field registry (whose flush()
|
||||
// saves it unconditionally, "even with no dirty fields"), and the three
|
||||
// type/subtype statistics files the storage adapter's count flush stamps.
|
||||
// Every one of them was re-stamped on a session that committed nothing.
|
||||
// A reader must leave `_system/` exactly as it found it — the same law the
|
||||
// clean-shutdown marker already lives under (see the generation-store
|
||||
// guard below and `Brainy.openReadOnly`).
|
||||
await Promise.all([
|
||||
// Flush HNSW dirty nodes (deferred persistence mode)
|
||||
(async () => {
|
||||
if (this.index && typeof this.index.flush === 'function') {
|
||||
if (this.index && !this.isReadOnly && typeof this.index.flush === 'function') {
|
||||
await this.index.flush()
|
||||
}
|
||||
})(),
|
||||
// Flush metadata index (field indexes + EntityIdMapper)
|
||||
(async () => {
|
||||
if (this.metadataIndex && typeof this.metadataIndex.flush === 'function') {
|
||||
if (this.metadataIndex && !this.isReadOnly && typeof this.metadataIndex.flush === 'function') {
|
||||
await this.metadataIndex.flush()
|
||||
}
|
||||
})(),
|
||||
// Flush graph adjacency index (LSM trees)
|
||||
(async () => {
|
||||
if (this.graphIndex && typeof this.graphIndex.flush === 'function') {
|
||||
if (this.graphIndex && !this.isReadOnly && typeof this.graphIndex.flush === 'function') {
|
||||
await this.graphIndex.flush()
|
||||
}
|
||||
})(),
|
||||
// Flush storage adapter counts
|
||||
(async () => {
|
||||
if (this.storage && typeof this.storage.flushCounts === 'function') {
|
||||
if (this.storage && !this.isReadOnly && typeof this.storage.flushCounts === 'function') {
|
||||
await this.storage.flushCounts()
|
||||
}
|
||||
})(),
|
||||
// Flush aggregation index state
|
||||
(async () => {
|
||||
if (this._aggregationIndex) {
|
||||
if (this._aggregationIndex && !this.isReadOnly) {
|
||||
await this._aggregationIndex.flush()
|
||||
}
|
||||
})(),
|
||||
|
|
@ -20910,21 +20921,37 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
|
||||
// Phase 2: Close components to release resources (timers, file handles)
|
||||
// Data is already safe on disk from Phase 1
|
||||
//
|
||||
// READ-ONLY GUARD, same law as Phase 1. Each of these closes is a WRITER:
|
||||
// the graph index drains both LSM MemTables to SSTables and stamps its
|
||||
// watermark, and the vector/metadata `close` hooks — optional doors the
|
||||
// reference engine leaves unimplemented, but which a native provider fills
|
||||
// in — persist their buffered state. None of that is a reader's to write.
|
||||
//
|
||||
// A reader still has to RELEASE what it holds, which is why this is a
|
||||
// branch rather than a skip: `stopBackgroundFlush()` is the non-writing
|
||||
// half of the graph index's close, clearing the auto-flush interval that
|
||||
// would otherwise outlive the session. The optional hooks have no
|
||||
// non-writing counterpart to call, and a provider that buffers nothing on
|
||||
// a read-only open has nothing to release.
|
||||
await Promise.all([
|
||||
(async () => {
|
||||
if (this.graphIndex && typeof this.graphIndex.close === 'function') {
|
||||
if (!this.graphIndex) return
|
||||
if (this.isReadOnly) {
|
||||
this.graphIndex.stopBackgroundFlush()
|
||||
} else if (typeof this.graphIndex.close === 'function') {
|
||||
await this.graphIndex.close()
|
||||
}
|
||||
})(),
|
||||
(async () => {
|
||||
const index = this.index as JsHnswVectorIndex & VectorIndexOptionalHooks
|
||||
if (index && typeof index.close === 'function') {
|
||||
if (index && !this.isReadOnly && typeof index.close === 'function') {
|
||||
await index.close()
|
||||
}
|
||||
})(),
|
||||
(async () => {
|
||||
const metadataIndex = this.metadataIndex as MetadataIndexManager & MetadataIndexOptionalHooks
|
||||
if (metadataIndex && typeof metadataIndex.close === 'function') {
|
||||
if (metadataIndex && !this.isReadOnly && typeof metadataIndex.close === 'function') {
|
||||
await metadataIndex.close()
|
||||
}
|
||||
})(),
|
||||
|
|
|
|||
|
|
@ -1105,13 +1105,31 @@ export class GraphAdjacencyIndex implements GraphIndexProvider {
|
|||
}
|
||||
|
||||
/**
|
||||
* Clean shutdown
|
||||
* Stop the auto-flush interval WITHOUT writing anything.
|
||||
*
|
||||
* The non-writing half of {@link close}, for a shutdown that must leave the
|
||||
* store byte-identical — a read-only brain's close. `close()` itself is a
|
||||
* writer: it drains both LSM MemTables to SSTables and stamps the watermark,
|
||||
* which is exactly right for a writer and forbidden for a reader. A reader
|
||||
* still has to release this interval, though: it is the one piece of this
|
||||
* index that outlives the close and could fire against a store the session no
|
||||
* longer owns.
|
||||
*
|
||||
* @returns Nothing.
|
||||
*/
|
||||
async close(): Promise<void> {
|
||||
stopBackgroundFlush(): void {
|
||||
if (this.flushTimer) {
|
||||
clearInterval(this.flushTimer)
|
||||
this.flushTimer = undefined
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean shutdown — drains both trees and stamps the watermark. THIS WRITES;
|
||||
* a read-only brain must call {@link stopBackgroundFlush} instead.
|
||||
*/
|
||||
async close(): Promise<void> {
|
||||
this.stopBackgroundFlush()
|
||||
|
||||
// Close both LSM-trees (will flush MemTables to SSTables)
|
||||
if (this.initialized) {
|
||||
|
|
|
|||
|
|
@ -149,12 +149,12 @@ describe('a read-only brain writes no clean-shutdown evidence', () => {
|
|||
brain = null
|
||||
|
||||
// The FILE SET under `_system/` is unchanged — a reader creates and
|
||||
// removes nothing. (Other files under `_system/` — e.g. the metadata
|
||||
// field registry, which stamps its own `lastUpdated` on every persist —
|
||||
// are a pre-existing, separate concern outside this fix's scope: this
|
||||
// pin is specifically about the generation store's clean-shutdown
|
||||
// evidence, not about every subsystem's close() being a true no-op for
|
||||
// a reader.)
|
||||
// removes nothing. This pin is specifically about the generation store's
|
||||
// clean-shutdown evidence. The wider law — that a reader leaves EVERY
|
||||
// file under `_system/` byte-identical, which this fix left open as a
|
||||
// known residual (the metadata field registry and the three statistics
|
||||
// files were still re-stamped by a reader's close) — is closed and pinned
|
||||
// in `readonly-close-writes-nothing.test.ts`.
|
||||
const after = snapshotDir(systemDir())
|
||||
expect([...after.keys()].sort()).toEqual([...before.keys()].sort())
|
||||
|
||||
|
|
|
|||
261
tests/integration/readonly-close-writes-nothing.test.ts
Normal file
261
tests/integration/readonly-close-writes-nothing.test.ts
Normal file
|
|
@ -0,0 +1,261 @@
|
|||
/**
|
||||
* @module tests/integration/readonly-close-writes-nothing
|
||||
* @description A READ-ONLY BRAIN LEAVES `_system/` BYTE-IDENTICAL — the WHOLE
|
||||
* directory, not just the clean-shutdown marker.
|
||||
*
|
||||
* `readonly-close-no-marker` closed the marker half of this law and named the
|
||||
* rest as a known, out-of-scope residual:
|
||||
*
|
||||
* "Other files under `_system/` — e.g. the metadata field registry, which
|
||||
* stamps its own `lastUpdated` on every persist — are a pre-existing,
|
||||
* separate concern outside this fix's scope."
|
||||
*
|
||||
* This is that residual, closed. MEASURED on the base before the fix, a
|
||||
* read-only open → read → close rewrote FOUR files:
|
||||
*
|
||||
* _system/__metadata_field_registry__.json.gz
|
||||
* _system/type-statistics.json.gz
|
||||
* _system/subtype-statistics.json.gz
|
||||
* _system/verb-subtype-statistics.json.gz
|
||||
*
|
||||
* THE CAUSE was not the closes the marker fix guarded — it was Phase 1 of
|
||||
* `closeDurableSteps`, where every component flush ran unconditionally. A flush
|
||||
* is a write by definition: `MetadataIndexManager#flush()` saves the field
|
||||
* registry "even with no dirty fields" (its own comment), and the storage
|
||||
* adapter's count flush re-stamps the three statistics files. A session that
|
||||
* committed nothing re-stamped all four. Phase 2's closes were ungated too —
|
||||
* the graph index's close drains both LSM MemTables and stamps a watermark,
|
||||
* and the optional vector/metadata `close` hooks (unimplemented in the
|
||||
* reference engine, filled in by a native provider) persist buffered state.
|
||||
*
|
||||
* THE LAW. A reader writes nothing, anywhere under `_system/`, at open or at
|
||||
* close. It still RELEASES what it holds: the graph index's auto-flush interval
|
||||
* is cleared through `stopBackgroundFlush()`, the non-writing half of its
|
||||
* close, so nothing outlives the session.
|
||||
*
|
||||
* WHY IT MATTERS beyond tidiness: `_system/` is where a store keeps its
|
||||
* evidence about itself — what the writer committed, what the projections have
|
||||
* seen. A reader that rewrites any of it is vouching for a state it only
|
||||
* observed, and on shared or snapshot storage it mutates bytes another process
|
||||
* owns.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
|
||||
import { mkdtempSync, rmSync, readdirSync, readFileSync, statSync } from 'node:fs'
|
||||
import { createHash } from 'node:crypto'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { Brainy } from '../../src/brainy.js'
|
||||
import { NounType, VerbType } from '../../src/types/graphTypes.js'
|
||||
|
||||
/** Recursively hash every regular file under `dir`, keyed by its path relative to `dir`. */
|
||||
function snapshotDir(dir: string): Map<string, string> {
|
||||
const out = new Map<string, string>()
|
||||
const walk = (rel: string): void => {
|
||||
const abs = rel ? join(dir, rel) : dir
|
||||
let entries: string[]
|
||||
try {
|
||||
entries = readdirSync(abs)
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
for (const name of entries) {
|
||||
const childRel = rel ? join(rel, name) : name
|
||||
const childAbs = join(dir, childRel)
|
||||
const st = statSync(childAbs)
|
||||
if (st.isDirectory()) {
|
||||
walk(childRel)
|
||||
} else if (st.isFile()) {
|
||||
out.set(childRel, createHash('sha256').update(readFileSync(childAbs)).digest('hex'))
|
||||
}
|
||||
}
|
||||
}
|
||||
walk('')
|
||||
return out
|
||||
}
|
||||
|
||||
/** Every path where `after` differs from `before`, labelled — the failure message. */
|
||||
function diff(before: Map<string, string>, after: Map<string, string>): string[] {
|
||||
const lines: string[] = []
|
||||
for (const [path, hash] of after) {
|
||||
if (!before.has(path)) lines.push(`ADDED ${path}`)
|
||||
else if (before.get(path) !== hash) lines.push(`CHANGED ${path}`)
|
||||
}
|
||||
for (const path of before.keys()) if (!after.has(path)) lines.push(`REMOVED ${path}`)
|
||||
return lines.sort()
|
||||
}
|
||||
|
||||
describe('a read-only brain writes nothing under `_system/`', () => {
|
||||
let dir: string
|
||||
let brain: Brainy | null = null
|
||||
|
||||
beforeEach(() => {
|
||||
dir = mkdtempSync(join(tmpdir(), 'brainy-readonly-writes-'))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
if (brain) {
|
||||
try {
|
||||
await brain.close()
|
||||
} catch {
|
||||
/* already closed */
|
||||
}
|
||||
brain = null
|
||||
}
|
||||
try {
|
||||
rmSync(dir, { recursive: true, force: true })
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
})
|
||||
|
||||
const systemDir = () => join(dir, '_system')
|
||||
|
||||
/**
|
||||
* A writer seeds a store with nouns, verbs and queryable metadata — enough
|
||||
* that the field registry, the statistics files and the graph index all hold
|
||||
* real content — then closes cleanly.
|
||||
*/
|
||||
async function seedStore(): Promise<void> {
|
||||
const writer = new Brainy({
|
||||
requireSubtype: false,
|
||||
storage: { type: 'filesystem', path: dir }
|
||||
})
|
||||
await writer.init()
|
||||
for (let i = 0; i < 6; i++) {
|
||||
await writer.add({
|
||||
id: `seed-${i}`,
|
||||
data: `seed entity ${i}`,
|
||||
type: i % 2 === 0 ? NounType.Concept : NounType.Document,
|
||||
metadata: { lane: i % 2 === 0 ? 'alpha' : 'beta', rank: i, tags: [`t${i}`, 'shared'] },
|
||||
vector: []
|
||||
})
|
||||
}
|
||||
for (let i = 1; i < 6; i++) {
|
||||
await writer.relate({ from: 'seed-0', to: `seed-${i}`, type: VerbType.RelatedTo })
|
||||
}
|
||||
await writer.flush()
|
||||
await writer.close()
|
||||
}
|
||||
|
||||
it('open → read → close leaves every file under `_system/` byte-identical', async () => {
|
||||
await seedStore()
|
||||
|
||||
const before = snapshotDir(systemDir())
|
||||
expect(before.size, 'the writer left a populated `_system/`').toBeGreaterThan(0)
|
||||
|
||||
brain = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
|
||||
expect(brain.isReadOnly).toBe(true)
|
||||
|
||||
// Exercise the read surface that drives each subsystem: statistics (counts),
|
||||
// a metadata filter (field index + registry), a graph walk (adjacency), a
|
||||
// vector search, and a direct get.
|
||||
await brain.stats()
|
||||
await brain.find({ where: { lane: 'alpha' }, limit: 10 } as any)
|
||||
await brain.find({ where: { tags: 'shared' }, limit: 10 } as any)
|
||||
await brain.find({ connected: { from: 'seed-0', direction: 'out' }, limit: 10 } as any)
|
||||
await brain.get('seed-1')
|
||||
|
||||
await brain.close()
|
||||
brain = null
|
||||
|
||||
const after = snapshotDir(systemDir())
|
||||
const changes = diff(before, after)
|
||||
expect(changes, `a reader modified \`_system/\`:\n${changes.join('\n')}`).toEqual([])
|
||||
}, 120_000)
|
||||
|
||||
it('names the four files that used to change — the measured shape of the defect', async () => {
|
||||
await seedStore()
|
||||
const before = snapshotDir(systemDir())
|
||||
|
||||
// These are the exact paths the base rewrote. Naming them keeps the pin
|
||||
// honest about what it caught: if a future change reintroduces the write,
|
||||
// the test above fails and this one says which subsystem did it.
|
||||
const previouslyRewritten = [
|
||||
'__metadata_field_registry__.json.gz',
|
||||
'type-statistics.json.gz',
|
||||
'subtype-statistics.json.gz',
|
||||
'verb-subtype-statistics.json.gz'
|
||||
]
|
||||
for (const name of previouslyRewritten) {
|
||||
expect(before.has(name), `fixture must contain ${name}`).toBe(true)
|
||||
}
|
||||
|
||||
brain = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
|
||||
await brain.stats()
|
||||
await brain.find({ where: { lane: 'alpha' }, limit: 10 } as any)
|
||||
await brain.close()
|
||||
brain = null
|
||||
|
||||
const after = snapshotDir(systemDir())
|
||||
for (const name of previouslyRewritten) {
|
||||
expect(after.get(name), `${name} was rewritten by a reader`).toBe(before.get(name))
|
||||
}
|
||||
}, 120_000)
|
||||
|
||||
it('a reader that only opens and closes — touching nothing — writes nothing', async () => {
|
||||
await seedStore()
|
||||
const before = snapshotDir(systemDir())
|
||||
|
||||
brain = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
|
||||
await brain.close()
|
||||
brain = null
|
||||
|
||||
const changes = diff(before, snapshotDir(systemDir()))
|
||||
expect(changes, `an idle reader modified \`_system/\`:\n${changes.join('\n')}`).toEqual([])
|
||||
}, 120_000)
|
||||
|
||||
it('two readers in sequence each leave the store exactly as they found it', async () => {
|
||||
await seedStore()
|
||||
const before = snapshotDir(systemDir())
|
||||
|
||||
for (let i = 0; i < 2; i++) {
|
||||
const reader = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
|
||||
await reader.find({ where: { lane: 'beta' }, limit: 10 } as any)
|
||||
await reader.close()
|
||||
const changes = diff(before, snapshotDir(systemDir()))
|
||||
expect(changes, `reader ${i + 1} modified \`_system/\`:\n${changes.join('\n')}`).toEqual([])
|
||||
}
|
||||
}, 120_000)
|
||||
|
||||
it('the store outside `_system/` is untouched too — a reader writes nowhere', async () => {
|
||||
await seedStore()
|
||||
const before = snapshotDir(dir)
|
||||
|
||||
brain = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: dir } })
|
||||
await brain.stats()
|
||||
await brain.find({ where: { lane: 'alpha' }, limit: 10 } as any)
|
||||
await brain.close()
|
||||
brain = null
|
||||
|
||||
const changes = diff(before, snapshotDir(dir))
|
||||
expect(changes, `a reader modified the store:\n${changes.join('\n')}`).toEqual([])
|
||||
}, 120_000)
|
||||
|
||||
it('a WRITER still persists on close — the guard did not disarm the write path', async () => {
|
||||
await seedStore()
|
||||
const before = snapshotDir(systemDir())
|
||||
|
||||
const writer = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
|
||||
await writer.init()
|
||||
await writer.add({
|
||||
id: 'after-reader',
|
||||
data: 'a new row',
|
||||
type: NounType.Concept,
|
||||
metadata: { lane: 'gamma', rank: 99 },
|
||||
vector: []
|
||||
})
|
||||
await writer.close()
|
||||
|
||||
// The writer's close DID move `_system/` — that is the whole point of the
|
||||
// asymmetry, and the guard must not have flattened it.
|
||||
expect(diff(before, snapshotDir(systemDir())).length).toBeGreaterThan(0)
|
||||
|
||||
// And the row is really there on the next open.
|
||||
const reopened = new Brainy({ requireSubtype: false, storage: { type: 'filesystem', path: dir } })
|
||||
await reopened.init()
|
||||
brain = reopened
|
||||
const hits = await reopened.find({ where: { lane: 'gamma' }, limit: 10 } as any)
|
||||
expect(hits.length).toBe(1)
|
||||
}, 120_000)
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue