debian: clean up stale ftpsync lock files before sync

[yaoge123]

If a previous ftpsync run was killed (OOM, container restart, ...), the Archive-Update-in-Progress-* marker stays in the mirror tree and is served to downstream mirrors, falsely signalling an ongoing sync.

ftpsync normally takes over an existing lock when kill -0 on the recorded PID fails — but our workers run in Docker containers, and a container restart resets the PID namespace. The stale lock's PID can then collide with a live in-container process, kill -0 succeeds, and ftpsync keeps exiting Unable to start rsync, lock file still exists, PID ... on every run (ftpsync takes the lock via noclobber echo $$ > ${LOCK}, see archvsync bin/ftpsync). We hit this repeatedly at NJU and had to remove the markers manually.

The fix: remove marker files older than 12h at job start, in one find(1) call:

if [[ -n "${TUNASYNC_WORKING_DIR:-}" ]]; then
	find "${TUNASYNC_WORKING_DIR}" -maxdepth 1 -type f \
		-name 'Archive-Update-in-Progress-*' -mmin +720 -print -delete || true
fi

• The 12h age limit protects a manual or push-triggered ftpsync currently running on the same tree (tunasync serializes its own jobs, but not out-of-band runs).
• -maxdepth 1 -type f keeps find on the top level, so a matching directory is never descended into nor deleted; || true prevents a find error from aborting the script under set -e/pipefail, and the guard keeps set -u happy when TUNASYNC_WORKING_DIR is unset.
• Assumes ftpsync's TO equals TUNASYNC_WORKING_DIR (noted in the script comment).

Verified locally: a 13h-old Archive-Update-in-Progress-* directory containing an old file is left fully intact, stale lock files are removed and logged, and the block is a no-op when TUNASYNC_WORKING_DIR is unset.

[Copilot]

Copilot review overview

🟡 Changes recommended

The new cleanup block can cause the script to abort unexpectedly under set -u/set -e (unset TUNASYNC_WORKING_DIR and fragile find command substitution), which is a behavioral regression risk.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Guard unset working dir and handle find errors safely · New

What changed in this PR

This PR prevents downstream mirrors from seeing stale Debian ftpsync “sync in progress” markers when a previous run was killed, by proactively removing old Archive-Update-in-Progress-* files at the start of a job.

Changes:

• Add a startup cleanup loop that deletes Archive-Update-in-Progress-* markers older than 12 hours.
• Log each stale marker removal to aid debugging.

File	Description
debian.sh	Adds pre-sync cleanup of stale Archive-Update-in-Progress-* lock/marker files to avoid false “sync in progress” signals.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yaoge123]

Addressed in 38f5146: the block is now guarded against an unset TUNASYNC_WORKING_DIR, and the staleness check uses find -mmin +720 with stderr suppressed instead of a -z/-mmin -720 command substitution.

[yaoge123]

Copilot review items addressed:

• The stale-lock check now uses find "${lock}" -mmin +720 -print -quit | grep -q . as a conditional pipeline instead of a command-substitution emptiness test, so a find error cannot terminate the script under set -e/pipefail (3fb211e).
• The set -u guard for an unset TUNASYNC_WORKING_DIR was already added in 38f5146 ([[ -n "${TUNASYNC_WORKING_DIR:-}" ]]); verified the cleanup block is skipped cleanly when the variable is unset or empty.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The cleanup must avoid failing when a matching stale path is a directory.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Guard unset working dir and handle find errors safely

[yaoge123]

Round-2 review item addressed in 88602c4: the stale-lock find now uses -type f, so a matching stale path that is a directory is skipped instead of reaching rm -f (which would fail and abort the script under set -e). Verified locally: a 13h-old directory named Archive-Update-in-Progress-* is left untouched and the cleanup block completes with exit 0, while stale lock files are still removed.

[happyaron]

Thanks. One question about the failure mode first: ftpsync already takes over an existing Archive-Update-in-Progress-* lock when kill -0 <pid> fails, and its cleanup trap removes the lock at the end of that run. So a killed run's marker should normally be gone after the next sync. Is what you're seeing PID reuse after a container restart, where the stale PID matches a live process and ftpsync keeps exiting with "lock file still exists"? If so, please put that in the comment and commit message, since it's the real reason this is needed.

On the 12h threshold: the comment says any existing lock belongs to a dead run, which would justify no age limit, and the limit means up to 12h of failed runs before recovery. If you're keeping it to protect a manual or push-triggered ftpsync on the same tree, please say so in the comment.

Optional simplification. This does the same thing without the loop and avoids the set -e concerns:

find "${TUNASYNC_WORKING_DIR}" -maxdepth 1 -type f \
    -name 'Archive-Update-in-Progress-*' -mmin +720 -print -delete || true

This also assumes ftpsync's TO equals TUNASYNC_WORKING_DIR, which is worth a note in the comment.

[Copilot]

Copilot review overview

🟡 Changes recommended

Exclude matching directories from cleanup before approval.

Review effort: Lite
Findings: 1

Open (1)

• Prevent find from descending into lock directories · New

[yaoge123]

Resolved in 8a2da45 by adopting the reviewer's suggested one-shot form: find "${TUNASYNC_WORKING_DIR}" -maxdepth 1 -type f -name 'Archive-Update-in-Progress-*' -mmin +720 -print -delete || true. -maxdepth 1 keeps find on the top level, so a matching directory is never descended into nor deleted (verified locally: a 13h-old Archive-Update-in-Progress-* directory containing an old file is left fully intact).

[yaoge123]

@happyaron Thanks for the review. All points addressed in 8a2da45:

1. Failure mode confirmed. You called it exactly. From ftpsync source (archvsync master, bin/ftpsync around line 542): the lock is taken via noclobber echo $$ > ${LOCK}; if that fails, modern bash does kill -0 $(< ${LOCK}) and exits Unable to start rsync, lock file still exists, PID ... when the recorded PID is alive. Our workers run in Docker containers, and a container restart resets the PID namespace — the stale lock's PID then collides with a live in-container process, kill -0 succeeds, and ftpsync keeps exiting "lock file still exists" on every run. That is precisely what we hit in production; it is now documented in the script comment and the commit message.
2. 12h threshold rationale added to the comment: it protects a manual or push-triggered ftpsync currently running on the same tree (tunasync serializes its own jobs, but not out-of-band runs).
3. Simplification adopted: single find -maxdepth 1 -type f -name ... -mmin +720 -print -delete || true (also resolves Copilot's follow-up about descending into directories). The comment now notes the assumption that ftpsync's TO equals TUNASYNC_WORKING_DIR.

[yaoge123]

Branch cleanup note: this branch has been squashed to a single commit, 62e0850. All commit SHAs referenced earlier in the review threads (38f5146, 3fb211e, 88602c4, 8a2da45) are now orphaned commits — the links still open, but please rely on the current diff, whose tree is byte-identical to the previous head 8a2da45.

Review items → how they were addressed (all included in the current diff)

• Unset TUNASYNC_WORKING_DIR aborts under set -u; find in command substitution is fragile under set -e/pipefail (Copilot round 1) → the block is guarded by [[ -n "${TUNASYNC_WORKING_DIR:-}" ]] and the final form is a single find ... -print -delete || true, so a find error cannot terminate the script.
• find could descend into a matching directory and rm -f would then abort the sync (Copilot round 2) → -maxdepth 1 -type f keeps find on the top level and deletes files only; verified locally that a 13h-old Archive-Update-in-Progress-* directory containing an old file is left fully intact.
• Failure-mode question (@happyaron) → confirmed from archvsync bin/ftpsync: the lock is taken via noclobber echo $$ > ${LOCK}; on failure modern bash does kill -0 $(< ${LOCK}) and exits "lock file still exists" when the recorded PID is alive. A container restart resets the PID namespace, so the stale PID can collide with a live in-container process — this is now documented in the script comment and the commit message.
• 12h threshold rationale → documented: it protects a manual or push-triggered ftpsync currently running on the same tree (tunasync serializes its own jobs, but not out-of-band runs).
• Optional simplification → adopted verbatim: one find "${TUNASYNC_WORKING_DIR}" -maxdepth 1 -type f -name 'Archive-Update-in-Progress-*' -mmin +720 -print -delete || true; the TO == TUNASYNC_WORKING_DIR assumption is noted in the comment.

Some earlier in-thread replies described intermediate states of the branch; please rely on the current diff and this summary.