fix(checkpoints): sweep tmp_pack debris stranded by timed-out store gcs
A git gc killed by _run_git's timeout strands tmp_pack_* files in the bare store's objects/pack; gc.auto=0 means git itself never reclaims them (~16 GB observed on one host). clear_stale_tmp_packs() already sweeps this debris class for the update checkout/worktree paths — teach it to resolve a bare repo's objects/pack (no .git/ layer) and call it from prune_checkpoints(), unconditionally under _store_has_head so the daily auto-prune and 'hermes checkpoints prune' both reap it even when no ref moved (a sweep is a cheap directory listing, unlike the pack-rewriting gc that stays gated on refs having moved).
This commit is contained in:
@@ -92,8 +92,12 @@ def clear_stale_git_locks(repo_root: Path, *, min_age_seconds: Optional[int] = N
|
||||
|
||||
|
||||
def clear_stale_tmp_packs(repo_root: Path, *, min_age_seconds: Optional[int] = None) -> List[str]:
|
||||
"""Remove aborted-fetch temp pack files under ``.git/objects/pack``; same contract as clear_stale_git_locks."""
|
||||
pack_dir = Path(repo_root) / ".git" / "objects" / "pack"
|
||||
"""Remove aborted-transfer temp pack files; same contract as clear_stale_git_locks.
|
||||
|
||||
Resolves ``.git/objects/pack`` for a checkout and ``objects/pack`` for a bare repo such as
|
||||
the checkpoint store — a ``git gc`` killed by a timeout strands the same debris there."""
|
||||
git_dir = Path(repo_root) / ".git"
|
||||
pack_dir = (git_dir if git_dir.is_dir() else Path(repo_root)) / "objects" / "pack"
|
||||
|
||||
def _candidates():
|
||||
try:
|
||||
|
||||
@@ -81,6 +81,21 @@ def test_skips_sweep_while_git_is_running(tmp_path, monkeypatch):
|
||||
assert p.exists()
|
||||
|
||||
|
||||
def test_bare_repo_pack_dir_is_swept(tmp_path, monkeypatch):
|
||||
"""A bare repo (e.g. the checkpoint store) has no .git/ layer — objects/pack hangs directly
|
||||
off the repo root, where a gc killed mid-repack strands the same debris (#115410)."""
|
||||
monkeypatch.setattr("hermes_cli.gitlock._git_proc_running", lambda: False)
|
||||
pack = tmp_path / "objects" / "pack"
|
||||
pack.mkdir(parents=True)
|
||||
debris = pack / "tmp_pack_killedGc"
|
||||
debris.write_bytes(b"x" * 256)
|
||||
_age(debris, STALE_TMP_PACK_MIN_AGE_SECONDS + 60)
|
||||
|
||||
removed = clear_stale_tmp_packs(tmp_path)
|
||||
assert removed == [str(debris)]
|
||||
assert not debris.exists()
|
||||
|
||||
|
||||
def test_no_git_dir_is_a_noop(tmp_path):
|
||||
assert clear_stale_tmp_packs(tmp_path) == []
|
||||
|
||||
|
||||
@@ -1122,6 +1122,37 @@ class TestGcOnlyAfterStoreMutation:
|
||||
assert len(gc_calls) == 1
|
||||
|
||||
|
||||
class TestPruneSweepsTmpPackDebris:
|
||||
"""A ``git gc`` killed by the store timeout strands ``tmp_pack_*`` files in
|
||||
``objects/pack/``; ``gc.auto=0`` means git itself never reclaims them and the gc
|
||||
only runs when a ref moved — so the prune sweeps the debris unconditionally (#115410)."""
|
||||
|
||||
def test_sweeps_debris_even_when_no_ref_moved(self, checkpoint_base, tmp_path, monkeypatch):
|
||||
import tools.checkpoint_manager as cm
|
||||
monkeypatch.setattr(cm, "CHECKPOINT_BASE", checkpoint_base)
|
||||
monkeypatch.setattr("hermes_cli.gitlock._git_proc_running", lambda: False)
|
||||
work = tmp_path / "proj"
|
||||
work.mkdir()
|
||||
(work / "f").write_text("f")
|
||||
CheckpointManager(enabled=True).ensure_checkpoint(str(work), "seed")
|
||||
|
||||
pack = checkpoint_base / "store" / "objects" / "pack"
|
||||
pack.mkdir(parents=True, exist_ok=True)
|
||||
debris = pack / "tmp_pack_killedGc"
|
||||
debris.write_bytes(b"x" * 512)
|
||||
stamp = time.time() - 11 * 60 # past the sweep's 10-minute age floor
|
||||
os.utime(debris, (stamp, stamp))
|
||||
fresh = pack / "tmp_pack_inFlight"
|
||||
fresh.write_bytes(b"y")
|
||||
|
||||
result = prune_checkpoints(retention_days=30, delete_orphans=False, checkpoint_base=checkpoint_base)
|
||||
|
||||
assert result["deleted_stale"] == 0 # no ref moved: the expensive gc never ran…
|
||||
assert not debris.exists() # …but the debris is still swept
|
||||
assert fresh.exists() # a pack possibly being written NOW is spared
|
||||
assert result["bytes_freed"] >= 512
|
||||
|
||||
|
||||
class TestMaybeAutoPruneCheckpoints:
|
||||
def test_prunes_once_then_skips_within_interval(self, tmp_path):
|
||||
base = tmp_path / "checkpoints"
|
||||
|
||||
@@ -26,6 +26,7 @@ from typing import Dict, Iterator, List, NamedTuple, Optional, Set, Tuple
|
||||
|
||||
from hermes_constants import get_hermes_home
|
||||
from hermes_cli._subprocess_compat import windows_hide_flags
|
||||
from hermes_cli.gitlock import clear_stale_tmp_packs
|
||||
from utils import env_int
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -1126,6 +1127,10 @@ def prune_checkpoints(retention_days: int = 7, delete_orphans: bool = True, chec
|
||||
_prune_pre_v2_repos(base, cutoff, delete_orphans, orphan_allowlist, result)
|
||||
store = _store_path(base)
|
||||
if _store_has_head(store):
|
||||
# A gc killed by the store timeout strands tmp_pack_* files that gc.auto=0 means git
|
||||
# itself never reclaims; sweep them even when no ref moved (a sweep is a directory
|
||||
# listing, unlike the pack-rewriting gc gated on refs below).
|
||||
clear_stale_tmp_packs(store)
|
||||
# gc rewrites the whole pack — the entire cost of a prune on a large store — so it runs
|
||||
# only when a ref moved: deleted here, or rewritten by a checkpoint that left it pending.
|
||||
deleted_before = result["deleted_orphan"] + result["deleted_stale"]
|
||||
|
||||
Reference in New Issue
Block a user