vrr/openclaw
2
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-10-03 01:29:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 29

fix(doctor): batch the NOCOW open-file check and close agent databases before the rewrite (#162919)

Browse source
This commit is contained in:
Peter Steinberger 2026-10-01 11:36:55 -07:00 • committed by GitHub
parent 4487ee82aa
commit 302dbadcb8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 163 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

7
docs/reference/database-schemas/integrity-and-recovery.md
View file

@ -463,8 +463,11 @@ place. This updater-to-Doctor environment contract lets the updater account for
physical identity changes separately from schema migration. Operator runs outside
a managed update keep the normal explicit repair behavior.
Doctor drains its database handles and awaits its inspection workers before the
rewrite. A refusal saying `store files are open (pids: …)` names the processes
Doctor drains its database handles, including pooled auth-profile readers for
all agent stores under the active state directory, and awaits its inspection
workers before the rewrite. It checks every regular file in each store directory
with bounded `fuser` batches so large directories fit the operating system's
argument limit. A refusal saying `store files are open (pids: …)` names the processes
reported by `fuser`. A refusal saying `fuser could not establish that all handles
are closed` includes the inspection error; check that `fuser` is installed and
can inspect processes through `/proc`. Both refusals leave the original store

4
src/commands/doctor-maintenance-state.ts
View file

@ -43,6 +43,10 @@ export function createDoctorMaintenanceState(options: {
const closeResources = async () => {
await resources?.close();
await inspections?.close();
// Auth inspection readers are pooled separately from canonical agent handles.
const { closeAuthProfileReadPool } =
await import("../agents/auth-profiles/sqlite-read-pool.js");
closeAuthProfileReadPool({ kind: "root", rootPath: resolveStateDir(selectedEnv) });
resources = undefined;
inspections = undefined;
};

114
src/commands/doctor-sqlite-nocow.test.ts
View file

@ -3,11 +3,14 @@ import fs from "node:fs";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
import { loadPersistedAuthProfileStore } from "../agents/auth-profiles/persisted.js";
import { openNodeSqliteDatabase } from "../infra/node-sqlite.js";
import { readSqliteReaderDiagnosticsForPath } from "../infra/sqlite-reader-lifecycle.js";
import { isSqlitePathOnBtrfs, setSqliteDirectoryNoCow } from "../infra/sqlite-wal-filesystem.js";
import { openOpenClawAgentDatabase } from "../state/openclaw-agent-db.js";
import { openOpenClawStateDatabase } from "../state/openclaw-state-db.js";
import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js";
import { prepareDoctorDatabasePreflight } from "./doctor-database-preflight.js";
import { beginDoctorMaintenance } from "./doctor-maintenance.js";
import { inspectDoctorSqliteNoCow, repairDoctorSqliteNoCow } from "./doctor-sqlite-nocow.js";
import {
@ -96,19 +99,41 @@ describe("Doctor btrfs NOCOW", () => {
try {
const paths = await maintenance!.run(async () => {
const state = openOpenClawStateDatabase();
const agent = openOpenClawAgentDatabase({ agentId: "main" });
for (const database of [state, agent]) {
const agents = ["main", "secondary", "unconfigured"].map((agentId) =>
openOpenClawAgentDatabase({ agentId }),
);
for (const database of [state, ...agents]) {
database.db.exec(
"CREATE TABLE nocow_payload(value TEXT); INSERT INTO nocow_payload VALUES ('preserved');",
);
}
return inspectDoctorSqliteNoCow([state.path, agent.path]).paths;
for (const agent of agents) {
loadPersistedAuthProfileStore(path.dirname(agent.path));
}
const preflight = await prepareDoctorDatabasePreflight({
cfg: { agents: { list: [{ id: "main" }, { id: "secondary" }] } },
});
expect(preflight.agentDatabaseMigrationDiscovery?.discovery.targets).toHaveLength(3);
return inspectDoctorSqliteNoCow([state.path, ...agents.map((agent) => agent.path)])
.paths;
});
expect(paths).toHaveLength(4);
const toolRunner = vi.mocked(spawnSync).getMockImplementation()!;
const handleCounts: number[] = [];
vi.mocked(spawnSync).mockImplementation((command, args, options) => {
if (command === "fuser") {
for (const pathname of paths) {
handleCounts.push(readSqliteReaderDiagnosticsForPath(pathname).connectionCount);
}
}
return toolRunner(command, args, options);
});
expect(paths).toHaveLength(2);
const originals = paths.map((pathname) => fs.statSync(pathname).ino);
await maintenance!.repairSqliteNoCow(paths);
expect(maintenance!.warnings).toEqual([]);
expect(fixture.exchanges).toBe(2);
expect(fixture.exchanges).toBe(4);
expect(handleCounts.length).toBeGreaterThan(0);
expect(handleCounts.every((count) => count === 0)).toBe(true);
for (const [index, pathname] of paths.entries()) {
expect(fs.statSync(pathname).ino).not.toBe(originals[index]);
const storeDir = path.dirname(pathname);
@ -131,7 +156,7 @@ describe("Doctor btrfs NOCOW", () => {
}
expect(
log.mock.calls.filter(([line]) => line.startsWith("Rewrote SQLite")),
).toHaveLength(2);
).toHaveLength(4);
} finally {
await maintenance?.release();
}
@ -270,6 +295,83 @@ describe("Doctor btrfs NOCOW", () => {
},
);
it.each([
{ name: "many sibling files", count: 2_001, suffix: "", outcome: "holders" },
{ name: "long UTF-8 paths", count: 320, suffix: "界".repeat(60), outcome: "closed" },
{
name: "an inspection failure in a later batch",
count: 320,
suffix: "界".repeat(60),
outcome: "inspection-error",
},
])(
"inspects every file in bounded fuser batches with $name",
async ({ count, suffix, outcome }) => {
seedDatabase();
const nested = path.join(directory, "workshop-skills");
fs.mkdirSync(nested);
const files = [
sqlitePath,
`${sqlitePath}-wal`,
`${sqlitePath}-shm`,
path.join(directory, "sibling.txt"),
];
for (let index = 0; index < count; index++) {
const pathname = path.join(nested, `${index}-${suffix}.txt`);
fs.writeFileSync(pathname, "preserved sibling");
files.push(pathname);
}
const original = fs.statSync(sqlitePath);
const tool = vi.mocked(spawnSync).getMockImplementation()!;
const batches: string[][] = [];
vi.mocked(spawnSync).mockImplementation((command, args, options) => {
if (command !== "fuser") {
return tool(command, args, options);
}
const argv = [...(args ?? [])];
batches.push(argv);
const result = { status: 1, stdout: "", stderr: "", pid: 0, output: [], signal: null };
if (
argv.length > 2_000 ||
argv.reduce((bytes, pathname) => bytes + Buffer.byteLength(pathname, "utf8") + 1, 0) >
64 * 1024
) {
return {
...result,
status: null,
error: Object.assign(new Error("spawnSync fuser E2BIG"), { code: "E2BIG" }),
};
}
if (outcome === "holders") {
return { ...result, status: 0, stdout: batches.length === 1 ? "12345 12345" : "67890" };
}
if (outcome === "inspection-error" && batches.length > 1) {
return { ...result, stderr: "Cannot stat file /proc/123/fd/4: Permission denied" };
}
return result;
});
const notes = (await repair()).join("\n");
expect(notes).not.toContain("E2BIG");
expect(batches.length).toBeGreaterThan(1);
expect(batches.flat().slice(0, files.length).toSorted()).toEqual(files.toSorted());
if (outcome === "closed") {
expect(notes).toContain("Rewrote SQLite store directory with NOCOW");
expect(fixture.exchanges).toBe(1);
expect(fs.readFileSync(files.at(-1)!, "utf8")).toBe("preserved sibling");
expect(batches.flat().filter((pathname) => pathname === sqlitePath)).toHaveLength(2);
} else {
expect(notes).toContain(
outcome === "holders"
? "store files are open (pids: 12345, 67890)"
: "fuser could not establish that all handles are closed: Cannot stat file /proc/123/fd/4: Permission denied",
);
expect(fixture.exchanges).toBe(0);
expect(fs.statSync(sqlitePath).ino).toBe(original.ino);
}
},
);
it.each([
{
name: "open handles",

63
src/commands/doctor-sqlite-nocow.ts
View file

@ -16,6 +16,9 @@ import { DoctorMaintenanceRefusalError } from "../infra/update-doctor-result.js"
import { assertDoctorSqliteMaintenancePathsNotAliased } from "./doctor-sqlite-maintenance-lock.js";
const TOOL_TIMEOUT_MS = 2_000;
const FUSER_MAX_PATHS = 2_000;
// Leave room for the inherited environment and argv pointers under Linux ARG_MAX.
const FUSER_MAX_PATH_BYTES = 64 * 1024;
function hasNoCow(pathname: string): boolean {
const result = spawnSync("lsattr", ["-d", "--", pathname], {
@ -147,29 +150,55 @@ async function quickCheck(pathname: string) {
}
function assertNoOpenFiles(paths: readonly string[]) {
const result = spawnSync(
"fuser",
paths.map((pathname) => path.resolve(pathname)),
{
const batches: string[][] = [];
let batch: string[] = [];
let batchBytes = 0;
for (const pathname of paths) {
const absolute = path.resolve(pathname);
const bytes = Buffer.byteLength(absolute, "utf8") + 1;
if (
batch.length > 0 &&
(batch.length >= FUSER_MAX_PATHS || batchBytes + bytes > FUSER_MAX_PATH_BYTES)
) {
batches.push(batch);
batch = [];
batchBytes = 0;
}
batch.push(absolute);
batchBytes += bytes;
}
if (batch.length > 0) {
batches.push(batch);
}
const pids = new Set<string>();
let inspectionError: string | undefined;
for (const args of batches) {
const result = spawnSync("fuser", args, {
encoding: "utf8",
timeout: TOOL_TIMEOUT_MS,
},
);
const stdout = result.stdout?.trim() ?? "";
const stderr = result.stderr?.trim() ?? "";
if (result.status === 0 && /^\d+(?:\s+\d+)*$/u.test(stdout)) {
const pids = [...new Set(stdout.split(/\s+/u))].join(", ");
});
const stdout = result.stdout?.trim() ?? "";
const stderr = result.stderr?.trim() ?? "";
if (result.status === 0 && /^\d+(?:\s+\d+)*$/u.test(stdout)) {
for (const pid of stdout.split(/\s+/u)) {
pids.add(pid);
}
} else if (result.error || result.status !== 1 || stdout || stderr) {
inspectionError ??=
stderr ||
result.error?.message ||
(result.signal ? `signal ${result.signal}` : stdout || `exit status ${result.status}`);
}
}
if (pids.size > 0) {
throw new Error(
`store files are open (pids: ${pids}); stop processes using this store before retrying`,
`store files are open (pids: ${[...pids].join(", ")}); stop processes using this store before retrying`,
);
}
if (result.error || result.status !== 1 || stdout || stderr) {
const detail =
stderr ||
result.error?.message ||
(result.signal ? `signal ${result.signal}` : stdout || `exit status ${result.status}`);
if (inspectionError) {
throw new Error(
`fuser could not establish that all handles are closed: ${detail}; ensure fuser is installed and can inspect processes using this store`,
`fuser could not establish that all handles are closed: ${inspectionError}; ensure fuser is installed and can inspect processes using this store`,
);
}
}