[Koha-bugs] [Bug 11078] rebuild_zebra.pl can lose updates due to race condition during full rebuilds

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Mon Feb 10 04:53:03 CET 2014


http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11078

--- Comment #49 from Doug Kingston <dpk at randomnotes.org> ---
(In reply to M. de Rooy from comment #48)
> QA Comment:
> First, I like this patch and would like to see it reach master soon.
> Compliments for the solid testing of Martin by the way. I have taken the
> liberty of trusting Martin and Robin here for the package related stuff.
> I tested daemon and non-daemon simultaneously. Added some debug sleeps and
> prints to have some grip on the tests.
> Although I am inclined to pass QA on this patch (no complaints from qa
> tools), I see some room for minor improvements in a follow-up:
> 
> 1) There is the probably very rare circumstance of: The flock call produces
> a fatal error if used on a machine that doesn't implement flock(2), fcntl(2)
> locking, or lockf(3).
> Could you wrap the flock call inside an eval in a function? The daemon
> should only eval the flock at least once..

Please point me at a similar usage elsewhere so I can make sure to do what you
intend here.

> 
> 2) You are waiting on the lock with the one-time invocation. If I am not
> running daemon mode, but I run the one-time invocation via cron very
> regularly, I would (personally) prefer to skip the one-pass instead of
> waiting for the lock.
> I agree that it is somewhat arbitrary what is the best thing here. Could we
> resolve that by adding a simple command-line parameter that tells
> rebuild-zebra to wait or skip? (Skip by default, I guess :-)

I do not have a preference on the default behavior here.  I can also see people
being surprised that their command exits without waiting by default as the most
likely interference is from an incremental update at about the same time which
will exit quickly.  Adding a flag to control waiting or exiting is fine.  It
should complement the default behavior.

I would like some sort of consensus from the community before I start changing
code.  If a couple of more people concur, I can make the change.  Lets try to
close on this issue quickly - can you reach out to a couple of senior folks for
an opinion?  Otherwise, lets leave as is.

> 
> 3) What about t/db_dependent/zebra_config.pl, called from Search.t?

Well spotted, I will add a patch for this file too.  Its a one liner.

> 
> 4) And finally :) what about the defaulting to /var/lock if the zebra lock
> has not been defined or so? mkdir /var/lock/rebuild: Permission denied at
> misc/migration_tools/rebuild_zebra.pl line 161. I guess that this permission
> problem could popup in many cases more..

Something like this?
my $lockdir = C4::Context->config("zebra_lockdir") // "/var/lock";
$lockdir .= "/rebuild";
unless (-d $lockdir) {
    make_path($lockdir, {verbose=>0, mode=>oct(755), error=>\$err})
    $lockdir = "/var/lock" if (@$err);
}
my $lockfile = $lockdir . "/rebuild..LCK";


> 
> With four points after all, I am sorry that I did no longer dare to move it
> to Passed QA rightaway. So first setting it to Failed QA in order to get
> some feedback. Thanks.

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list