xcpdb: avoid race when shards are added

It's possible for the rename() sequence to cause read-only
daemons using ->xdb_shards_flat to load an incomplete set of
contiguous shards and get invalid docids for search results.

With this change, we favor the case where search is momentarily
unavailable rather than giving wrong results during the small
window where Xapcmd->commit_changes runs.

diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index daef896c37e9a435639505211d7d68e3f74a531f..44e0f8e58dca6ac1374ce01d953252a9f7a4bb8c 100644 (file)
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -20,16 +20,23 @@ sub commit_changes ($$$$) {
        my $reshard = $opt->{reshard};
 
        $SIG{INT} or die 'BUG: $SIG{INT} not handled';
-       my @old_shard;
-       my $over_chg;
-       my $mode;
-
-       while (my ($old, $newdir) = each %$tmp) {
+       my (@old_shard, $over_chg);
+
+       # Sort shards highest-to-lowest, since ->xdb_shards_flat
+       # determines the number of shards to load based on the max;
+       # and we'd rather xdb_shards_flat to momentarily fail rather
+       # than load out-of-date shards
+       my @order = sort {
+               my ($x) = ($a =~ m!/([0-9]+)/*\z!);
+               my ($y) = ($b =~ m!/([0-9]+)/*\z!);
+               ($y // -1) <=> ($x // -1) # we may have non-shards
+       } keys %$tmp;
+
+       my ($dname) = ($order[0] =~ m!(.*/)[^/]+/*\z!);
+       my $mode = (stat($dname))[2];
+       for my $old (@order) {
                next if $old eq ''; # no invalid paths
-               $mode //= do {
-                       my ($dname) = ($old =~ m!(.*/)[^/]+/*\z!);
-                       (stat($dname))[2];
-               };
+               my $newdir = $tmp->{$old};
                my $have_old = -e $old;
                if (!$have_old && !defined($opt->{reshard})) {
                        die "failed to stat($old): $!";
@@ -57,11 +64,7 @@ sub commit_changes ($$$$) {
                                        die "rename $old => $new/old: $!\n";
                }
                rename($new, $old) or die "rename $new => $old: $!\n";
-               if ($have_old) {
-                       my $prev = "$old/old";
-                       remove_tree($prev) or
-                               die "failed to remove $prev: $!\n";
-               }
+               push @old_shard, "$old/old" if $have_old;
        }
 
        # trigger ->check_inodes in read-only daemons