ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(file-sync): sweep stale sync-back dirs too and tighten the stale window to 6 h

Browse Source 
The salvaged commit only swept `hermes-sync-back-*.tar`. The extraction
staging dir (`tempfile.TemporaryDirectory(prefix="hermes-sync-back-")`) is
leaked by the same hard kill, so the sweep now reclaims both shapes and
returns the count. The window drops from 24 h to 6 h (the reporter's
value): archives appear every few minutes on a busy gateway and a live
transfer is never hours old. Tests trimmed to two invariants: the sweep
touches only stale prefixed entries, and a real sync_back reclaims a
leaked archive while writing its own tar under the identifiable prefix.
This commit is contained in:
teknium1
2026-09-14 17:50:01 -07:00
committed by Teknium
parent 6a03d5a94c
commit bae9f8ab85
2 changed files with 68 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/tools/test_file_sync_back.py
+36 -28
View File
@@ -5,6 +5,8 @@ import logging
import os
import signal
import tarfile
import tempfile
import time
from pathlib import Path
from unittest.mock import MagicMock, patch
@@ -14,11 +16,12 @@ fcntl = pytest.importorskip("fcntl")
from tools.environments.file_sync import (
FileSyncManager,
_cleanup_stale_sync_back_tars,
_cleanup_stale_sync_back_temp,
_sha256_file,
_SYNC_BACK_BACKOFF,
_SYNC_BACK_MAX_RETRIES,
_SYNC_BACK_STALE_TAR_SECONDS,
_SYNC_BACK_STALE_SECONDS,
_SYNC_BACK_TEMP_PREFIX,
)
@@ -91,47 +94,52 @@ def _make_manager(
return mgr
class TestStaleSyncBackTarCleanup:
"""Hard-killed sync-back archives are cleaned up conservatively."""
class TestStaleSyncBackTempCleanup:
"""Sync-back temp entries leaked by a hard kill are reclaimed by the next sync-back (#110812)."""
def test_removes_only_stale_hermes_sync_back_tars(self, tmp_path, monkeypatch):
stale = tmp_path / "hermes-sync-back-stale.tar"
def test_removes_only_stale_prefixed_entries(self, tmp_path, monkeypatch):
stale_tar = tmp_path / "hermes-sync-back-stale.tar"
stale_dir = tmp_path / "hermes-sync-back-stale-staging"
recent = tmp_path / "hermes-sync-back-recent.tar"
unrelated = tmp_path / "other-process.tar"
for path in (stale, recent, unrelated):
for path in (stale_tar, recent, unrelated):
path.write_bytes(b"tar")
stale_dir.mkdir()
(stale_dir / "root").mkdir()
now = 10_000.0
os.utime(stale, (now - _SYNC_BACK_STALE_TAR_SECONDS - 1,) * 2)
os.utime(recent, (now - _SYNC_BACK_STALE_TAR_SECONDS + 1,) * 2)
for path in (stale_tar, stale_dir):
os.utime(path, (now - _SYNC_BACK_STALE_SECONDS - 1,) * 2)
os.utime(recent, (now - _SYNC_BACK_STALE_SECONDS + 1,) * 2)
monkeypatch.setattr("tools.environments.file_sync.time.time", lambda: now)
_cleanup_stale_sync_back_tars(tmp_path)
assert _cleanup_stale_sync_back_temp(tmp_path) == 2
assert not stale.exists()
assert not stale_tar.exists()
assert not stale_dir.exists()
assert recent.exists()
assert unrelated.exists()
def test_preserves_active_tar_when_stat_or_unlink_fails(self, tmp_path, monkeypatch):
active = tmp_path / "hermes-sync-back-active.tar"
stale = tmp_path / "hermes-sync-back-stale.tar"
active.write_bytes(b"tar")
stale.write_bytes(b"tar")
now = 10_000.0
os.utime(stale, (now - _SYNC_BACK_STALE_TAR_SECONDS - 1,) * 2)
monkeypatch.setattr("tools.environments.file_sync.time.time", lambda: now)
original_unlink = Path.unlink
def test_sync_back_sweeps_leaked_entry_and_uses_identifiable_tar(self, tmp_path, monkeypatch):
tmp_root = tmp_path / "tmproot"
tmp_root.mkdir()
monkeypatch.setattr(tempfile, "tempdir", str(tmp_root))
leaked = tmp_root / "hermes-sync-back-leaked.tar"
leaked.write_bytes(b"x" * 1024)
old = time.time() - _SYNC_BACK_STALE_SECONDS - 60
os.utime(leaked, (old, old))
def fail_for_stale(path, *args, **kwargs):
if path == stale:
raise OSError("busy")
return original_unlink(path, *args, **kwargs)
seen = {}
monkeypatch.setattr(Path, "unlink", fail_for_stale)
def download(dest: Path):
seen["tar"] = dest
_make_tar({"root/.hermes/x.txt": b"hi"}, dest)
_cleanup_stale_sync_back_tars(tmp_path)
mgr = _make_manager(tmp_path, bulk_download_fn=download)
mgr.sync_back()
assert active.exists()
assert stale.exists()
assert seen["tar"].name.startswith(_SYNC_BACK_TEMP_PREFIX)
assert not leaked.exists()
assert list(tmp_root.iterdir()) == []
# ---------------------------------------------------------------------------
tools/environments/file_sync.py
+32 -23
View File
@@ -48,32 +48,41 @@ GetFilesFn = Callable[[], list[tuple[str, str]]] # () -> [(host_path, remote_pa
_SYNC_BACK_MAX_RETRIES = 3
_SYNC_BACK_BACKOFF = (2, 4, 8) # seconds between retries
_SYNC_BACK_MAX_BYTES = 2 * 1024 * 1024 * 1024 # 2 GiB — refuse to extract larger tars
_SYNC_BACK_TEMP_TAR_PREFIX = "hermes-sync-back-"
_SYNC_BACK_STALE_TAR_SECONDS = 24 * 60 * 60
_SYNC_BACK_TEMP_PREFIX = "hermes-sync-back-"
# A sync-back temp entry (the downloaded tar or the extraction staging dir) is only leaked by
# a hard kill (SIGKILL/OOM/power loss — the ``finally`` never runs), so anything older than
# this is safe to reclaim; a live transfer is hours younger than the cutoff.
_SYNC_BACK_STALE_SECONDS = 6 * 60 * 60
def _cleanup_stale_sync_back_tars(temp_dir: Path | None = None) -> None:
"""Remove old sync-back archives left behind when a process is hard-killed.
def _cleanup_stale_sync_back_temp(temp_dir: Path | None = None) -> int:
"""Remove sync-back tars and staging dirs left behind by a hard-killed process.
Only archives created with this module's dedicated prefix are considered. A
cleanup race or permission error must not prevent the current sync-back.
Only entries carrying this module's prefix and older than ``_SYNC_BACK_STALE_SECONDS``
are touched. Returns the number of entries removed; a permission error or a race with
another sync-back must not prevent the current one.
"""
directory = temp_dir or Path(tempfile.gettempdir())
cutoff = time.time() - _SYNC_BACK_STALE_TAR_SECONDS
cutoff = time.time() - _SYNC_BACK_STALE_SECONDS
removed = 0
try:
candidates = directory.glob(f"{_SYNC_BACK_TEMP_TAR_PREFIX}*.tar")
for candidate in candidates:
try:
if candidate.is_symlink() or not candidate.is_file():
continue
if candidate.stat().st_mtime >= cutoff:
continue
candidate.unlink()
logger.debug("sync_back: removed stale temporary archive %s", candidate)
except OSError:
logger.debug("sync_back: could not remove stale temporary archive %s", candidate)
candidates = list(directory.glob(f"{_SYNC_BACK_TEMP_PREFIX}*"))
except OSError:
logger.debug("sync_back: could not scan temporary directory %s", directory)
return 0
for candidate in candidates:
try:
if candidate.is_symlink() or candidate.lstat().st_mtime >= cutoff:
continue
if candidate.is_dir():
shutil.rmtree(candidate)
else:
candidate.unlink()
removed += 1
logger.debug("sync_back: removed stale temporary entry %s", candidate)
except OSError:
logger.debug("sync_back: could not remove stale temporary entry %s", candidate)
return removed
def iter_sync_files(container_base: str = "/root/.hermes") -> list[tuple[str, str]]:
@@ -337,13 +346,13 @@ class FileSyncManager:
except Exception:
file_mapping = []
# A hard kill bypasses the finally below. Reclaim only old archives using
# our dedicated prefix before allocating another full-tree download.
_cleanup_stale_sync_back_tars()
# A hard kill bypasses the finally below. Reclaim only old entries carrying our
# prefix before allocating another full-tree download.
_cleanup_stale_sync_back_temp()
# mkstemp + close: NamedTemporaryFile keeps an exclusive handle on Windows, so the
# backend's open(dest, "wb") / write_bytes on the same path raised PermissionError.
fd, tar_path = tempfile.mkstemp(prefix=_SYNC_BACK_TEMP_TAR_PREFIX, suffix=".tar")
fd, tar_path = tempfile.mkstemp(prefix=_SYNC_BACK_TEMP_PREFIX, suffix=".tar")
os.close(fd)
try:
self._bulk_download_fn(Path(tar_path))
@@ -359,7 +368,7 @@ class FileSyncManager:
tar_size, _SYNC_BACK_MAX_BYTES)
return
with tempfile.TemporaryDirectory(prefix="hermes-sync-back-") as staging:
with tempfile.TemporaryDirectory(prefix=_SYNC_BACK_TEMP_PREFIX) as staging:
with tarfile.open(tar_path) as tar:
tar.extractall(staging, filter="data")