mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
fix(local-model): end a runtime an earlier server left behind, and keep its version
The supervisor reports the child's pid the moment it is spawned, before it answers, so a server killed during a start still leaves a pid that names the runtime as ours. stop() takes that stored pid (previousPid) and ends a runtime an earlier server left behind when its command line is a binary under our runtime folder, so no caller deletes the folder under a live process. start() takes a version: an installed model keeps the runtime version it was installed with when a later build pins another one. removeLocalModelFiles() refuses a local-model folder that is itself a symbolic link (every check compared against the real path of the root, so a link in its place passed whatever it pointed at). removePartialModelBlobs() deletes what an interrupted download left in the model store, which the runtime answers "not found" for and never deletes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
30ffdaeecd
commit
6319fffce1
4 files changed
+362
-29
No files matched your search
@@ -37,6 +37,7 @@ import {
|
||||
OLLAMA_RUNTIME_PIN,
|
||||
parseInferenceComputeTotal,
|
||||
removeLocalModelFiles,
|
||||
removePartialModelBlobs,
|
||||
RUNTIME_DOWNLOAD_RETRIES,
|
||||
RUNTIME_KEEP_ALIVE,
|
||||
RUNTIME_MAX_RESTARTS,
|
||||
@@ -935,6 +936,42 @@ describe('removeLocalModelFiles', () => {
|
||||
expect(await readFile(join(outside, 'precious.gguf'), 'utf8')).toBe('the only copy');
|
||||
});
|
||||
|
||||
it('refuses a local model folder that is itself a symlink, and deletes nothing behind it', async () => {
|
||||
// <data dir>/local-model is a link to a folder somewhere else that happens to hold the
|
||||
// three names this feature deletes.
|
||||
const elsewhere = await outsideDir();
|
||||
await mkdir(join(elsewhere, 'home'), { recursive: true });
|
||||
await writeFile(join(elsewhere, 'home', 'thesis.txt'), 'the only copy');
|
||||
await mkdir(join(elsewhere, 'runtime'), { recursive: true });
|
||||
await mkdir(join(elsewhere, 'models', 'blobs'), { recursive: true });
|
||||
await writeFile(join(elsewhere, 'models', 'blobs', 'sha256-abc'), 'weights');
|
||||
await mkdir(join(paths.root, '..'), { recursive: true });
|
||||
await symlink(elsewhere, paths.root);
|
||||
try {
|
||||
await expect(removeLocalModelFiles({ models: true, paths })).rejects.toThrow(/symbolic link/);
|
||||
expect(await readFile(join(elsewhere, 'home', 'thesis.txt'), 'utf8')).toBe('the only copy');
|
||||
expect(await readFile(join(elsewhere, 'models', 'blobs', 'sha256-abc'), 'utf8')).toBe('weights');
|
||||
expect(existsSync(join(elsewhere, 'runtime'))).toBe(true);
|
||||
// The partial blobs are not deleted through the link either.
|
||||
await writeFile(join(elsewhere, 'models', 'blobs', 'sha256-def-partial'), 'half');
|
||||
expect(await removePartialModelBlobs(paths)).toBe(0);
|
||||
expect(existsSync(join(elsewhere, 'models', 'blobs', 'sha256-def-partial'))).toBe(true);
|
||||
} finally {
|
||||
// The link itself, never what it points at.
|
||||
await rm(paths.root, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses a local model folder that is a file', async () => {
|
||||
await mkdir(join(paths.root, '..'), { recursive: true });
|
||||
await writeFile(paths.root, 'not a folder');
|
||||
try {
|
||||
await expect(removeLocalModelFiles({ models: true, paths })).rejects.toThrow(/not a folder/);
|
||||
} finally {
|
||||
await rm(paths.root, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('removes a symlink it meets inside a folder, not the tree behind it', async () => {
|
||||
const outside = await outsideDir();
|
||||
await writeFile(join(outside, 'keep.txt'), 'keep');
|
||||
@@ -980,3 +1017,54 @@ describe('removeLocalModelFiles', () => {
|
||||
expect(await readFile(join(outside, 'runtime', 'keep.txt'), 'utf8')).toBe('keep');
|
||||
});
|
||||
});
|
||||
|
||||
describe('removePartialModelBlobs', () => {
|
||||
const blob = (name: string): string => join(paths.modelsDir, 'blobs', name);
|
||||
|
||||
it('deletes what an interrupted download left, and keeps every finished blob and the manifests', async () => {
|
||||
await mkdir(join(paths.modelsDir, 'blobs'), { recursive: true });
|
||||
await mkdir(join(paths.modelsDir, 'manifests', 'registry.ollama.ai'), { recursive: true });
|
||||
await writeFile(blob('sha256-aaa'), 'finished weights');
|
||||
await writeFile(blob('sha256-bbb-partial'), '12345');
|
||||
await writeFile(blob('sha256-bbb-partial-0'), '678');
|
||||
await writeFile(blob('sha256-bbb-partial-1'), '9');
|
||||
await writeFile(join(paths.modelsDir, 'manifests', 'registry.ollama.ai', 'latest'), '{}');
|
||||
|
||||
// The bytes that were freed: 5 + 3 + 1.
|
||||
expect(await removePartialModelBlobs(paths)).toBe(9);
|
||||
expect(existsSync(blob('sha256-bbb-partial'))).toBe(false);
|
||||
expect(existsSync(blob('sha256-bbb-partial-0'))).toBe(false);
|
||||
expect(existsSync(blob('sha256-bbb-partial-1'))).toBe(false);
|
||||
expect(await readFile(blob('sha256-aaa'), 'utf8')).toBe('finished weights');
|
||||
expect(existsSync(join(paths.modelsDir, 'manifests', 'registry.ollama.ai', 'latest'))).toBe(true);
|
||||
});
|
||||
|
||||
it('is fine without a model store, and deletes nothing a second time', async () => {
|
||||
expect(await removePartialModelBlobs(paths)).toBe(0);
|
||||
await mkdir(join(paths.modelsDir, 'blobs'), { recursive: true });
|
||||
await writeFile(blob('sha256-ccc-partial'), 'xx');
|
||||
expect(await removePartialModelBlobs(paths)).toBe(2);
|
||||
expect(await removePartialModelBlobs(paths)).toBe(0);
|
||||
});
|
||||
|
||||
it('touches only regular files directly in the blob folder', async () => {
|
||||
const outside = await outsideDir();
|
||||
await writeFile(join(outside, 'keep-partial'), 'keep');
|
||||
await mkdir(join(paths.modelsDir, 'blobs', 'a-partial-folder'), { recursive: true });
|
||||
await writeFile(join(paths.modelsDir, 'blobs', 'a-partial-folder', 'inside-partial'), 'keep');
|
||||
// A link whose name carries the marker: the link stays, and so does what it points at.
|
||||
await symlink(join(outside, 'keep-partial'), blob('sha256-ddd-partial'));
|
||||
expect(await removePartialModelBlobs(paths)).toBe(0);
|
||||
expect(await readFile(join(outside, 'keep-partial'), 'utf8')).toBe('keep');
|
||||
expect(existsSync(join(paths.modelsDir, 'blobs', 'a-partial-folder', 'inside-partial'))).toBe(true);
|
||||
});
|
||||
|
||||
it('leaves a blob folder alone that is a symlink to somewhere else', async () => {
|
||||
const outside = await outsideDir();
|
||||
await writeFile(join(outside, 'sha256-eee-partial'), "half of somebody else's model");
|
||||
await mkdir(paths.modelsDir, { recursive: true });
|
||||
await symlink(outside, join(paths.modelsDir, 'blobs'));
|
||||
expect(await removePartialModelBlobs(paths)).toBe(0);
|
||||
expect(existsSync(join(outside, 'sha256-eee-partial'))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -25,7 +25,7 @@ import { rm } from 'node:fs/promises';
|
||||
import { createServer, type Server } from 'node:net';
|
||||
import type { ChildProcess, SpawnOptions } from 'node:child_process';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { localModelPaths, type LocalModelPaths } from '../src/local-model/paths.js';
|
||||
import { localModelPaths, runtimeBinaryForVersion, type LocalModelPaths } from '../src/local-model/paths.js';
|
||||
import {
|
||||
createRuntimeSupervisor,
|
||||
RUNTIME_MAX_RESTARTS,
|
||||
@@ -794,3 +794,112 @@ describe('stop', () => {
|
||||
expect(process.listenerCount('exit')).toBe(before);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------- A runtime an earlier server left behind, and the pid that names it ----------
|
||||
|
||||
describe('the pid is told the moment the child is spawned', () => {
|
||||
it('reports it before the child answers, so a server killed during the start leaves a pid', async () => {
|
||||
// The child never answers: the start waits, and is told to stop.
|
||||
const fake = fakeRuntime({ spawn: { neverListens: true } });
|
||||
const { supervisor, changes } = supervise(fake, {
|
||||
startTimeoutMs: 60_000,
|
||||
sleep: () => new Promise((resolve) => setTimeout(resolve, 5)),
|
||||
});
|
||||
const starting = supervisor.start({ port, contextLength: CONTEXT });
|
||||
await until(() => children(fake).length === 1, 'the child to be spawned');
|
||||
await until(() => changes.some((status) => status.pid === children(fake)[0].pid), 'the pid to be reported');
|
||||
const early = changes.find((status) => status.pid === children(fake)[0].pid);
|
||||
// Not running yet: it has not answered.
|
||||
expect(early).toMatchObject({ running: false, pid: children(fake)[0].pid });
|
||||
await supervisor.stop();
|
||||
await starting;
|
||||
// A start that failed leaves no pid behind.
|
||||
expect(supervisor.status()).toEqual({ running: false, restarts: 0, gaveUp: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe('stop with the pid the state file kept', () => {
|
||||
it('ends a runtime of ours that this process never started', async () => {
|
||||
let gone = false;
|
||||
const fake = fakeRuntime();
|
||||
const { supervisor, kills } = supervise(fake, {
|
||||
commandOfPid: async (pid) => (pid === ORPHAN_PID && !gone ? ownCommand() : undefined),
|
||||
kill: (pid, signal) => {
|
||||
kills.push([pid, signal]);
|
||||
gone = true;
|
||||
},
|
||||
});
|
||||
await supervisor.stop({ previousPid: ORPHAN_PID });
|
||||
expect(kills).toEqual([[ORPHAN_PID, 'SIGTERM']]);
|
||||
// Nothing was spawned to do it.
|
||||
expect(children(fake)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('ends one that runs another version of ours: the folder decides, not the version', async () => {
|
||||
let gone = false;
|
||||
const fake = fakeRuntime();
|
||||
const { supervisor, kills } = supervise(fake, {
|
||||
commandOfPid: async (pid) =>
|
||||
pid === ORPHAN_PID && !gone ? `${runtimeBinaryForVersion(paths, '0.39.2')} serve` : undefined,
|
||||
kill: (pid, signal) => {
|
||||
kills.push([pid, signal]);
|
||||
gone = true;
|
||||
},
|
||||
});
|
||||
await supervisor.stop({ previousPid: ORPHAN_PID });
|
||||
expect(kills).toEqual([[ORPHAN_PID, 'SIGTERM']]);
|
||||
});
|
||||
|
||||
it('never signals a pid that runs something else now, nor one that is gone', async () => {
|
||||
const fake = fakeRuntime();
|
||||
const { supervisor, kills } = supervise(fake, {
|
||||
commandOfPid: async (pid) => (pid === ORPHAN_PID ? '/usr/local/bin/ollama serve' : undefined),
|
||||
});
|
||||
await supervisor.stop({ previousPid: ORPHAN_PID });
|
||||
await supervisor.stop({ previousPid: ORPHAN_PID + 1 });
|
||||
await supervisor.stop({ previousPid: 1 });
|
||||
await supervisor.stop({ previousPid: Number.NaN });
|
||||
expect(kills).toEqual([]);
|
||||
});
|
||||
|
||||
it('stops its own child first, and the stored pid only when it is somebody else', async () => {
|
||||
const fake = fakeRuntime();
|
||||
const { supervisor, kills } = supervise(fake);
|
||||
const started = await supervisor.start({ port, contextLength: CONTEXT });
|
||||
expect(started.ok).toBe(true);
|
||||
await supervisor.stop({ previousPid: children(fake)[0].pid });
|
||||
expect(children(fake)[0].signals).toEqual(['SIGTERM']);
|
||||
// The child is gone by then: its pid names nothing of ours any more.
|
||||
expect(kills).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('start with the version an installed model was installed with', () => {
|
||||
it('runs that version from its own folder and expects it to answer as that version', async () => {
|
||||
const fake = fakeRuntime({ version: '0.39.2' });
|
||||
const { supervisor } = supervise(fake, { pin: { ...fake.pin(), version: '0.40.15' } });
|
||||
const result = await supervisor.start({ port, contextLength: CONTEXT, version: '0.39.2' });
|
||||
expect(result).toMatchObject({ ok: true });
|
||||
expect(children(fake)[0].command).toBe(runtimeBinaryForVersion(paths, '0.39.2'));
|
||||
// Asked again the same way: the child is kept. Asked for the pinned version: replaced.
|
||||
await supervisor.start({ port, contextLength: CONTEXT, version: '0.39.2' });
|
||||
expect(children(fake)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('a restart after a crash starts the same version again', async () => {
|
||||
const fake = fakeRuntime({ version: '0.39.2' });
|
||||
const { supervisor } = supervise(fake, { pin: { ...fake.pin(), version: '0.40.15' } });
|
||||
await supervisor.start({ port, contextLength: CONTEXT, version: '0.39.2' });
|
||||
children(fake)[0].crash();
|
||||
await until(() => children(fake).length === 2 && supervisor.status().running, 'the restart');
|
||||
expect(children(fake)[1].command).toBe(runtimeBinaryForVersion(paths, '0.39.2'));
|
||||
});
|
||||
|
||||
it('refuses a version that is no version before anything is spawned', async () => {
|
||||
const fake = fakeRuntime();
|
||||
const { supervisor } = supervise(fake);
|
||||
const result = await supervisor.start({ port, contextLength: CONTEXT, version: '../../bin' });
|
||||
expect(result).toMatchObject({ ok: false, code: 'runtime-start-failed' });
|
||||
expect(children(fake)).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user