prepare: serialise concurrent model preparation with flock

[TyroneNel]

Split out of #124 at @mhenrichsen's request, so that PR stays a chat-template fix and this can be reviewed on its own merits.

Why

docker/prepare.sh is idempotent per step but not concurrency-safe, and concurrency is the normal case rather than user error: the entrypoint runs prepare before every start, so a booting container races docker compose run --rm prepare, and two containers starting together after a crash race each other. Two runs against one model dir can interleave a shard rewrite with an index write, and the damage surfaces far from its cause — a shard missing from model.safetensors.index.json, a config that disagrees with the tensors on disk, or a half-fetched fast variant that verify.sh then reports somewhere else entirely.

What

docker/prepare.sh takes an exclusive flock on <models dir>/.prepare.lock:

• the directory comes from dirname "$BASE", so the lock sits beside the model dir and BASE_MODEL_DIR cannot move it inside it. The hunk originally attached to Translate chat-template effort vocabulary so OpenAI clients do not get HTTP 400 #124 used ${BASE_MODEL_DIR:-/app/models}, which put the lock inside the model directory whenever that variable was set;
• PREPARE_LOCK_WAIT (default 600 s) bounds the wait, and only a timeout refuses, so a legitimate concurrent prepare is waited for rather than rejected on sight;
• the lock is advisory and is released when the holder exits, so a leftover .prepare.lock file is inert. Nothing has to clean it up, and a stale file cannot block a prepare.

One correction to the underlying premise

The original hunk was rejected partly because "a stale lock now refuses to prepare". flock is released when the holding process exits, so that failure needs a live holder to occur at all; the real cost was refusing a legitimate concurrent prepare, which the bounded wait removes. Said plainly because the fix here targets the real failure mode rather than the stated one.

Docs

Gotcha 59 (main's last entry is 57, #124 claims 58, so this takes 59 — it needs renumbering if #124 has not merged first) and a bullet in docs/docker.md where the entrypoint's prepare step is described.

Verification

flock is present in the built image (util-linux 2.39.3, checked inside ghcr.io/syv-ai/qwen38-27b-rtx3090:latest) and in the shell used to test. The lock lines were extracted verbatim from docker/prepare.sh and exercised:

Check	Result
lock path with BASE_MODEL_DIR set	<models>/.prepare.lock; nothing written inside the model dir
second prepare while a lock is held, PREPARE_LOCK_WAIT=1	waited 1 s, then exit 1 with the refusal message
holder releases inside the window (PREPARE_LOCK_WAIT=15)	second proceeded, exit 0
leftover .prepare.lock, no holder	acquired immediately — stale file inert
wait default with the variable unset	600 s
bash -n docker/prepare.sh	clean

Limits

No full prepare run (re-downloads ~19.5 GB) and no image rebuild. The exercised lines are the script's own lock lines, run under the same shell and flock version the image ships.

[mhenrichsen]

Merging — and you are right about the premise, which I got wrong.

I said a stale lock would refuse to prepare. flock is advisory and released when the holding process exits, so a leftover .prepare.lock file cannot block anything; that failure needs a live holder. Reproduced on the box: a lock file left behind with no holder is acquired immediately. Thanks for saying it plainly rather than working around my objection.

The rest, exercised with the lock lines as they appear in docker/prepare.sh:

check	result
lock path with BASE_MODEL_DIR set	<models>/.prepare.lock — beside the dir, 0 files written inside it
second prepare, holder live, PREPARE_LOCK_WAIT=1	waited, then refused, exit 1
holder releases inside the window (=15)	acquired, exit 0
leftover lock file, no holder	acquired immediately

MODELS_ROOT=$(dirname "$BASE") is the right derivation — it follows BASE wherever BASE_MODEL_DIR puts it instead of assuming /app/models, which is what the original hunk got wrong.

The bounded wait is what makes this a different change from the one I pushed back on. Refusing on sight turns the normal case — the entrypoint's prepare racing compose run --rm prepare — into a failure; waiting 600 s turns it into a pause nobody notices. Same mechanism, opposite default, and the default was the objection.

Gotcha 59 is correct now: #124 merged a minute ago with 58.

[mhenrichsen]

Merging — and you are right about the premise, which I got wrong.

I said a stale lock would refuse to prepare. flock is advisory and released when the holding process exits, so a leftover .prepare.lock file cannot block anything; that failure needs a live holder. Reproduced on the box: a lock file left behind with no holder is acquired immediately. Thanks for saying it plainly rather than working around my objection.

The rest, exercised with the lock lines as they appear in docker/prepare.sh:

check	result
lock path with BASE_MODEL_DIR set	<models>/.prepare.lock — beside the dir, 0 files written inside it
second prepare, holder live, PREPARE_LOCK_WAIT=1	waited, then refused, exit 1
holder releases inside the window (=15)	acquired, exit 0
leftover lock file, no holder	acquired immediately

MODELS_ROOT=$(dirname "$BASE") is the right derivation — it follows BASE wherever BASE_MODEL_DIR puts it instead of assuming /app/models, which is what the original hunk got wrong.

The bounded wait is what makes this a different change from the one I pushed back on. Refusing on sight turns the normal case — the entrypoint's prepare racing compose run --rm prepare — into a failure; waiting 600 s turns it into a pause nobody notices. Same mechanism, opposite default, and the default was the objection.

Gotcha 59 is correct now: #124 merged a minute ago with 58.

---

Rebased your branch onto main myself rather than bouncing it back — #124 merged a few minutes before this and touched the same two files, so the conflict was my sequencing, not yours. docker/prepare.sh and docs/docker.md merged clean; the only conflict was both PRs appending to docs/gotchas.md, resolved by keeping 58 then 59, which is the numbering you predicted. Your commit and authorship are unchanged (828c957).

Checked after rebasing: the lock is taken at line 24, the step loop starts at line 59, so it still precedes every mutating step — including #124's translate_chat_template.py call, which now runs inside the lock. bash -n clean.