From 00eb104408f1a8702e8028391c0523b07183a739 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 19 Feb 2014 13:58:00 +0100 Subject: [PATCH] Bug 11078: QA Follow-up for missing file permissions on lockfile Content-Type: text/plain; charset=utf-8 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 Tested with daemon and one-off invocation simultaneously. Tested new wait parameter. Tried all variations of lock directory (changing permissions etc.) --- misc/migration_tools/rebuild_zebra.pl | 42 +++++++++++++++++++++++--------- 1 files changed, 30 insertions(+), 12 deletions(-) diff --git a/misc/migration_tools/rebuild_zebra.pl b/misc/migration_tools/rebuild_zebra.pl index ec5f518..de3894f 100755 --- 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. -- 1.7.7.6