Bug 11078: QA Follow-up for missing file permissions on lockfile

The original patch creates a lockfile in the ZEBRA_LOCKDIR.
It can fall back to /var/lock or even /tmp.
If the create fails, it dies. This can be considered as very
exceptional.

This followup adjusts the fallback location in /var/lock or /tmp
slightly.  It appends the database name to the folder in order to
prevent interfering between multiple Koha instances. Creation of the
lockfile has been moved to a subroutine extending directory and file
creation testing.

In the very unlikely case that we cannot create the lockfile (after
three separate tries), this follow-up allows you to continue instead
of die.  This is just as we did before we had file locking here. Every
time skipping a reindex could cause more harm than continuing and
having the race condition once in a while.

Test plan:
Test adding and removing lockdir from your koha-conf.xml. Check fallback.
Note that fallback in /var/lock or /tmp must contain database name.
Remove the lockdir config line and remove permissions from fallback. In
this case the reindex should continue but with a warning.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with daemon and one-off invocation simultaneously.
Tested new wait parameter.
Tried all variations of lock directory (changing permissions etc.)

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(cherry picked from commit 07de37f0e5ea8685573794ff6a84ab30e32f1973)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/misc/migration_tools/rebuild_zebra.pl b/misc/migration_tools/rebuild_zebra.pl
index ec5f518..de3894f 100755 (executable)
--- a/misc/migration_tools/rebuild_zebra.pl
+++ b/misc/migration_tools/rebuild_zebra.pl
@@ -14,6 +14,8 @@ use C4::Items;
 use Koha::RecordProcessor;
 use XML::LibXML;
 
+use constant LOCK_FILENAME => 'rebuild..LCK';
+
 # script that checks zebradir structure & create directories & mandatory files if needed
 #
 #
@@ -161,13 +163,20 @@ my ($biblioitemnumbertagfield,$biblioitemnumbertagsubfield) = &GetMarcFromKohaFi
 # does not exist and cannot be created, we fall back on /tmp - which will
 # always work.
 
-my $lockdir = C4::Context->config("zebra_lockdir") // "/var/lock";
-$lockdir .= "/rebuild";
-unless (-d $lockdir) {
-    eval { mkpath($lockdir, 0, oct(755)) };
-    $lockdir = "/tmp" if ($@);
-}
-my $lockfile = $lockdir . "/rebuild..LCK";
+my ($lockfile, $LockFH);
+foreach( C4::Context->config("zebra_lockdir"), "/var/lock/zebra_".C4::Context->config('database'), "/tmp/zebra_".C4::Context->config('database') ) {
+    #we try three possibilities (we really want to lock :)
+    next if !$_;
+    ($LockFH, $lockfile) = _create_lockfile($_.'/rebuild');
+    last if defined $LockFH;
+}
+if( !defined $LockFH ) {
+    print "WARNING: Could not create lock file $lockfile: $!\n";
+    print "Please check your koha-conf.xml for ZEBRA_LOCKDIR.\n";
+    print "Verify file permissions for it too.\n";
+    $use_flock=0; #we disable file locking now and will continue without it
+        #note that this mimics old behavior (before we used the lockfile)
+};
 
 if ( $verbose_logging ) {
     print "Zebra configuration information\n";
@@ -175,7 +184,7 @@ if ( $verbose_logging ) {
     print "Zebra biblio directory      = $biblioserverdir\n";
     print "Zebra authorities directory = $authorityserverdir\n";
     print "Koha directory              = $kohadir\n";
-    print "Lockfile                    = $lockfile\n";
+    print "Lockfile                    = $lockfile\n" if $lockfile;
     print "BIBLIONUMBER in :     $biblionumbertagfield\$$biblionumbertagsubfield\n";
     print "BIBLIOITEMNUMBER in : $biblioitemnumbertagfield\$$biblioitemnumbertagsubfield\n";
     print "================================\n";
@@ -194,7 +203,6 @@ my $tester = XML::LibXML->new();
 # to prevent the potential for a infinite backlog from cron invocations, but an
 # option (wait-for-lock) is provided to let the program wait for the lock.
 # See http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11078 for details.
-open my $LockFH, q{>}, $lockfile or die "$lockfile: $!";
 if ($daemon_mode) {
     while (1) {
         # For incremental updates, skip the update if the updates are locked
@@ -210,9 +218,9 @@ if ($daemon_mode) {
     if (_flock($LockFH, $lock_mode)) {
         do_one_pass();
         _flock($LockFH, LOCK_UN);
-    } else {
-        # Can't die() here because we have files to dlean up.
-        print "Aborting rebuild.  Unable to flock $lockfile: $!\n";
+    }
+    else {
+        print "Skipping rebuild/update because flock failed on $lockfile: $!\n";
     }
 }
 
@@ -787,6 +795,16 @@ sub _flock {
     }
 }
 
+sub _create_lockfile { #returns undef on failure
+    my $dir= shift;
+    unless (-d $dir) {
+        eval { mkpath($dir, 0, oct(755)) };
+        return if $@;
+    }
+    return if !open my $fh, q{>}, $dir.'/'.LOCK_FILENAME;
+    return ( $fh, $dir.'/'.LOCK_FILENAME );
+}
+
 sub print_usage {
     print <<_USAGE_;
 $0: reindex MARC bibs and/or authorities in Zebra.