fix(memory): clean stale reindex temp files (#92891)

* fix(memory): clean stale reindex temp files

* fix(memory): harden stale reindex cleanup

* fix(memory): serialize safe reindex cleanup

* fix(memory): satisfy reindex lock lint

---------

Co-authored-by: zengwen <zeng_wen@foxmail.com>
Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
ZengWen-DT
2026-06-15 00:40:43 +08:00
committed by GitHub
parent ecaebfc51b
commit a42bda5b37
4 changed files with 487 additions and 46 deletions
@@ -1,9 +1,23 @@
import fsSync from "node:fs";
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { DatabaseSync } from "node:sqlite";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { openMemoryDatabaseAtPath } from "./manager-db.js";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import {
cleanupAgedMemoryReindexTempFiles,
openMemoryDatabaseAtPath,
openMemoryReindexTempDatabaseAtPath,
} from "./manager-db.js";
import {
acquireMemoryReindexLock,
resolveMemoryReindexLockPath,
tryAcquireMemoryReindexLock,
} from "./manager-reindex-lock.js";
async function expectPathMissing(targetPath: string): Promise<void> {
await expect(fs.access(targetPath)).rejects.toThrow("ENOENT");
}
describe("openMemoryDatabaseAtPath readOnly probe", () => {
let fixtureRoot = "";
@@ -17,6 +31,10 @@ describe("openMemoryDatabaseAtPath readOnly probe", () => {
await fs.rm(fixtureRoot, { recursive: true, force: true });
});
afterEach(() => {
vi.restoreAllMocks();
});
it("allows opening when the database file exists", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
const dir = path.dirname(dbPath);
@@ -41,6 +59,21 @@ describe("openMemoryDatabaseAtPath readOnly probe", () => {
expect(stat.size).toBeGreaterThan(0);
});
it("refuses to create a missing live database while a safe reindex holds the lock", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
await fs.mkdir(path.dirname(dbPath), { recursive: true });
const reindexLock = acquireMemoryReindexLock(dbPath);
expect(() => openMemoryDatabaseAtPath(dbPath, false, true)).toThrow(
/another reindex is active/,
);
await expectPathMissing(dbPath);
reindexLock.release();
const db = openMemoryDatabaseAtPath(dbPath, false, true);
db.close();
});
it("refuses to auto-create an empty database when allowCreate is false", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "absent-index.sqlite");
@@ -54,12 +87,132 @@ describe("openMemoryDatabaseAtPath readOnly probe", () => {
it("allows open with allowCreate=true for temp database creation", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "temp-index.sqlite");
const db = openMemoryDatabaseAtPath(dbPath, false, true);
const db = openMemoryReindexTempDatabaseAtPath(dbPath, false);
db.exec("CREATE TABLE IF NOT EXISTS meta(key TEXT PRIMARY KEY, value TEXT)");
db.close();
await expectPathMissing(resolveMemoryReindexLockPath(dbPath));
const reopen = openMemoryDatabaseAtPath(dbPath, false, false);
expect(reopen).toBeDefined();
reopen.close();
});
});
it("removes aged orphan reindex temp files before opening the live database", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
const dir = path.dirname(dbPath);
await fs.mkdir(dir, { recursive: true });
const seed = new DatabaseSync(dbPath);
seed.exec("CREATE TABLE IF NOT EXISTS meta(key TEXT PRIMARY KEY, value TEXT)");
seed.close();
const orphanBase = `${dbPath}.tmp-11111111-2222-3333-4444-555555555555`;
for (const suffix of ["", "-wal", "-shm"]) {
const filePath = `${orphanBase}${suffix}`;
await fs.writeFile(filePath, "orphan");
const old = new Date(Date.now() - 48 * 60 * 60_000);
await fs.utimes(filePath, old, old);
}
const db = openMemoryDatabaseAtPath(dbPath, false);
db.close();
await expectPathMissing(orphanBase);
await expectPathMissing(`${orphanBase}-wal`);
await expectPathMissing(`${orphanBase}-shm`);
});
it("keeps young reindex temp files during live database startup", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
const dir = path.dirname(dbPath);
await fs.mkdir(dir, { recursive: true });
const seed = new DatabaseSync(dbPath);
seed.exec("CREATE TABLE IF NOT EXISTS meta(key TEXT PRIMARY KEY, value TEXT)");
seed.close();
const activeBase = `${dbPath}.tmp-aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee`;
for (const suffix of ["", "-wal", "-shm"]) {
await fs.writeFile(`${activeBase}${suffix}`, "active");
}
const db = openMemoryDatabaseAtPath(dbPath, false);
db.close();
await expect(fs.access(activeBase)).resolves.toBeUndefined();
await expect(fs.access(`${activeBase}-wal`)).resolves.toBeUndefined();
await expect(fs.access(`${activeBase}-shm`)).resolves.toBeUndefined();
});
it("keeps aged reindex temp files while another process holds the reindex lock", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
const dir = path.dirname(dbPath);
await fs.mkdir(dir, { recursive: true });
const seed = new DatabaseSync(dbPath);
seed.exec("CREATE TABLE IF NOT EXISTS meta(key TEXT PRIMARY KEY, value TEXT)");
seed.close();
const activeBase = `${dbPath}.tmp-99999999-aaaa-bbbb-cccc-dddddddddddd`;
for (const suffix of ["", "-wal", "-shm"]) {
const filePath = `${activeBase}${suffix}`;
await fs.writeFile(filePath, "active");
const old = new Date(Date.now() - 48 * 60 * 60_000);
await fs.utimes(filePath, old, old);
}
const reindexLock = acquireMemoryReindexLock(dbPath);
cleanupAgedMemoryReindexTempFiles(dbPath);
const db = openMemoryDatabaseAtPath(dbPath, false);
db.close();
await expect(fs.access(activeBase)).resolves.toBeUndefined();
await expect(fs.access(`${activeBase}-wal`)).resolves.toBeUndefined();
await expect(fs.access(`${activeBase}-shm`)).resolves.toBeUndefined();
reindexLock.release();
cleanupAgedMemoryReindexTempFiles(dbPath);
await expectPathMissing(activeBase);
await expectPathMissing(`${activeBase}-wal`);
await expectPathMissing(`${activeBase}-shm`);
});
it("keeps aged reindex temp files while the live database is absent", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
await fs.mkdir(path.dirname(dbPath), { recursive: true });
const orphanBase = `${dbPath}.tmp-abcdef12-aaaa-bbbb-cccc-123456789abc`;
await fs.writeFile(orphanBase, "recovery candidate");
const old = new Date(Date.now() - 48 * 60 * 60_000);
await fs.utimes(orphanBase, old, old);
const db = openMemoryDatabaseAtPath(dbPath, false, true);
db.close();
await expect(fs.access(orphanBase)).resolves.toBeUndefined();
});
it("serializes safe reindexes and releases the lock for the next owner", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
await fs.mkdir(path.dirname(dbPath), { recursive: true });
const first = acquireMemoryReindexLock(dbPath);
expect(tryAcquireMemoryReindexLock(dbPath)).toBeUndefined();
expect(() => acquireMemoryReindexLock(dbPath)).toThrow(/another reindex is active/);
first.release();
const second = tryAcquireMemoryReindexLock(dbPath);
expect(second).toBeDefined();
second?.release();
await expect(fs.access(resolveMemoryReindexLockPath(dbPath))).resolves.toBeUndefined();
});
it("does not block database startup when orphan discovery fails", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
await fs.mkdir(path.dirname(dbPath), { recursive: true });
const seed = new DatabaseSync(dbPath);
seed.close();
vi.spyOn(fsSync, "readdirSync").mockImplementationOnce(() => {
throw Object.assign(new Error("scan failed"), { code: "EACCES" });
});
const db = openMemoryDatabaseAtPath(dbPath, false);
db.close();
});
});
+212 -26
View File
@@ -1,4 +1,5 @@
// Memory Core plugin module implements manager db behavior.
import fs from "node:fs";
import path from "node:path";
import type { DatabaseSync } from "node:sqlite";
import {
@@ -7,35 +8,124 @@ import {
ensureDir,
requireNodeSqlite,
} from "openclaw/plugin-sdk/memory-core-host-engine-storage";
import {
acquireMemoryReindexLock,
tryAcquireMemoryReindexLock,
type MemoryReindexLockHandle,
} from "./manager-reindex-lock.js";
export function openMemoryDatabaseAtPath(
dbPath: string,
allowExtension: boolean,
allowCreate = true,
): DatabaseSync {
const dir = path.dirname(dbPath);
ensureDir(dir);
const { DatabaseSync } = requireNodeSqlite();
// When allowCreate is false, probe with readOnly first.
// DatabaseSync auto-creates the file in read-write mode, which
// produces an empty database with schema but no meta row when the
// file is momentarily absent during an index swap. readOnly: true
// throws SQLITE_CANTOPEN when the file does not exist, preventing
// the auto-create race.
if (!allowCreate) {
try {
const probe = new DatabaseSync(dbPath, { readOnly: true });
probe.close();
} catch (err) {
const msg = (err as Error).message ?? "";
if (msg.includes("unable to open database file") || msg.includes("SQLITE_CANTOPEN")) {
throw new Error(
`Memory database not found at ${dbPath}; refusing to auto-create an empty database during an index swap window.`,
{ cause: err },
);
}
// Hard-killed safe reindexes cannot run JS cleanup on their temp DB triplet.
// Startup only removes old sibling triplets so another live process can still
// own a young temp DB without losing its in-flight rebuild.
const reindexTempFileWithoutLockMinAgeMs = 24 * 60 * 60_000;
const reindexTempUuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/;
const memoryIndexFileSuffixes = ["", "-wal", "-shm"] as const;
const reindexTempEntrySuffixes = ["-wal", "-shm", ""] as const;
function resolveReindexTempBaseName(dbBaseName: string, entryName: string): string | undefined {
for (const suffix of reindexTempEntrySuffixes) {
if (!entryName.endsWith(suffix)) {
continue;
}
const baseName = entryName.slice(0, entryName.length - suffix.length);
const tempPrefix = `${dbBaseName}.tmp-`;
if (!baseName.startsWith(tempPrefix)) {
continue;
}
const uuid = baseName.slice(tempPrefix.length);
if (reindexTempUuidPattern.test(uuid)) {
return baseName;
}
}
return undefined;
}
function isRegularFile(filePath: string): boolean {
try {
return fs.statSync(filePath).isFile();
} catch {
return false;
}
}
export function cleanupAgedMemoryReindexTempFiles(dbPath: string, nowMs = Date.now()): void {
// A missing live database can be the brief Windows swap window. Never delete
// the only complete temp candidate while the canonical path is absent.
if (!isRegularFile(dbPath)) {
return;
}
const dir = path.dirname(dbPath);
const dbBaseName = path.basename(dbPath);
let reindexLock: MemoryReindexLockHandle | undefined;
try {
reindexLock = tryAcquireMemoryReindexLock(dbPath);
} catch {
// Startup cleanup is best effort; the actual reindex path acquires the same
// lock strictly before it creates or publishes a replacement database.
return;
}
if (!reindexLock) {
return;
}
try {
const tempBaseNames = new Set<string>();
let entries: fs.Dirent[];
try {
entries = fs.readdirSync(dir, { withFileTypes: true });
} catch {
return;
}
for (const entry of entries) {
if (!entry.isFile()) {
continue;
}
const tempBaseName = resolveReindexTempBaseName(dbBaseName, entry.name);
if (tempBaseName) {
tempBaseNames.add(tempBaseName);
}
}
for (const tempBaseName of tempBaseNames) {
if (!isRegularFile(dbPath)) {
return;
}
const filePaths = memoryIndexFileSuffixes.map((suffix) =>
path.join(dir, `${tempBaseName}${suffix}`),
);
const stats: fs.Stats[] = [];
let hasUnknownFileState = false;
for (const filePath of filePaths) {
try {
stats.push(fs.statSync(filePath));
} catch (err) {
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
hasUnknownFileState = true;
break;
}
}
}
if (hasUnknownFileState || stats.length === 0) {
continue;
}
const newestMtimeMs = Math.max(...stats.map((stat) => stat.mtimeMs));
if (nowMs - newestMtimeMs < reindexTempFileWithoutLockMinAgeMs) {
continue;
}
for (const filePath of filePaths) {
try {
fs.rmSync(filePath, { force: true });
} catch {}
}
}
} finally {
try {
reindexLock.release();
} catch {}
}
}
function openConfiguredMemoryDatabaseAtPath(dbPath: string, allowExtension: boolean): DatabaseSync {
const { DatabaseSync } = requireNodeSqlite();
const db = new DatabaseSync(dbPath, { allowExtension });
configureMemorySqliteWalMaintenance(db, { databasePath: dbPath });
// busy_timeout is per-connection and resets to 0 on restart.
@@ -45,6 +135,102 @@ export function openMemoryDatabaseAtPath(
return db;
}
type ExistingMemoryDatabaseOpenResult =
| { status: "opened"; db: DatabaseSync }
| { status: "missing"; cause: unknown };
function isMemoryDatabaseMissingError(err: unknown): boolean {
const message = err instanceof Error ? err.message : String(err);
return message.includes("unable to open database file") || message.includes("SQLITE_CANTOPEN");
}
function tryOpenExistingMemoryDatabaseAtPath(
dbPath: string,
allowExtension: boolean,
): ExistingMemoryDatabaseOpenResult {
const { DatabaseSync } = requireNodeSqlite();
let probe: DatabaseSync;
try {
probe = new DatabaseSync(dbPath, { readOnly: true });
} catch (err) {
if (isMemoryDatabaseMissingError(err)) {
return { status: "missing", cause: err };
}
throw err;
}
// Keep the read-only handle open until the read-write handle exists. On
// Windows this prevents a safe reindex from creating an absent-path window.
let db: DatabaseSync;
try {
db = openConfiguredMemoryDatabaseAtPath(dbPath, allowExtension);
} catch (err) {
try {
probe.close();
} catch {}
throw err;
}
try {
probe.close();
} catch (err) {
closeMemoryDatabase(db);
throw err;
}
return { status: "opened", db };
}
export function openMemoryDatabaseAtPath(
dbPath: string,
allowExtension: boolean,
allowCreate = true,
): DatabaseSync {
const dir = path.dirname(dbPath);
ensureDir(dir);
cleanupAgedMemoryReindexTempFiles(dbPath);
const existing = tryOpenExistingMemoryDatabaseAtPath(dbPath, allowExtension);
if (existing.status === "opened") {
return existing.db;
}
if (!allowCreate) {
throw new Error(
`Memory database not found at ${dbPath}; refusing to auto-create an empty database during an index swap window.`,
{ cause: existing.cause },
);
}
// A missing canonical path can be an initial create or the Windows swap
// window. Only the safe-reindex owner may create or publish during that gap.
const openLock = acquireMemoryReindexLock(dbPath);
let db: DatabaseSync;
try {
const lockedExisting = tryOpenExistingMemoryDatabaseAtPath(dbPath, allowExtension);
db =
lockedExisting.status === "opened"
? lockedExisting.db
: openConfiguredMemoryDatabaseAtPath(dbPath, allowExtension);
} catch (err) {
try {
openLock.release();
} catch {}
throw err;
}
try {
openLock.release();
} catch (err) {
closeMemoryDatabase(db);
throw err;
}
return db;
}
export function openMemoryReindexTempDatabaseAtPath(
dbPath: string,
allowExtension: boolean,
): DatabaseSync {
ensureDir(path.dirname(dbPath));
return openConfiguredMemoryDatabaseAtPath(dbPath, allowExtension);
}
export function closeMemoryDatabase(db: DatabaseSync): void {
closeMemorySqliteWalMaintenance(db);
db.close();
@@ -0,0 +1,83 @@
// Memory Core plugin module implements cross-process safe-reindex locking.
// The dedicated sibling DB follows custom store paths and relies on SQLite to
// release its exclusive transaction automatically after process/container death.
import type { DatabaseSync } from "node:sqlite";
import { requireNodeSqlite } from "openclaw/plugin-sdk/memory-core-host-engine-storage";
export type MemoryReindexLockHandle = {
release: () => void;
};
export function resolveMemoryReindexLockPath(dbPath: string): string {
return `${dbPath}.reindex-lock.sqlite`;
}
function isSqliteBusyError(err: unknown): boolean {
const code = (err as { code?: unknown }).code;
if (code === "SQLITE_BUSY" || code === "SQLITE_LOCKED") {
return true;
}
const message = err instanceof Error ? err.message : String(err);
return /SQLITE_(?:BUSY|LOCKED)|database is locked/i.test(message);
}
function openMemoryReindexLockDatabase(dbPath: string): DatabaseSync {
const lockPath = resolveMemoryReindexLockPath(dbPath);
const { DatabaseSync } = requireNodeSqlite();
const lockDb = new DatabaseSync(lockPath);
try {
lockDb.exec("PRAGMA busy_timeout = 0");
return lockDb;
} catch (err) {
try {
lockDb.close();
} catch {}
throw err;
}
}
export function tryAcquireMemoryReindexLock(dbPath: string): MemoryReindexLockHandle | undefined {
const lockDb = openMemoryReindexLockDatabase(dbPath);
try {
// SQLite releases this transaction automatically when a process or
// container dies, so ownership never depends on PID namespaces or leases.
lockDb.exec("BEGIN EXCLUSIVE");
} catch (err) {
lockDb.close();
if (isSqliteBusyError(err)) {
return undefined;
}
throw err;
}
return {
release: () => {
let releaseError: unknown;
try {
lockDb.exec("ROLLBACK");
} catch (err) {
releaseError = err;
}
try {
lockDb.close();
} catch (err) {
releaseError ??= err;
}
if (releaseError) {
throw new Error("Failed to release memory reindex lock", { cause: releaseError });
}
},
};
}
export function acquireMemoryReindexLock(dbPath: string): MemoryReindexLockHandle {
const lock = tryAcquireMemoryReindexLock(dbPath);
if (lock) {
return lock;
}
throw Object.assign(
new Error(
`Memory reindex lock is held at ${resolveMemoryReindexLockPath(dbPath)}; another reindex is active.`,
),
{ code: "SQLITE_BUSY" },
);
}
@@ -45,8 +45,13 @@ import {
type EmbeddingProviderId,
type EmbeddingProviderRuntime,
} from "./embeddings.js";
import { runMemoryAtomicReindex } from "./manager-atomic-reindex.js";
import { closeMemoryDatabase, openMemoryDatabaseAtPath } from "./manager-db.js";
import { removeMemoryIndexFiles, runMemoryAtomicReindex } from "./manager-atomic-reindex.js";
import {
cleanupAgedMemoryReindexTempFiles,
closeMemoryDatabase,
openMemoryDatabaseAtPath,
openMemoryReindexTempDatabaseAtPath,
} from "./manager-db.js";
import { isMemoryEmbeddingOperationError } from "./manager-embedding-errors.js";
import {
applyMemoryFallbackProviderState,
@@ -54,6 +59,7 @@ import {
resolveFallbackCurrentProviderId,
type MemoryProviderLifecycleState,
} from "./manager-provider-state.js";
import { acquireMemoryReindexLock, type MemoryReindexLockHandle } from "./manager-reindex-lock.js";
import {
resolveConfiguredScopeHash,
resolveConfiguredSourcesForMeta,
@@ -2381,9 +2387,10 @@ export abstract class MemoryManagerSyncOps {
const dbPath = resolveUserPath(this.settings.store.path);
const tempDbPath = `${dbPath}.tmp-${randomUUID()}`;
const tempDb = openMemoryDatabaseAtPath(tempDbPath, this.settings.store.vector.enabled);
const originalDb = this.db;
let reindexLock: MemoryReindexLockHandle | undefined;
let tempDb: DatabaseSync | undefined;
let tempDbClosed = false;
let originalDbClosed = false;
const originalRetryState = this.snapshotReindexRetryState();
@@ -2417,24 +2424,27 @@ export abstract class MemoryManagerSyncOps {
this.vectorReady = originalDbClosed ? null : originalState.vectorReady;
};
this.db = tempDb;
this.embeddingCacheMirrorDb = originalDb;
this.lastMetaSerialized = null;
this.resetVectorState();
this.fts.available = false;
this.fts.loadError = undefined;
this.ensureSchema();
let nextMeta: MemoryIndexMeta | null;
let publishedIndex = false;
try {
nextMeta = await runMemoryAtomicReindex({
cleanupAgedMemoryReindexTempFiles(dbPath);
reindexLock = acquireMemoryReindexLock(dbPath);
tempDb = openMemoryReindexTempDatabaseAtPath(tempDbPath, this.settings.store.vector.enabled);
const openedTempDb = tempDb;
this.db = openedTempDb;
this.embeddingCacheMirrorDb = originalDb;
this.lastMetaSerialized = null;
this.resetVectorState();
this.fts.available = false;
this.fts.loadError = undefined;
this.ensureSchema();
const nextMeta = await runMemoryAtomicReindex({
targetPath: dbPath,
tempPath: tempDbPath,
beforeTempCleanup: () => {
if (!tempDbClosed) {
closeMemoryDatabase(tempDb);
closeMemoryDatabase(openedTempDb);
tempDbClosed = true;
}
},
@@ -2504,7 +2514,7 @@ export abstract class MemoryManagerSyncOps {
this.writeMeta(meta);
this.pruneEmbeddingCacheIfNeeded?.();
closeMemoryDatabase(tempDb);
closeMemoryDatabase(openedTempDb);
tempDbClosed = true;
closeMemoryDatabase(originalDb);
originalDbClosed = true;
@@ -2520,7 +2530,7 @@ export abstract class MemoryManagerSyncOps {
} catch (err) {
this.embeddingCacheMirrorDb = null;
try {
if (!tempDbClosed && this.db === tempDb) {
if (tempDb && !tempDbClosed && this.db === tempDb) {
closeMemoryDatabase(tempDb);
tempDbClosed = true;
}
@@ -2532,6 +2542,9 @@ export abstract class MemoryManagerSyncOps {
this.vector.dims = this.readMeta()?.vectorDims;
throw err;
}
try {
await removeMemoryIndexFiles(tempDbPath);
} catch {}
restoreOriginalState();
this.restoreReindexRetryState(originalRetryState);
this.markFailedFullReindexRetry({
@@ -2539,6 +2552,12 @@ export abstract class MemoryManagerSyncOps {
sessions: shouldRetrySessionsOnFailure,
});
throw err;
} finally {
try {
reindexLock?.release();
} catch (err) {
log.warn(`failed to release memory reindex lock for ${dbPath}: ${formatErrorMessage(err)}`);
}
}
}