Skip to content

Commit eed5c46

Browse files
fix(onboard): make the Ollama model pull timeout configurable (#3037)
## Summary The Ollama model pull was wrapped with a hardcoded 10-minute wall-clock timeout in both pull paths, so sustained network throughput below ~8 MB/s could not complete the default 4.7 GB qwen2.5:7b pull (each retry resumed from the last layer but still hit SIGTERM at the next 10-minute boundary). Replace the hardcoded value with a `NEMOCLAW_OLLAMA_PULL_TIMEOUT` env-var override (seconds) defaulting to 30 minutes, and surface the override hint in the timeout error messages. ## Related Issue Closes #2610 ## Changes - Add `DEFAULT_OLLAMA_PULL_TIMEOUT_MS` (30 minutes), `getOllamaPullTimeoutMs()` (parses `NEMOCLAW_OLLAMA_PULL_TIMEOUT` in seconds with fallback for unset / empty / non-numeric / non-positive inputs), and `pullTimeoutErrorHint()` (prints elapsed minutes, the resume-on-retry note, and the env var hint) in `src/lib/onboard-ollama-proxy.ts`. - Wire the helper into `pullOllamaModelViaCli` (replacing the hardcoded `timeout: 600_000` on `spawnSync`) and `pullOllamaModelViaHttp` (replacing the hardcoded `TIMEOUT_MS = 600_000` on the curl `--max-time` deadline). The HTTP-path non-timeout error path retains its existing wording. - Export `getOllamaPullTimeoutMs` so the unit tests can exercise the parser without shelling out to `ollama pull`. - Add `test/ollama-pull-timeout.test.ts` covering: default fallback when unset, empty, or whitespace; positive integer seconds → ms conversion; truncated fractional seconds; non-numeric fallback; zero/negative fallback. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable Ollama model-pull timeout via environment variable (30-minute default) with improved timeout error messaging. * **Tests** * Added test suite validating timeout configuration, conversion, and error handling behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
1 parent 6c07b82 commit eed5c46

2 files changed

Lines changed: 144 additions & 8 deletions

File tree

src/lib/onboard-ollama-proxy.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -306,18 +306,37 @@ function printOllamaExposureWarning() {
306306
console.log("");
307307
}
308308

309+
const DEFAULT_OLLAMA_PULL_TIMEOUT_MS = 30 * 60 * 1000;
310+
const PULL_TIMEOUT_ENV = "NEMOCLAW_OLLAMA_PULL_TIMEOUT";
311+
312+
function getOllamaPullTimeoutMs(): number {
313+
const raw = process.env[PULL_TIMEOUT_ENV];
314+
if (typeof raw !== "string" || raw.trim() === "") return DEFAULT_OLLAMA_PULL_TIMEOUT_MS;
315+
const seconds = Number(raw.trim());
316+
if (!Number.isFinite(seconds) || seconds <= 0) return DEFAULT_OLLAMA_PULL_TIMEOUT_MS;
317+
return Math.floor(seconds * 1000);
318+
}
319+
320+
function pullTimeoutErrorHint(timeoutMs: number): string {
321+
const minutes = Math.round(timeoutMs / 60_000);
322+
return [
323+
` Model pull timed out after ${minutes} minutes.`,
324+
" Already-downloaded layers are kept; re-running the pull resumes them.",
325+
` Set ${PULL_TIMEOUT_ENV}=<seconds> to raise the wall-clock limit (default ${Math.round(DEFAULT_OLLAMA_PULL_TIMEOUT_MS / 60_000)} minutes).`,
326+
].join("\n");
327+
}
328+
309329
function pullOllamaModelViaCli(model) {
330+
const timeoutMs = getOllamaPullTimeoutMs();
310331
const result = spawnSync("bash", ["-c", `ollama pull ${shellQuote(model)}`], {
311332
cwd: ROOT,
312333
encoding: "utf8",
313334
stdio: "inherit",
314-
timeout: 600_000,
335+
timeout: timeoutMs,
315336
env: buildSubprocessEnv(),
316337
});
317338
if (result.signal === "SIGTERM") {
318-
console.error(
319-
` Model pull timed out after 10 minutes. Try a smaller model or check your network connection.`,
320-
);
339+
console.error(pullTimeoutErrorHint(timeoutMs));
321340
return false;
322341
}
323342
return result.status === 0;
@@ -332,7 +351,7 @@ function pullOllamaModelViaHttp(model) {
332351
const host = getResolvedOllamaHost();
333352
const url = `http://${host}:${OLLAMA_PORT}/api/pull`;
334353
const body = JSON.stringify({ model, stream: true });
335-
const TIMEOUT_MS = 600_000; // 10 min, matches the CLI path
354+
const TIMEOUT_MS = getOllamaPullTimeoutMs();
336355
const isTTY = Boolean(process.stdout.isTTY);
337356
const BAR_WIDTH = 40;
338357

@@ -343,7 +362,7 @@ function pullOllamaModelViaHttp(model) {
343362
"--connect-timeout",
344363
"10",
345364
"--max-time",
346-
String(Math.floor(TIMEOUT_MS / 1000)),
365+
String(TIMEOUT_MS / 1000),
347366
"-X",
348367
"POST",
349368
"-H",
@@ -450,11 +469,11 @@ function pullOllamaModelViaHttp(model) {
450469
if (code !== 0) {
451470
// curl exit 28 = CURLE_OPERATION_TIMEDOUT (--max-time hit).
452471
if (code === 28) {
453-
console.error(` Model pull timed out after ${TIMEOUT_MS / 60_000} minutes.`);
472+
console.error(pullTimeoutErrorHint(TIMEOUT_MS));
454473
} else {
455474
console.error(` Model pull exited with code ${String(code)} (network error).`);
475+
console.error(" Already-downloaded layers are kept; re-running the pull resumes them.");
456476
}
457-
console.error(" Already-downloaded layers are kept; re-running the pull resumes them.");
458477
resolve(false);
459478
return;
460479
}
@@ -551,6 +570,7 @@ function unloadOllamaModels() {
551570
export {
552571
ensureOllamaAuthProxy,
553572
getOllamaProxyToken,
573+
getOllamaPullTimeoutMs,
554574
isProxyHealthy,
555575
killStaleProxy,
556576
persistProxyToken,

test/ollama-pull-timeout.test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import fs from "fs";
5+
import os from "os";
6+
import path from "path";
7+
import { spawnSync } from "child_process";
8+
9+
import { afterEach, describe, expect, it } from "vitest";
10+
11+
import { getOllamaPullTimeoutMs } from "../dist/lib/onboard-ollama-proxy.js";
12+
13+
const ENV = "NEMOCLAW_OLLAMA_PULL_TIMEOUT";
14+
const DEFAULT_MS = 30 * 60 * 1000;
15+
16+
describe("getOllamaPullTimeoutMs", () => {
17+
const original = process.env[ENV];
18+
afterEach(() => {
19+
if (original === undefined) delete process.env[ENV];
20+
else process.env[ENV] = original;
21+
});
22+
23+
it("falls back to the 30-minute default when the env var is unset", () => {
24+
delete process.env[ENV];
25+
expect(getOllamaPullTimeoutMs()).toBe(DEFAULT_MS);
26+
});
27+
28+
it("falls back to the default when the env var is empty or whitespace", () => {
29+
process.env[ENV] = "";
30+
expect(getOllamaPullTimeoutMs()).toBe(DEFAULT_MS);
31+
process.env[ENV] = " ";
32+
expect(getOllamaPullTimeoutMs()).toBe(DEFAULT_MS);
33+
});
34+
35+
it("converts a positive integer seconds value to milliseconds", () => {
36+
process.env[ENV] = "1800";
37+
expect(getOllamaPullTimeoutMs()).toBe(1_800_000);
38+
});
39+
40+
it("converts fractional second inputs to milliseconds", () => {
41+
process.env[ENV] = "1.5";
42+
expect(getOllamaPullTimeoutMs()).toBe(1_500);
43+
});
44+
45+
it("preserves sub-second precision when passing the HTTP pull timeout to curl", () => {
46+
const repoRoot = path.join(import.meta.dirname, "..");
47+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-ollama-pull-timeout-"));
48+
const scriptPath = path.join(tmpDir, "http-timeout-check.js");
49+
const proxyPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "onboard-ollama-proxy.js"));
50+
const localInferencePath = JSON.stringify(path.join(repoRoot, "dist", "lib", "local-inference.js"));
51+
const script = `
52+
const { EventEmitter } = require("events");
53+
const { PassThrough } = require("stream");
54+
const childProcess = require("child_process");
55+
const localInference = require(${localInferencePath});
56+
57+
let captured = null;
58+
childProcess.spawn = (cmd, args) => {
59+
captured = { cmd, args };
60+
const child = new EventEmitter();
61+
child.stdout = new PassThrough();
62+
child.stderr = new PassThrough();
63+
process.nextTick(() => {
64+
child.stdout.end('{"status":"success"}\\n', () => {
65+
setImmediate(() => child.emit("close", 0));
66+
});
67+
});
68+
return child;
69+
};
70+
71+
localInference.setResolvedOllamaHost(localInference.OLLAMA_HOST_DOCKER_INTERNAL);
72+
process.env.${ENV} = "0.5";
73+
74+
const { pullOllamaModel } = require(${proxyPath});
75+
76+
const originalLog = console.log;
77+
console.log = () => {};
78+
pullOllamaModel("qwen2.5:7b")
79+
.then((ok) => {
80+
console.log = originalLog;
81+
originalLog(JSON.stringify({ ok, captured }));
82+
})
83+
.catch((error) => {
84+
console.log = originalLog;
85+
console.error(error);
86+
process.exit(1);
87+
});
88+
`;
89+
fs.writeFileSync(scriptPath, script);
90+
91+
const result = spawnSync(process.execPath, [scriptPath], {
92+
cwd: repoRoot,
93+
encoding: "utf-8",
94+
});
95+
96+
expect(result.status, result.stderr).toBe(0);
97+
const payload = JSON.parse(result.stdout.trim());
98+
expect(payload.ok).toBe(true);
99+
expect(payload.captured.cmd).toBe("curl");
100+
const maxTimeIndex = payload.captured.args.indexOf("--max-time");
101+
expect(maxTimeIndex).toBeGreaterThanOrEqual(0);
102+
expect(payload.captured.args[maxTimeIndex + 1]).toBe("0.5");
103+
});
104+
105+
it("falls back to the default for non-numeric values", () => {
106+
process.env[ENV] = "thirty-minutes";
107+
expect(getOllamaPullTimeoutMs()).toBe(DEFAULT_MS);
108+
});
109+
110+
it("falls back to the default for zero or negative values", () => {
111+
process.env[ENV] = "0";
112+
expect(getOllamaPullTimeoutMs()).toBe(DEFAULT_MS);
113+
process.env[ENV] = "-60";
114+
expect(getOllamaPullTimeoutMs()).toBe(DEFAULT_MS);
115+
});
116+
});

0 commit comments

Comments
 (0)