Repository navigation
feat(harvest): advisory cross-machine lock on the shared store - #9
Conversation
Two machines harvesting into the same Syncthing-synced store at once produce sync-conflict copies of harvested.json / usage.jsonl. Add a best-effort advisory lock: a run drops .harvest.lock (host/pid/ts) before it starts and removes it when done; a second run that sees a live lock defers instead of racing. Locks older than harvest_lock_stale_seconds (default 1h) are treated as crashed leftovers and stolen, so a dead machine can't wedge the store. Wired into both harvest paths (CLI _harvest_all and the daemon nightly job). The nightly schedule stagger already prevents scheduled overlaps; this closes the manual-run case that the stagger can't. Advisory, not a hard mutex: the lock file itself syncs with latency, so two runs started within one sync cycle can still both acquire. It eliminates every collision beyond that window. Tests: 12 unit (acquire/release/steal/stale/context-manager) + 1 CLI integration (defer-without-write when store is locked). Full suite 442 passed, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015V5ds97jNLw4cH2NJcjLze
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 800917d042
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if existing is not None and age is not None and age <= stale_seconds: | ||
| raise LockHeld(existing.get("host", "?"), existing.get("pid", -1), age) |
There was a problem hiding this comment.
Avoid stealing unreadable locks that may be live
If a second harvester observes .harvest.lock after another process has created it but before the JSON write is visible (or while Syncthing has delivered a partial/empty lock file), _read_lock() returns None, so this condition falls through to the stale/unreadable branch and unlinks the file. In that concurrent-start scenario both harvesters can believe they acquired the lock, defeating the guard this change adds; unreadable locks should be retried or treated as held unless they can be proven stale.
Useful? React with 👍 / 👎.
feat(harvest): advisory cross-machine lock on the shared store
Problem
Two machines (Mac / Pi / PC) harvesting into the same Syncthing-synced store at once produce sync-conflict copies of
harvested.json/usage.jsonl. The nightly schedule stagger prevents scheduled overlaps, but not manualharvest --allruns started close together.Change
Best-effort advisory lock on the shared store:
.harvest.lock(host / pid / timestamp) before it starts and removes it when done.harvest_lock_stale_seconds(default 1h, well past a ~30-min run) is treated as a crashed leftover and stolen, so a dead machine can't wedge the store.Wired into both harvest paths: CLI
_harvest_all(now wraps_harvest_all_locked) and the daemon nightly job. New configharvest_lock_stale_seconds.Honest limitation
Advisory, not a hard mutex: the lock file itself syncs with latency, so two runs started within one sync cycle can still both acquire. It eliminates every collision beyond that window; combined with the 60-min schedule stagger, the residual is near-zero.
Tests
test_lock.py): acquire / release / steal-stale / respect-live / unreadable / context-manager release-on-exception.test_cli.py):--alldefers without writing when the store is locked.🤖 Generated with Claude Code
https://claude.ai/code/session_015V5ds97jNLw4cH2NJcjLze