mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(db): keep SQLite temp files in memory so trips can be deleted (#2518)
Deleting a trip failed with SQLITE_IOERR_GETTEMPPATH when the container runs with a read-only root filesystem and no tmpfs on /tmp. The cascade behind DELETE FROM trips keeps a statement journal, SQLite moves it to a temp file once it passes 64 KiB, and it looks for a directory for that file in SQLITE_TMPDIR, TMPDIR, /var/tmp, /usr/tmp, /tmp and the working directory. None of them is writable there. Installs on the shipped compose file have a tmpfs on /tmp and never hit it. Every connection the server opens now comes from openDatabase() in src/db/connection.ts, which sets temp_store = MEMORY: the main database (also when it is reopened after a restore), each plugin's own database and the uploaded backup checked before a restore. reset-admin.js and the key rotation script cannot import src/ in the image, so they set the pragma themselves. A test pins that nothing else in src/ constructs a handle, and one deletes a full trip on the real schema and watches for temp files.
This commit is contained in:
@@ -50,6 +50,10 @@ const db = new Database(dbPath);
|
||||
// journal mode below waits for a busy server instead of failing on the spot.
|
||||
// Five seconds, same as src/db/database.ts.
|
||||
db.exec('PRAGMA busy_timeout = 5000');
|
||||
// Temp files in memory, like every connection src/db/connection.ts opens: a
|
||||
// container with a read-only root and no tmpfs on /tmp has nowhere to put them
|
||||
// (#2518).
|
||||
db.exec('PRAGMA temp_store = MEMORY');
|
||||
db.exec(`PRAGMA journal_mode = ${journalMode}`);
|
||||
db.exec(`PRAGMA synchronous = ${synchronous}`);
|
||||
|
||||
|
||||
@@ -184,6 +184,10 @@ async function main() {
|
||||
|
||||
const db = new Database(dbPath);
|
||||
db.pragma('busy_timeout = 5000');
|
||||
// Temp files in memory, like every connection src/db/connection.ts opens: a
|
||||
// container with a read-only root and no tmpfs on /tmp has nowhere to put
|
||||
// them, and the rotation below runs as one long transaction (#2518).
|
||||
db.pragma('temp_store = MEMORY');
|
||||
|
||||
// The rollback copy has to be a consistent snapshot, not a plain file copy:
|
||||
// this script is documented as running against a live server, so in WAL mode
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import Database from 'better-sqlite3';
|
||||
|
||||
/**
|
||||
* Opens a SQLite connection. Every handle the server opens comes from here (the
|
||||
* main database, each plugin's own database, a backup being checked before a
|
||||
* restore), so what has to hold for all of them is set in one place.
|
||||
*
|
||||
* Temporary storage stays in memory. Statement journals, sort runs and transient
|
||||
* indices move to a temp file once they outgrow a small in-memory buffer, and
|
||||
* SQLite looks for a directory to hold that file in SQLITE_TMPDIR, TMPDIR,
|
||||
* /var/tmp, /usr/tmp, /tmp and the working directory. A container with a
|
||||
* read-only root filesystem and no tmpfs on /tmp has none of them writable, so
|
||||
* the first statement that needs a temp file fails with SQLITE_IOERR_GETTEMPPATH.
|
||||
* Deleting a trip is where that showed: the cascade keeps a statement journal
|
||||
* larger than the 64 KiB SQLite holds before it spills to disk (#2518).
|
||||
*
|
||||
* The data directory is the only place TREK can count on being able to write,
|
||||
* and SQLite has no supported per-connection way to put temp files there, so
|
||||
* they stay in RAM. The shipped compose file already had them there, its /tmp
|
||||
* being a tmpfs. VACUUM INTO, the only VACUUM TREK runs, writes straight into
|
||||
* its target file and builds no temporary database.
|
||||
*/
|
||||
export function openDatabase(filename: string, options?: Database.Options): Database.Database {
|
||||
const db = new Database(filename, options);
|
||||
db.pragma('temp_store = MEMORY');
|
||||
return db;
|
||||
}
|
||||
@@ -1,7 +1,8 @@
|
||||
import Database from 'better-sqlite3';
|
||||
import type Database from 'better-sqlite3';
|
||||
import path from 'path';
|
||||
import fs from 'fs';
|
||||
import { readEnv } from '../app-config';
|
||||
import { openDatabase } from './connection';
|
||||
import { applyDurabilityPragmas } from './durability';
|
||||
import { createTables } from './schema';
|
||||
import { runMigrations } from './migrations';
|
||||
@@ -39,7 +40,7 @@ function initDb(): void {
|
||||
_db = null;
|
||||
}
|
||||
|
||||
_db = new Database(dbPath);
|
||||
_db = openDatabase(dbPath);
|
||||
// Ahead of the journal switch now: changing journal_mode needs an exclusive
|
||||
// lock, which a sibling process (reset-admin, the rotation script) may hold.
|
||||
_db.exec('PRAGMA busy_timeout = 5000');
|
||||
|
||||
@@ -4,7 +4,8 @@ import path from 'path';
|
||||
import { pipeline } from 'node:stream/promises';
|
||||
import { readEnv } from '../../app-config';
|
||||
import fs from 'fs';
|
||||
import Database from 'better-sqlite3';
|
||||
import type Database from 'better-sqlite3';
|
||||
import { openDatabase } from '../../db/connection';
|
||||
import { db, closeDb, reinitialize } from '../../db/database';
|
||||
import { VALID_INTERVALS } from './auto-backup.settings';
|
||||
import { invalidatePermissionsCache } from '../permissions/permissions-cache';
|
||||
@@ -436,7 +437,7 @@ export async function restoreFromZip(storage: StorageService, zipPath: string):
|
||||
|
||||
let uploadedDb: InstanceType<typeof Database> | null = null;
|
||||
try {
|
||||
uploadedDb = new Database(extractedDb, { readonly: true });
|
||||
uploadedDb = openDatabase(extractedDb, { readonly: true });
|
||||
|
||||
const integrityResult = uploadedDb.prepare('PRAGMA integrity_check').get() as { integrity_check: string };
|
||||
if (integrityResult.integrity_check !== 'ok') {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import Database from 'better-sqlite3';
|
||||
import type { Database as Db } from 'better-sqlite3';
|
||||
import { openDatabase } from '../../../db/connection';
|
||||
import { pluginDataDir, pluginDbFile, pluginsDataRoot } from '../paths';
|
||||
|
||||
/**
|
||||
@@ -63,7 +63,7 @@ export class PluginDataDb {
|
||||
constructor(pluginId: string) {
|
||||
this.pluginId = pluginId;
|
||||
fs.mkdirSync(pluginDataDir(pluginId), { recursive: true });
|
||||
this.db = new Database(pluginDbFile(pluginId));
|
||||
this.db = openDatabase(pluginDbFile(pluginId));
|
||||
this.db.pragma('journal_mode = WAL');
|
||||
this.db.pragma('foreign_keys = ON');
|
||||
openDbs.add(this);
|
||||
|
||||
@@ -0,0 +1,207 @@
|
||||
import fs from 'fs';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import Database from 'better-sqlite3';
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||
|
||||
import { openDatabase } from '../../../src/db/connection';
|
||||
import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { TripsService } from '../../../src/nest/trips/trips.service';
|
||||
import {
|
||||
createDayAssignment,
|
||||
createDayNote,
|
||||
createPackingItem,
|
||||
createPlace,
|
||||
createTrip,
|
||||
createUser,
|
||||
} from '../../helpers/factories';
|
||||
|
||||
// #2518: a container with a read-only root filesystem and no tmpfs on /tmp
|
||||
// gives SQLite nowhere to put a temp file, so deleting a trip failed with
|
||||
// SQLITE_IOERR_GETTEMPPATH. Every connection TREK opens keeps its temp storage
|
||||
// in memory now, and these tests pin that down.
|
||||
|
||||
const SERVER_ROOT = path.resolve(__dirname, '../../..');
|
||||
const MEMORY = 2; // PRAGMA temp_store: 0 default, 1 file, 2 memory
|
||||
|
||||
let tmpDir: string;
|
||||
let schemaFile: string;
|
||||
|
||||
beforeAll(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-connection-'));
|
||||
// Real schema, built once and copied per test: the delete below has to
|
||||
// cascade through the same tables and foreign keys a live install has.
|
||||
schemaFile = path.join(tmpDir, 'schema.db');
|
||||
const db = openDatabase(schemaFile);
|
||||
db.exec('PRAGMA journal_mode = WAL');
|
||||
db.exec('PRAGMA foreign_keys = ON');
|
||||
createTables(db);
|
||||
runMigrations(db);
|
||||
db.exec('PRAGMA wal_checkpoint(TRUNCATE)');
|
||||
db.close();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function freshCopy(name: string): string {
|
||||
const file = path.join(tmpDir, `${name}.db`);
|
||||
fs.copyFileSync(schemaFile, file);
|
||||
return file;
|
||||
}
|
||||
|
||||
describe('openDatabase', () => {
|
||||
it('keeps temp storage in memory on a read-write handle', () => {
|
||||
const db = openDatabase(freshCopy('rw'));
|
||||
try {
|
||||
expect(db.pragma('temp_store', { simple: true })).toBe(MEMORY);
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps temp storage in memory on a read-only handle', () => {
|
||||
const db = openDatabase(freshCopy('ro'), { readonly: true });
|
||||
try {
|
||||
expect(db.readonly).toBe(true);
|
||||
expect(db.pragma('temp_store', { simple: true })).toBe(MEMORY);
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('is what the server opens its main database with', async () => {
|
||||
const { db } = await import('../../../src/db/database');
|
||||
expect(db.pragma('temp_store', { simple: true })).toBe(MEMORY);
|
||||
});
|
||||
});
|
||||
|
||||
// SQLite names every temp file etilqs_<random>. Linux unlinks the file the
|
||||
// moment it is opened, so it only shows among the process's open descriptors;
|
||||
// Windows keeps it in the temp directory until it is closed. macOS offers
|
||||
// neither view, which is why the test below does not run there.
|
||||
const canSeeTempFiles = process.platform === 'linux' || process.platform === 'win32';
|
||||
|
||||
function sqliteTempFiles(): string[] {
|
||||
if (process.platform === 'linux') {
|
||||
return fs
|
||||
.readdirSync('/proc/self/fd')
|
||||
.flatMap((fd) => {
|
||||
try {
|
||||
return [fs.readlinkSync(`/proc/self/fd/${fd}`)];
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
})
|
||||
.filter((target) => target.includes('etilqs_'));
|
||||
}
|
||||
return fs.readdirSync(os.tmpdir()).filter((name) => name.startsWith('etilqs_'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a trip the size of a real one on `db`, deletes it through
|
||||
* TripsService.remove and reports which SQLite temp files were open while the
|
||||
* delete ran. A trigger calls back into the test after the cascade, still
|
||||
* inside the statement, which is when a spilled statement journal is open.
|
||||
*/
|
||||
function deleteTripAndWatchTempFiles(db: Database.Database): {
|
||||
tempFiles: string[];
|
||||
tripLeft: number;
|
||||
placesLeft: number;
|
||||
} {
|
||||
db.exec('PRAGMA journal_mode = WAL');
|
||||
db.exec('PRAGMA foreign_keys = ON');
|
||||
|
||||
const { user } = createUser(db);
|
||||
const trip = createTrip(db, user.id, {
|
||||
title: 'Niederlande Hoofddorp',
|
||||
start_date: '2026-06-01',
|
||||
end_date: '2026-06-14',
|
||||
});
|
||||
const days = db.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];
|
||||
for (let i = 0; i < 25; i++) {
|
||||
const place = createPlace(db, trip.id, { name: `Ort ${i}`, description: 'Grachten und Tulpen. '.repeat(10) });
|
||||
createDayAssignment(db, days[i % days.length].id, place.id);
|
||||
}
|
||||
days.forEach((day) => createDayNote(db, day.id, trip.id));
|
||||
for (let i = 0; i < 20; i++) createPackingItem(db, trip.id, { name: `Teil ${i}` });
|
||||
|
||||
const before = new Set(sqliteTempFiles());
|
||||
const seen = new Set<string>();
|
||||
db.function('trek_watch_temp_files', () => {
|
||||
for (const file of sqliteTempFiles()) if (!before.has(file)) seen.add(file);
|
||||
return null;
|
||||
});
|
||||
db.exec('CREATE TRIGGER trek_watch_trip_delete AFTER DELETE ON trips BEGIN SELECT trek_watch_temp_files(); END');
|
||||
|
||||
const none = undefined as never;
|
||||
const trips = new TripsService(new DatabaseService(db), none, none, none, none, none, none, none, none, none);
|
||||
trips.remove(trip.id, user.id, 'user');
|
||||
|
||||
return {
|
||||
tempFiles: [...seen],
|
||||
tripLeft: (db.prepare('SELECT COUNT(*) AS n FROM trips WHERE id = ?').get(trip.id) as { n: number }).n,
|
||||
placesLeft: (db.prepare('SELECT COUNT(*) AS n FROM places WHERE trip_id = ?').get(trip.id) as { n: number }).n,
|
||||
};
|
||||
}
|
||||
|
||||
describe.runIf(canSeeTempFiles)('deleting a trip with content (#2518)', () => {
|
||||
it('needs a temp file on a connection left at the SQLite default', () => {
|
||||
// The control: proves the watcher sees temp files, and that this delete is
|
||||
// one SQLite cannot finish without one where no temp directory is writable.
|
||||
const db = new Database(freshCopy('default'));
|
||||
try {
|
||||
const result = deleteTripAndWatchTempFiles(db);
|
||||
expect(result.tempFiles.length).toBeGreaterThan(0);
|
||||
expect(result.tripLeft).toBe(0);
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('never touches a temp file on a connection TREK opens', () => {
|
||||
const db = openDatabase(freshCopy('trek'));
|
||||
try {
|
||||
const result = deleteTripAndWatchTempFiles(db);
|
||||
expect(result.tempFiles).toEqual([]);
|
||||
expect(result.tripLeft).toBe(0);
|
||||
expect(result.placesLeft).toBe(0);
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('every SQLite connection goes through openDatabase', () => {
|
||||
function sourceFiles(dir: string): string[] {
|
||||
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) return sourceFiles(full);
|
||||
return entry.name.endsWith('.ts') ? [full] : [];
|
||||
});
|
||||
}
|
||||
|
||||
it('leaves no other place in src/ constructing a better-sqlite3 handle', () => {
|
||||
const offenders = sourceFiles(path.join(SERVER_ROOT, 'src')).filter((file) => {
|
||||
if (file === path.join(SERVER_ROOT, 'src', 'db', 'connection.ts')) return false;
|
||||
const source = fs.readFileSync(file, 'utf8');
|
||||
// A value import (not `import type`) under whatever local name, then `new` on it.
|
||||
const imported = /import\s+(?!type\b)(\w+)[^;]*?from\s+['"]better-sqlite3['"]/.exec(source)?.[1];
|
||||
const required = /(\w+)\s*=\s*require\(\s*['"]better-sqlite3['"]\s*\)/.exec(source)?.[1];
|
||||
return [imported, required].some((name) => name && new RegExp(`new\\s+${name}\\s*\\(`).test(source));
|
||||
});
|
||||
expect(offenders.map((file) => path.relative(SERVER_ROOT, file))).toEqual([]);
|
||||
});
|
||||
|
||||
it('is mirrored by the standalone scripts that cannot import it', () => {
|
||||
// reset-admin.js and the key rotation run in the container without src/,
|
||||
// so they set the pragma themselves.
|
||||
for (const script of ['reset-admin.js', 'scripts/migrate-encryption.ts']) {
|
||||
const source = fs.readFileSync(path.join(SERVER_ROOT, script), 'utf8');
|
||||
expect(source, script).toMatch(/PRAGMA temp_store = MEMORY|pragma\('temp_store = MEMORY'\)/);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -856,6 +856,7 @@ describe('BACKUP-061 restoreFromZip extraction', () => {
|
||||
.mockReturnValueOnce({
|
||||
all: vi.fn().mockReturnValue([{ name: 'users' }, { name: 'trips' }, { name: 'trip_members' }, { name: 'places' }, { name: 'days' }]),
|
||||
}),
|
||||
pragma: vi.fn(),
|
||||
close: vi.fn(),
|
||||
};
|
||||
DatabaseMock.mockImplementation(function () {
|
||||
@@ -1059,6 +1060,7 @@ describe('BACKUP-042 restoreFromZip — integrity check fails', () => {
|
||||
get: vi.fn().mockReturnValue({ integrity_check: 'corruption' }),
|
||||
all: vi.fn(),
|
||||
}),
|
||||
pragma: vi.fn(),
|
||||
close: vi.fn(),
|
||||
};
|
||||
DatabaseMock.mockImplementation(function () {
|
||||
@@ -1095,6 +1097,7 @@ describe('BACKUP-043 restoreFromZip — missing required table', () => {
|
||||
.mockReturnValueOnce({
|
||||
all: vi.fn().mockReturnValue([{ name: 'users' }, { name: 'trips' }]),
|
||||
}),
|
||||
pragma: vi.fn(),
|
||||
close: vi.fn(),
|
||||
};
|
||||
DatabaseMock.mockImplementation(function () {
|
||||
@@ -1156,6 +1159,7 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => {
|
||||
{ name: 'days' },
|
||||
]),
|
||||
}),
|
||||
pragma: vi.fn(),
|
||||
close: vi.fn(),
|
||||
};
|
||||
DatabaseMock.mockImplementation(function () {
|
||||
@@ -1166,7 +1170,7 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => {
|
||||
|
||||
it('BACKUP-045a — returns { success: true } on full success', async () => {
|
||||
setupSuccessfulExtraction();
|
||||
setupAllTablesPresent();
|
||||
const uploaded = setupAllTablesPresent();
|
||||
|
||||
fsMock.existsSync.mockImplementation((p: string) => {
|
||||
if (String(p).endsWith('travel.db')) return true;
|
||||
@@ -1180,6 +1184,10 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => {
|
||||
const result = await restoreFromZip(stubStorage(), '/data/tmp/upload.zip');
|
||||
|
||||
expect(result).toEqual({ success: true });
|
||||
// The uploaded database is checked through the shared opener, read-only and
|
||||
// with its temp files in memory like every other handle (#2518).
|
||||
expect(DatabaseMock).toHaveBeenCalledWith(expect.stringContaining('travel.db'), { readonly: true });
|
||||
expect(uploaded.pragma).toHaveBeenCalledWith('temp_store = MEMORY');
|
||||
});
|
||||
|
||||
it('BACKUP-045b — closeDb is called before file copy operations', async () => {
|
||||
@@ -1325,6 +1333,7 @@ describe('BACKUP-046 restoreFromZip — uploads rehydration through storage', ()
|
||||
{ name: 'days' },
|
||||
]),
|
||||
}),
|
||||
pragma: vi.fn(),
|
||||
close: vi.fn(),
|
||||
};
|
||||
DatabaseMock.mockImplementation(function () {
|
||||
|
||||
Reference in New Issue
Block a user