2
0
mirror of https://github.com/xcat2/xcat-dep.git synced 2026-09-12 04:26:25 +00:00

fix(xcat-dep): scope run lock per-arch and tolerate NFS silly-rename in wipe_tree

Follow-up to the review-fix commit, which broke the two-arch build:

- The run lock was on <output_root>/.sbuild-all.lock, shared by BOTH arch stages.
  But within ONE pipeline run the amd64 and ppc64el stages run concurrently on their
  own hosts against the same --output-root -- a shared lock serializes/deadlocks them.
  Lock per-arch (<output_root>/.sbuild-all.<arch>.lock): only same-arch stages (same
  host) contend, so a local flock is authoritative and cross-run same-arch races are
  still blocked, without touching the intended amd64||ppc64el parallelism.

- wipe_tree died on .nfsXXXX silly-rename artifacts (an already-unlinked file a
  still-open handle -- e.g. an aborted peer build -- keeps alive). Those are not stale
  debs and self-heal. Build #26 hit exactly this: the ppc stage's wipe FATAL'd on
  leftover .nfs* from an aborted run, and failFast then SIGTERM'd the amd64 conserver
  builds mid-apt (the 'conserver rc=1' was collateral). wipe_tree now retries once,
  then dies ONLY if a real (non-.nfs*) file survives; .nfs*-only leftovers are
  tolerated (assemble globs *.deb, never .nfs*, so they can't reach the repo).

perl -c clean; prove t/sbuild-all.t: 71/71.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
This commit is contained in:
Daniel Hilst
2026-08-12 11:58:13 -03:00
parent c808e06da3
commit 3e9cd35ee1
+27 -14
View File
@@ -27,6 +27,7 @@ use warnings;
use Cwd qw(abs_path);
use File::Basename qw(dirname basename);
use File::Path qw(make_path remove_tree);
use File::Find;
use File::Copy qw(copy);
use File::Temp qw(tempdir);
use Getopt::Long qw(GetOptions);
@@ -189,18 +190,18 @@ for my $cn (@dist_list) {
my $staging = "$output_root/staging";
unless ($dry_run) { make_path($staging); }
# Fail-fast exclusive run lock. $staging and $apt_dir are STABLE, SHARED paths (not per-run), so two
# overlapping sbuild-all runs against the same --output-root corrupt each other -- the observed
# "remove_tree .../staging/<cn>/<arch>: Directory not empty" is an NFS silly-rename from a concurrent
# run holding files open. Hold an exclusive flock on <output_root>/.sbuild-all.lock for the whole
# process (LOCK_NB -> fail fast rather than block), mirroring how mockbuild-all.pl locks its output
# base. Not taken under --dry-run (no side effects to protect).
# Fail-fast PER-ARCH run lock. Within ONE pipeline run the amd64 and ppc64el stages run CONCURRENTLY
# on their own hosts against the SAME --output-root (different arch subdirs), so a single shared lock
# would wrongly serialize them (or deadlock). Lock per-arch instead: <output_root>/.sbuild-all.<arch>.lock
# is only ever contended by same-arch stages, which all run on the SAME host -- so a plain local flock
# is authoritative (no cross-host NFS lockd needed). This still blocks a SECOND run's same-arch stage
# (cron vs manual) from racing on this arch's staging + the shared apt tree. Not taken under --dry-run.
unless ($dry_run) {
make_path($output_root);
my $lockfile = "$output_root/.sbuild-all.lock";
my $lockfile = "$output_root/.sbuild-all.$arch.lock";
open($RUN_LOCK_FH, '>', $lockfile) or die "FATAL: cannot open run lock $lockfile: $!\n";
unless (flock($RUN_LOCK_FH, LOCK_EX | LOCK_NB)) {
die "FATAL: another sbuild-all is running (lock held): $lockfile\n";
die "FATAL: another sbuild-all ($arch) is already running (lock held): $lockfile\n";
}
}
@@ -229,16 +230,28 @@ sub run {
return $ec;
}
# wipe_tree: remove_tree that FAILS LOUD. A bare remove_tree() carps-and-ignores an ENOTEMPTY (e.g. an
# NFS silly-rename from a concurrent run); make_path then no-ops on the surviving dir and stale debs
# persist. Capturing {error} and dying makes the corruption fatal instead of silent.
# wipe_tree: remove_tree that FAILS LOUD on real leftovers but TOLERATES NFS silly-rename artifacts.
# A bare remove_tree() carps-and-ignores errors, so stale debs could silently persist -- we must not
# do that. But an ENOTEMPTY here is usually a .nfsXXXX silly-rename: an already-unlinked file that a
# still-open handle (often a peer or aborted build) keeps alive; it is NOT stale build output and
# self-heals when the holder closes. So retry once after a short pause, then die ONLY if a real
# (non-.nfs*) file survives. If the sole survivors are .nfs* artifacts, warn and continue -- assemble
# globs *.deb (never .nfs*), so they cannot leak into the published repo.
sub wipe_tree {
my (@dirs) = @_;
remove_tree(@dirs, { safe => 1, error => \my $err });
if ($err && @$err) {
my @msgs = map { my ($f, $m) = %$_; ($f eq '') ? $m : "$f: $m" } @$err;
die "FATAL: failed to remove @dirs: " . join('; ', @msgs) . "\n";
return unless $err && @$err;
sleep 2; # give a transient silly-rename holder a chance to close
remove_tree(@dirs, { safe => 1, error => \my $err2 });
return unless $err2 && @$err2;
my @real;
for my $d (@dirs) {
next unless -d $d;
find(sub { push @real, $File::Find::name if -f $_ && $_ !~ /^\.nfs[0-9a-f]+$/i }, $d);
}
die "FATAL: failed to wipe @dirs -- real files survive: @real\n" if @real;
warn "WARN: @dirs still holds only NFS silly-rename (.nfs*) leftovers after retry; "
. "tolerating (they self-heal and are never *.deb)\n";
}
# deb_ver_gt: is Debian version $a strictly greater than $b? Uses dpkg's version comparison (the only