fix: hermes -w repack no longer stampedes a shared clone
Every `hermes -w` launch on a clone past the pack-sprawl threshold started its own `git repack -a -d` of the whole object store in a daemon thread. On a multi-agent box that meant dozens of concurrent multi-GB repacks of the same repo, each too starved (nice 19, under the others) to finish inside the 1800 s timeout. subprocess.run's timeout killed only `git repack`, so the `pack-objects` grandchild kept running with ppid 1 for days; the CLI exiting orphaned it the same way. Observed: 51 pack-objects processes, load 200 on 20 cores, 143 GB swap, 29 GB of `.tmp-*-pack` debris, and every `hermes` invocation taking 5-7 s of wall clock for 0.6 s of CPU. - `_claim_repack_slot`: one repack per clone per 6 h across processes (`.git/hermes-repack.lock`, mtime = stamp; stale takeover via `replace` so only one of N racers wins). - `_run_bounded_repack`: own process group + `kill_process_tree` on timeout and at exit, so the whole tree dies with the launcher. - incremental `git repack -d --geometric=2 --write-midx` instead of a full `-a` rewrite: consolidates sprawl in seconds instead of rewriting 10 GB per run. - `.tmp-<pid>-pack*` (pack-objects debris) joins gitlock's stale tmp-pack sweep and is swept before repacking.
This commit is contained in:
@@ -23,7 +23,8 @@ STALE_TMP_PACK_MIN_AGE_SECONDS = STALE_LOCK_MIN_AGE_SECONDS
|
||||
LOCK_NAMES = ("shallow.lock", "index.lock", "HEAD.lock", "MERGE_HEAD.lock")
|
||||
# Temp-file prefixes git writes into .git/objects/pack during a transfer and renames away on
|
||||
# success; anything left with these names after a fetch died is garbage by definition.
|
||||
_TMP_PACK_PREFIXES = ("tmp_pack_", "tmp_idx_", "tmp_rev_", "tmp_mtimes_")
|
||||
# ``.tmp-<pid>-pack*`` is the same thing from ``pack-objects`` (repack/gc) killed mid-write.
|
||||
_TMP_PACK_PREFIXES = ("tmp_pack_", "tmp_idx_", "tmp_rev_", "tmp_mtimes_", ".tmp-")
|
||||
|
||||
|
||||
def _git_proc_running() -> bool:
|
||||
|
||||
@@ -4,6 +4,7 @@ Every git call goes through ``_git``/``_git_out``/``_git_quiet`` (UTF-8 text, ca
|
||||
bounded timeout). Classification helpers fail SAFE toward "preserve". ``cli`` re-exports
|
||||
these names; ``_cprint`` is imported lazily from ``cli`` to avoid a cycle.
|
||||
"""
|
||||
import atexit
|
||||
import concurrent.futures
|
||||
import json
|
||||
import logging
|
||||
@@ -18,6 +19,7 @@ import uuid
|
||||
from pathlib import Path
|
||||
from typing import Dict, Optional
|
||||
|
||||
from hermes_cli._subprocess_compat import kill_process_tree
|
||||
from hermes_constants import get_hermes_home
|
||||
from utils import atomic_json_write
|
||||
|
||||
@@ -98,6 +100,70 @@ def _cleanup_failed_worktree_add(repo_root: str, wt_path: Path, branch_name: str
|
||||
|
||||
|
||||
_PACK_SPRAWL_THRESHOLD = 15
|
||||
_REPACK_TIMEOUT = 1800
|
||||
# One repack attempt per clone per interval, box-wide. Every ``hermes -w`` launch on a shared clone
|
||||
# used to start its own ``git repack -a`` of the whole store; on a multi-agent box that stacked 50+
|
||||
# concurrent multi-GB repacks (each too slow under the others to ever finish inside the timeout).
|
||||
_REPACK_MIN_INTERVAL = 6 * 3600
|
||||
_REPACK_LOCK = "hermes-repack.lock"
|
||||
|
||||
|
||||
def _claim_repack_slot(git_dir: Path) -> bool:
|
||||
"""Exactly one process per clone gets to repack per ``_REPACK_MIN_INTERVAL``.
|
||||
|
||||
The lock file's mtime is the stamp: younger than the interval means another launch is
|
||||
repacking (or just tried and timed out) — skip. A stale lock is taken over by ``replace``,
|
||||
which only one of N racing processes can win; the O_EXCL create then serializes against a
|
||||
process that found no lock at all.
|
||||
"""
|
||||
lock = git_dir / _REPACK_LOCK
|
||||
try:
|
||||
st = lock.stat()
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
else:
|
||||
if time.time() - st.st_mtime < _REPACK_MIN_INTERVAL:
|
||||
return False
|
||||
try:
|
||||
lock.replace(lock.with_suffix(".stale"))
|
||||
except OSError:
|
||||
return False
|
||||
try:
|
||||
fd = os.open(lock, os.O_CREAT | os.O_EXCL | os.O_WRONLY)
|
||||
except FileExistsError:
|
||||
return False
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as fh:
|
||||
fh.write(f"{os.getpid()}\n")
|
||||
lock.with_suffix(".stale").unlink(missing_ok=True)
|
||||
return True
|
||||
|
||||
|
||||
def _run_bounded_repack(repo_root: str) -> None:
|
||||
"""Incremental geometric repack whose whole process tree dies with the timeout or with us.
|
||||
|
||||
``repack`` forks ``pack-objects``; ``subprocess.run(timeout=)`` killed only the parent and
|
||||
left the grandchild packing for days, and a daemon thread's child outlived the CLI the same
|
||||
way. A new session/process group + ``atexit`` reaps both cases.
|
||||
"""
|
||||
cmd = ["git", "repack", "-d", "--geometric=2", "--write-midx", "--quiet"]
|
||||
if os.name == "posix":
|
||||
cmd = ["nice", "-n", "19", *cmd]
|
||||
group_kw: dict = {"process_group": 0}
|
||||
else:
|
||||
group_kw = {"creationflags": getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0)}
|
||||
proc = subprocess.Popen(cmd, cwd=repo_root, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL, **group_kw)
|
||||
|
||||
def _reap() -> None:
|
||||
if proc.poll() is None:
|
||||
kill_process_tree(proc)
|
||||
|
||||
atexit.register(_reap)
|
||||
try:
|
||||
proc.wait(timeout=_REPACK_TIMEOUT)
|
||||
except subprocess.TimeoutExpired:
|
||||
_reap()
|
||||
logger.info("git repack exceeded %ds; killed (next attempt in %dh)", _REPACK_TIMEOUT, _REPACK_MIN_INTERVAL // 3600)
|
||||
|
||||
|
||||
def _maintain_pack_health(repo_root: str) -> None:
|
||||
@@ -113,12 +179,12 @@ def _maintain_pack_health(repo_root: str) -> None:
|
||||
packs = len(list(pack_dir.glob("*.pack")))
|
||||
if packs < _PACK_SPRAWL_THRESHOLD:
|
||||
return
|
||||
if not _claim_repack_slot(pack_dir.parent.parent):
|
||||
return
|
||||
from hermes_cli.gitlock import clear_stale_tmp_packs
|
||||
clear_stale_tmp_packs(Path(repo_root))
|
||||
logger.info("git pack sprawl (%d packs) — repacking in background", packs)
|
||||
cmd = ["git", "repack", "-a", "-d", "--quiet"]
|
||||
if os.name == "posix":
|
||||
cmd = ["nice", "-n", "19", *cmd]
|
||||
subprocess.run(cmd, capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=1800,
|
||||
cwd=repo_root, check=False)
|
||||
_run_bounded_repack(repo_root)
|
||||
# Repacking can strand now-duplicated admin files; prune on the same pass.
|
||||
_git(["worktree", "prune"], repo_root, timeout=60, check=False)
|
||||
except Exception as e:
|
||||
|
||||
@@ -144,3 +144,52 @@ class TestMaintainPackHealth:
|
||||
from cli import _maintain_pack_health
|
||||
|
||||
_maintain_pack_health(str(tmp_path / "not-a-repo")) # must not raise
|
||||
|
||||
|
||||
class TestRepackStampede:
|
||||
"""Regression for the Sep 2026 shared-clone incident: every ``hermes -w`` launch started its
|
||||
own full repack, and a timed-out repack left ``pack-objects`` running for days."""
|
||||
|
||||
def test_one_repack_per_clone_per_interval(self, repo, monkeypatch):
|
||||
from hermes_cli import worktree_ops
|
||||
|
||||
monkeypatch.setattr(worktree_ops, "_PACK_SPRAWL_THRESHOLD", 0)
|
||||
runs: list = []
|
||||
monkeypatch.setattr(worktree_ops, "_run_bounded_repack", lambda root: runs.append(root))
|
||||
|
||||
for _ in range(3): # three concurrent-ish launches sharing the clone
|
||||
worktree_ops._maintain_pack_health(str(repo))
|
||||
assert runs == [str(repo)], "N launches inside the interval must produce exactly one repack"
|
||||
|
||||
# A stale stamp (older than the interval) hands the slot to the next launch.
|
||||
monkeypatch.setattr(worktree_ops, "_REPACK_MIN_INTERVAL", 0)
|
||||
worktree_ops._maintain_pack_health(str(repo))
|
||||
assert len(runs) == 2
|
||||
|
||||
@pytest.mark.linux_only
|
||||
def test_timeout_kills_the_whole_repack_tree(self, tmp_path, monkeypatch):
|
||||
import os
|
||||
import time
|
||||
|
||||
from hermes_cli import worktree_ops
|
||||
|
||||
# A stand-in ``git`` that forks a long-lived grandchild, the way repack forks pack-objects.
|
||||
shim_dir = tmp_path / "bin"
|
||||
shim_dir.mkdir()
|
||||
pidfile = tmp_path / "grandchild.pid"
|
||||
shim = shim_dir / "git"
|
||||
shim.write_text(f"#!/bin/sh\nsleep 300 &\necho $! > {pidfile}\nwait\n")
|
||||
shim.chmod(0o755)
|
||||
monkeypatch.setenv("PATH", f"{shim_dir}{os.pathsep}{os.environ['PATH']}")
|
||||
monkeypatch.setattr(worktree_ops, "_REPACK_TIMEOUT", 1)
|
||||
|
||||
worktree_ops._run_bounded_repack(str(tmp_path))
|
||||
|
||||
grandchild = int(pidfile.read_text().strip())
|
||||
deadline = time.time() + 5
|
||||
while time.time() < deadline:
|
||||
if not Path(f"/proc/{grandchild}").exists():
|
||||
return
|
||||
time.sleep(0.05)
|
||||
subprocess.run(["kill", "-9", str(grandchild)], check=False)
|
||||
pytest.fail("pack-objects stand-in survived the repack timeout")
|
||||
|
||||
Reference in New Issue
Block a user