Skip to content

fix(runner): bound container wait so one wedged run can't stall a batch - #316

Open
vaibhavdabas16 wants to merge 1 commit into
TIGER-AI-Lab:mainfrom
vaibhavdabas16:fix/host-side-container-timeout
Open

fix(runner): bound container wait so one wedged run can't stall a batch#316
vaibhavdabas16 wants to merge 1 commit into
TIGER-AI-Lab:mainfrom
vaibhavdabas16:fix/host-side-container-timeout

Conversation

@vaibhavdabas16

Copy link
Copy Markdown

What does this PR do?

Fixes #298 — both asks.

The only time limit lived inside the container (entrypoint.sh:251, MAX_WAIT=${TIME_LIMIT_S:-1800}). On the host side there was no deadline anywhere:

  • docker_wait() (docker.py:522) blocked on <engine> wait with while proc.poll() is None: and no bound
  • batch.py:313 awaited proc.communicate() with no per-job bound

If the in-container watchdog never fired — entrypoint crash, wedged Chromium, engine hiccup, zombie container — the host waited forever. clawbench-run blocked; in batch mode the job held a concurrency slot indefinitely. With batches routinely running 8–20 hours, one wedged container could stall a 130-task run overnight with no error and no summary.

Ask 1 — deadline in docker_wait

It now takes timeout_s and returns whether the deadline expired. On expiry it kills the container and returns True. run.py passes time_limit_s + HOST_TIMEOUT_GRACE_S (5 min), records host_timeout: ... as the failure reason, and classifies the run as infra_failure so it stays out of adjusted scoring — then carries on to copy results and write run-meta.json as usual. Partial data from a killed container is still worth keeping, and this preserves the same invariant #303 and #302 are about: a run that got far enough to produce anything always records it. --human runs stay unbounded by design.

Ask 2 — per-job bound in batch.py

proc.communicate() is wrapped in asyncio.wait_for, sized from the case's own time_limit plus BATCH_JOB_GRACE_S (15 min) so it sits above the run's own deadline — clawbench-run reports its own timeout first, and this only fires when the child process itself is wedged. On expiry it kills the process group, marks the job error, notes host_timeout in the job log, and the batch proceeds. --job-timeout overrides the derived value; 0 disables it.

job_timeout_s() reads time_limit from both corpus layouts — <case>/task.json and claw-eval's flat <suite>/<id>.json — and falls back to 1800s when the task file is unreadable.

On the new utils/timeouts.py

Three constants shared by run.py, docker.py and batch.py. batch.py cannot import them from run_support.docker, because that pulls in run_support.config, which resolves a container engine at import time and sys.exits when neither Docker nor Podman is installed — and batch.py must stay importable without one (tests/test_batch_and_tui_helpers.py depends on it).

A shared module seemed better than duplicating two integers across modules that must agree. That import-time probe is the underlying problem, and I've filed it separately as #315 with a suggested lazy-engine() fix; if that lands, this module could fold back into docker.py. Happy to take a different shape here if you'd prefer.

Corpus

  • v2
  • v1
  • both
  • not applicable

Host-side runner/batch change; no task data involved.

Test plan

  • New tests/test_host_timeout.py, 12 tests: job_timeout_s against both corpus layouts, four unreadable-task fallbacks, override and disable; docker_wait killing a never-exiting container and returning promptly; timeout_s=None preserving the old unbounded behaviour; BATCH_JOB_GRACE_S > HOST_TIMEOUT_GRACE_S so the layering cannot silently invert; and batch.py still importing with no container engine present.
  • Demonstrated the hang directly rather than only through tests — driving the unpatched docker_wait against a stub container that never exits, on a background thread:
    UNPATCHED docker_wait still running after 6s? True <- hangs forever PATCHED returned True after 2.0s; issued: ['kill', 'wedged']
  • Full suite: 204 passed, 3 skipped. The one failure, test_host_tasks.py::test_checked_task_json_files_parse_and_validate[v1-lite], reproduces identically on a clean main on this machine — those task files are git symlinks (mode 120000) that Windows checks out as text. Unrelated.
  • ruff check and ruff format --check clean.

Not verified: no live containerized run — I don't have Docker on this machine. The wedged-container path is exercised by injection, not by a genuinely stuck Chromium.

Related issues

Fixes #298. Root cause of the utils/timeouts.py workaround filed as #315.

One deliberate choice worth a maintainer's eye: a host timeout is classified as the existing infra_failure category with host_timeout: ... in failure_reason, matching the convention at run.py:415. The issue's wording ("infra_failure / host_timeout") could instead mean a distinct host_timeout category — that would need adding it to NON_MODEL_FAILURE_CATEGORIES and widening the infra_failure predicate in classify_run, which changes semantics shared with every other failure path. I took the conservative route; say the word and I'll switch it.

The only time limit lived inside the container (entrypoint.sh's
MAX_WAIT=${TIME_LIMIT_S:-1800}). docker_wait() blocked on `<engine> wait`
with `while proc.poll() is None:` and no deadline, and batch.py awaited
proc.communicate() with no per-job bound. If the in-container watchdog
never fired — entrypoint crash, wedged Chromium, engine hiccup, zombie
container — the host waited forever: clawbench-run blocked, and in batch
mode the job held a concurrency slot indefinitely. With batches routinely
running 8-20 hours, a single wedged container could stall 130 tasks
overnight with no error and no summary.

Two host-side deadlines, layered:

- docker_wait() takes timeout_s and returns whether it expired. On expiry
  it kills the container and returns True. run.py passes
  time_limit_s + HOST_TIMEOUT_GRACE_S (5 min), records
  "host_timeout: ..." as the failure reason, and classifies the run as
  infra_failure so it stays out of adjusted scoring — then carries on to
  copy results and write run-meta.json as usual. Partial data from a
  killed container is still worth keeping. --human runs stay unbounded.

- batch.py wraps proc.communicate() in asyncio.wait_for, sized from the
  case's own time_limit plus BATCH_JOB_GRACE_S (15 min) so it sits above
  the run's own deadline and only fires when the child itself is wedged.
  On expiry it kills the process group, marks the job "error", notes
  host_timeout in the job log, and the batch proceeds. --job-timeout
  overrides the derived value; 0 disables the bound.

job_timeout_s() reads time_limit from either corpus layout — <case>/task.json
and claw-eval's flat <suite>/<id>.json — and falls back to 1800s when the
task file is unreadable.

The three constants live in a new clawbench.utils.timeouts so run.py,
docker.py and batch.py share one definition. batch.py cannot import them
from run_support.docker: that module pulls in run_support.config, which
resolves a container engine at import time and exits when neither Docker
nor Podman is present, and batch.py must stay importable without one. A
test pins that property, and another pins BATCH_JOB_GRACE_S >
HOST_TIMEOUT_GRACE_S so the layering cannot silently invert.

Fixes TIGER-AI-Lab#298.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No host-side timeout on container wait — one wedged container stalls an entire batch indefinitely

2 participants