Bug 6435 - [ENH] Added daemon mode parameters to rebuild_zebra.pl
Summary: [ENH] Added daemon mode parameters to rebuild_zebra.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Doug Kingston
QA Contact: Bugs List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-31 11:59 UTC by Elaine Bradtke
Modified: 2015-06-04 23:23 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this: 2013-09-24 00:00:00
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Patch to add daemon mode to rebuild_zebra.pl (3.33 KB, patch)
2013-06-28 06:34 UTC, Doug Kingston
Details | Diff | Splinter Review
Improved patch to add daemon mode to rebuild_zebra.pl (4.84 KB, patch)
2013-07-17 04:09 UTC, Doug Kingston
Details | Diff | Splinter Review
My attempt to generate a useful patch from Doug's bizarre diff file. (3.59 KB, patch)
2013-08-23 03:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Convert Doug's diff into a patch. (3.59 KB, patch)
2013-08-23 04:03 UTC, M. Tompsett
Details | Diff | Splinter Review
Proper GIT patch for the daemon mode addition to rebuild_zebra.pl (4.63 KB, patch)
2013-09-13 18:43 UTC, Doug Kingston
Details | Diff | Splinter Review
Proper GIT patch for the daemon mode addition to rebuild_zebra.pl (w/typo fixed) (1.44 KB, patch)
2013-09-13 19:19 UTC, Doug Kingston
Details | Diff | Splinter Review
Bug 6435 Add daemon mode to rebuild_zebra.pl (4.88 KB, patch)
2013-09-24 06:03 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6435 Add daemon mode to rebuild_zebra.pl (w/typo fix) (1.48 KB, patch)
2013-09-24 06:04 UTC, Chris Cormack
Details | Diff | Splinter Review
Add locking to rebuild_zebra to prevent races (3.99 KB, patch)
2013-10-17 22:13 UTC, Doug Kingston
Details | Diff | Splinter Review
Small perl program to flock files and run commands (519 bytes, text/x-perl-script)
2013-10-17 22:32 UTC, Doug Kingston
Details
empty file to allow obsoleting other files. (30 bytes, text/plain)
2013-10-20 02:11 UTC, Doug Kingston
Details
Bug 6345 Add locking to rebuild_zebra (4.13 KB, patch)
2013-10-21 22:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
[ENH] Bug 6435 Add daemon mode to rebuild_zebra.pl (revised 10/27) (5.06 KB, patch)
2013-10-28 02:24 UTC, Doug Kingston
Details | Diff | Splinter Review
Bug 6435 Add daemon mode to rebuild_zebra.pl (5.10 KB, patch)
2013-11-10 21:59 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6435 Add daemon mode to rebuild_zebra.pl (5.17 KB, patch)
2013-11-11 12:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
QA Patch - Make daemon mode imply -z -a -b and abort on incompatible flags (2.30 KB, patch)
2013-11-17 21:20 UTC, Doug Kingston
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Elaine Bradtke 2011-05-31 11:59:09 UTC
This applies only to things catalogued since upgrading to 3.4
The search results for these new records show
no items in the hit list.
But if you click on the title to bring up the normal view, the items
are there, and are fully functional.
The OPAC seems unaffected.
The records created before we moved to 3.4 are also unaffected.
Comment 1 Chris Cormack 2011-05-31 12:11:49 UTC
I cant replicate this, both with and without xslt turned on
Comment 2 Doug Kingston 2011-05-31 12:35:49 UTC
Just to be sure, is it save to rerun remove_items_from_biblioitems.pl?  I can rerun to make sure this has been done.  (and then rebuild the zebra index)
Comment 3 Elaine Bradtke 2011-06-13 15:53:22 UTC
To clarify:
Records added before the switch display in the hit list with three columns:
Results, Location, and an unlabeled one that contains a list of items
for that biblio.
The biblios added after the upgrade follow after the ones added before so it's easy to spot the change in our catalogue.
The biblios added after the switch all say No items in the third
column.
But the items are there when you click on the title and even in the
hit list, you have a link to Edit items, even when it says no items.
The inconsistency is curious.  But I cannot find a difference between the records created before and after the switch that would cause this.
Comment 4 Kyle M Hall 2011-06-16 11:45:39 UTC
I just experienced the same problem on a freshly installed test server. All mandatory and optional data was imported at the time of the installation. No settings had been touched as far as I am aware. However, any items that were catalogued were showing in the results as "no items", but the record details listed the items just fine. 

I re-indexed the catalogue with "misc/migration_tools/rebuild_zebra.pl -b -r -v" and now everything shows up just fine.

When I set this installation up, I used the koha zebraqueue daemon to manage the zebraqueue. Could this be the source of the problem? It must be something zebra related for a re-index to fix the problem for me.
Comment 5 Doug Kingston 2011-07-06 22:23:01 UTC
We have now upgraded to 3.4.2 and the problem persists.  We can fix it by running './rebuild_zebra.pl -b', but this only fixes it until we update the record and then a search that returns the biblio will again show "No items".  This is definitely some sort of Koha to Zebra update issue.

The only errors I find recorded in /var/log/koha around that time are:

[Wed Jul 06 22:54:29 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:54:29 2011] search.pl: Use of uninitialized value $key in hash element at /usr/share/koha/lib/C4/Templates.pm line 227., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/detail.pl?biblionumber=14329
[Wed Jul 06 22:54:31 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:54:31 2011] search.pl: Use of uninitialized value $key in hash element at /usr/share/koha/lib/C4/Templates.pm line 227., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/search.pl?q=evelyn+sharp
[Wed Jul 06 22:55:39 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:55:39 2011] addbiblio.pl: Subroutine plugin_parameters redefined at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/callnumber.pl line 39., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/search.pl?q=evelyn+sharp
[Wed Jul 06 22:55:39 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:55:39 2011] addbiblio.pl: Subroutine plugin_javascript redefined at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/callnumber.pl line 42., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/search.pl?q=evelyn+sharp
[Wed Jul 06 22:55:39 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:55:39 2011] addbiblio.pl: Subroutine plugin redefined at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/callnumber.pl line 73., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/search.pl?q=evelyn+sharp
[Wed Jul 06 22:55:59 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:55:59 2011] search.pl: Use of uninitialized value $key in hash element at /usr/share/koha/lib/C4/Templates.pm line 227., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/detail.pl?biblionumber=14329
[Wed Jul 06 22:56:01 2011] [error] [client 92.234.0.16] [Wed Jul  6 22:56:01 2011] search.pl: Use of uninitialized value $key in hash element at /usr/share/koha/lib/C4/Templates.pm line 227., referer: http://catalogue-admin.efdss.org/cgi-bin/koha/catalogue/search.pl?q=evelyn+sharp

Zebraqueue daemon is running and appears to be happy.  Syslog show the following almost immediately when I update a record:

Jul  6 23:12:49 www Koha Zebraqueue [6696]: 1 operations waiting to be run 
Jul  6 23:12:49 www Koha Zebraqueue [6696]: Executing zebra operations 

Further ideas are most welcome.
Comment 6 Doug Kingston 2011-07-07 11:50:14 UTC
So it looks like some problem with the zebra incremental update managed by the zebraqueue daemon.  Rebuilds of the index (rebuild_zebra.pl -b) resolves the issue temporarily (until the next record update).  It appears to be an issue induced by the upgrade we did from v3.2.6 to v3.4.1.  We are now on v3.4.2 and the issue persists.
Comment 7 Sébastien Marie 2011-07-07 12:57:49 UTC
I can confirm this issue: we have the same (running 3.4.2), and it was noted after transition from 3.2.x to 3.4.x

As workaround I have set cronjobs for launch regulary 'rebuild_zebra.pl -b -a -z'.
Comment 8 Fridolin SOMERS 2013-06-17 14:23:25 UTC
I think this bug is a consequence of removing items form MARC.
When using rebuild_zebra.pl, in get_raw_marc_record() :
 - without "--noxml" option the items are added to MARC::Record in GetMarcBiblio() call.
-  with "--noxml" option, MARC::Record is build from biblioitems.marc and items are not added.

This might be corrected by Bug 10482.
Comment 9 Doug Kingston 2013-06-27 05:56:03 UTC
This is an interesting observation Fridolyn.  We are running 3.10.6.  BIB_INDEX_MODE=dom.  We have recently started seeing problems again with item records disappearing when the biblio record is updated.  If you do a search, you will get back the matching biblios, and the modified one will show "No items.", but if you drill into the result, the item *is* displayed, so its in the Koha system.  This happens with biblios that have a single item record.  If I reindex with "rebuild_zebra.pl -b -v -x" then the item reappears in catalog searchs.  Could it be that after the biblio change, an incomplete MARC record is  exported for update?  We are using the koha-index-daemon.
Comment 10 Fridolin SOMERS 2013-06-27 07:27:47 UTC
You mean zebraqueue-daemon.pl ?
Comment 11 Doug Kingston 2013-06-27 16:56:17 UTC
I was using koha-index-daemon (http://search.cpan.org/~fredericd/Koha-Contrib-Tamil-0.029/bin/koha-index-daemon) but clearly this does not correctly export all records.  So I have stopped and reverted back to the less elegant (but functioning) koha-zebraqueue-daemon (zebraqueue-daemon.pl).  We should either get this other daemon fixed or removed.  I forget how I got pointed to it originally.
Our problem is now resolved.
Comment 12 Jared Camins-Esakov 2013-06-27 17:01:09 UTC
zebraqueue_daemon is broken and was removed from Koha several versions ago. Do not use it. Your best bet would be to either use rebuild_zebra and teach your staff to deal with the delay, or to find someone who can troubleshoot the koha-index-daemon from Tamil.
Comment 13 Doug Kingston 2013-06-28 04:35:55 UTC
OK, I have gone to the cron job.  I will look into an alternative wrapper daemon that just checks the mysql queue and calls rebuild only when necessary.
Comment 14 Doug Kingston 2013-06-28 06:34:48 UTC Comment hidden (obsolete)
Comment 15 Frédéric Demians 2013-06-28 06:50:15 UTC
Your patch contains:

  sleep 5;

This must be configurable. 5 seconds is too small for large catalogs.
Comment 16 Doug Kingston 2013-07-17 04:09:00 UTC Comment hidden (obsolete)
Comment 17 Fridolin SOMERS 2013-08-13 11:02:11 UTC
(In reply to Doug Kingston from comment #16)
> Created attachment 19710 [details] [review] [review]
Your patch is very odd.
Did you create it with git format-patch ?
Comment 18 Liz Rea 2013-08-22 21:30:35 UTC
Hi Doug,

I would like to test this patch, however it appears to be formatted incorrectly - please see http://wiki.koha-community.org/wiki/Version_Control_Using_Git for help on setting up git and sending in patches. 

If you are having trouble with any of that, do say and I'll see about making a git formatted patch for you - but please have a try at doing it with git first. :)

Thanks,
Liz
Comment 19 M. Tompsett 2013-08-23 03:57:24 UTC Comment hidden (obsolete)
Comment 20 M. Tompsett 2013-08-23 04:03:14 UTC Comment hidden (obsolete)
Comment 21 Doug Kingston 2013-09-13 18:43:46 UTC Comment hidden (obsolete)
Comment 22 M. Tompsett 2013-09-13 18:46:00 UTC
"Seconcds to sleep between checks" typo in the patch.
Comment 23 Doug Kingston 2013-09-13 19:19:20 UTC Comment hidden (obsolete)
Comment 24 M. Tompsett 2013-09-20 13:00:33 UTC
Changed status, because Doug forgot. :)
Comment 25 I'm just a bot 2013-09-24 05:42:51 UTC
Applying: Bug 6435 Add daemon mode to rebuild_zebra.pl (w/typo fix)
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 6435 Add daemon mode to rebuild_zebra.pl (w/typo fix)
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 26 Chris Cormack 2013-09-24 05:56:49 UTC
Ahh I see the problem, the first patch has been obsoleted, there should be 2 patches on this bug, will restore the first one and test again.

Yay for the bot catching this!
Comment 27 Chris Cormack 2013-09-24 06:03:56 UTC Comment hidden (obsolete)
Comment 28 Chris Cormack 2013-09-24 06:04:12 UTC Comment hidden (obsolete)
Comment 29 Katrin Fischer 2013-10-05 22:16:52 UTC
Please provide a step by step test plan to make this easier to QA. Thx!
Comment 30 Doug Kingston 2013-10-06 01:55:11 UTC
Apply both patches above.  One is the real code and the small one fixes a typo.

Suggested test plan:
Invoke the daemon exercising all the options I added and observe it doing the
intended updates in line with the update interval.

1. Set up a test koha configuration (populate with data if you wish)
2. Ensure that the zebra database is up to date (rebuild_zebra.pl -b -a)
3. Stop any cronjob that may be updating zebra with rebuild_zebra.pl
4. Start rebuild_zebra in daemon mode (rebuild_zebra.pl -daemon -sleep 3 -b -a v
5. TEST: observe the periodic message "queued records: 0" every 3 seconds.  this will test the use of -sleep option.
6. Add an authority.
7. TEST: observe the daemon pick up the authority change and update zebra (which should be within 3 seconds of completing the add operation)
8. TEST: check that you can search for the new authority from the admin interface
9. Add a biblio.
10. TEST: observe teh daemon pickup up the biblio change and update zebra (which should be within 3 seconds of completing the add operation)
11. TEST: check that you can search for the new biblio in either OPAC or admin interface.

Possibly repeat steps 6-11 with authority and biblio modification and deletion.
Comment 31 Katrin Fischer 2013-10-06 09:00:08 UTC
Doug, you wrote the patch, so you are assignee :)
Comment 32 Doug Kingston 2013-10-17 22:13:40 UTC Comment hidden (obsolete)
Comment 33 Doug Kingston 2013-10-17 22:32:53 UTC Comment hidden (obsolete)
Comment 34 Martin Renvoize 2013-10-17 23:32:20 UTC
Any chance we can move the 'Add locking to rebuild_zebra to prevent races' followup patch to a dependant bug instead of adding it here.. That way QA can go on with the deamon mode patch without being held in waiting for a further signoff.  (PS, if you do that, I'll test and sign off on it).
Comment 35 Doug Kingston 2013-10-18 05:19:38 UTC
I have created separate bug 11078 to track the race condition and its locking fix.
11078 depends on this bug.  6435 does not create any race condition that did not already exist.  The patch for locking and the flock test program are attached to 11078.
Comment 36 Doug Kingston 2013-10-20 02:11:21 UTC Comment hidden (obsolete)
Comment 37 M. Tompsett 2013-10-20 16:00:45 UTC
Comment on attachment 22080 [details]
empty file to allow obsoleting other files.

There is no need for this file.
To obsolete files:
Click on "Details" beside the attachment name.
Click on "Edit Details" beside the name at the top.
Check the obsolete box near the middle/bottom right.
Click the appropriate save/submit button.
To un-obsolete files:
Click on link to show all the obsoleted items
Click on "Details" beside the attachment name.
Click on "Edit Details" beside the name at the top.
Un-check the obsolete box near the middle/bottom right.
Click the appropriate save/submit button.
Comment 38 Martin Renvoize 2013-10-21 22:10:07 UTC Comment hidden (obsolete)
Comment 39 Martin Renvoize 2013-10-21 22:12:22 UTC
Comment on attachment 22201 [details] [review]
Bug 6345 Add locking to rebuild_zebra

Oops.. wrong bug.
Comment 40 Katrin Fischer 2013-10-27 07:24:21 UTC
I think this doesn't work quite right if you use the daemon mode only for -b not for -a.

The command I ran was:
./misc/migration_tools/rebuild_zebra.pl -daemon -sleep 4 -b -v -z

Then I added a new authority, it keeps repeating the following lines:

queued records: 1
skipping authorities
====================
exporting biblio
====================

Records exported: 0

Records exported: 0
====================
REINDEXING zebra
====================

The record is not reindexed, but Koha keeps trying. I think this is because the SQL statement doesn't differentiate between authority and biblio records.

Doug, could you take a look?
Comment 41 Doug Kingston 2013-10-28 02:24:34 UTC Comment hidden (obsolete)
Comment 42 M. Tompsett 2013-10-28 04:52:16 UTC
I'm just curious if you would want to run daemon mode for only biblios or only authorities. If this process is going to run in daemon mode, would you not want both to be picked up? Otherwise, you'd have to run two daemons (one for bibs and one for auths). Changing the status to needs sign off, since this question doesn't really affect whether the code does what it is supposed to do, but I haven't tested it.
Comment 43 Katrin Fischer 2013-10-28 05:25:59 UTC
Yes, you'd probably want both, but for example if your library don't uses authorities and you don't turn it on and then someone catalogs an authority for testing... it shouldn't lead to trouble. It's just something that could go wrong that we might want to avoid either by documentation or by catching it in the code.
Comment 44 Doug Kingston 2013-11-10 21:38:20 UTC
I addressed the recent QA issue on 10/28 with the updated patch.  Can this be re-reviewed?
Comment 45 Chris Cormack 2013-11-10 21:59:16 UTC Comment hidden (obsolete)
Comment 46 Martin Renvoize 2013-11-11 12:09:38 UTC
Created attachment 22859 [details] [review]
Bug 6435 Add daemon mode to rebuild_zebra.pl

This change adds code to check the zebraqueue table with a cheap SQL query
and a daemon loop that checks for new entries and processes them incremantally
before sleeping for a controllable number of seconds.  The default is 5 seconds
which provides a near realtime search index update.  This is desirable particularly
for libraries that are doing active catalogue updating.  The query is adjusted
based on whether -a, -b, or -a -b are specified.

Help text updated.  Tested against a live 3.12 system.

Note that this fix will benefit from the fix to lack of locking (bug 11078)

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2013-11-11 13:00:37 UTC
I'm up and down on passing this one.. discussed it with Katrin..

1. Passes QA script (after I fixed some minor white space errors - Doug, please take a look at http://blog.bigballofwax.co.nz/2011/12/15/fixing-whitespace-when-apply-patches-with-git/ to stop these in their tracks before they hit QA with future patches ;) )

2. Tested with various combinations of options: -a -daemon, -b -daemon, -a -b -daemon, -a -b -x -daemon, all worked as expected along with not breaking existing options.


One niggle worth discussing: The intention of this patch is clear, to allow regular rebuilds as a daemon.. However, the patch allows all/any params to be passed along with the daemon mode. This slightly scares me as it becomes rather easy to break the system using the wrong combination of options. (for instance, running with -r would mean that every time a catalogue update happens, the zebra index disappears moments later whilst a full rebuild is happening)  I don't think it's a QA fail, but it may be worth documenting that certain options don't work well with a daemon model.
Comment 48 Martin Renvoize 2013-11-11 13:05:36 UTC
Follow-up note 1: I also tested the -sleep option and it worked well.
Follow-up note 2: As an afterthought, should the -z be implied by daemon mode?
Comment 49 Galen Charlton 2013-11-14 18:43:33 UTC
(In reply to Martin Renvoize from comment #48)
> Follow-up note 1: I also tested the -sleep option and it worked well.
> Follow-up note 2: As an afterthought, should the -z be implied by daemon
> mode?

In response to this and the previous comment, my view is that --daemon should imply (and be) "here's how you keep your Zebra index up to date."  IOW, I think --daemon should imply the following options:

-z
-a
-b

It should exclude (by refusing to start or by simply ignoring) the following options:

-r
-s
-k
-h
-y
-where
-length
-offset
-I | -skip-index

I wouldn't consider this a blocker for putting the patch in master, but lean towards considering a it blocker for putting in a stable branch.
Comment 50 Doug Kingston 2013-11-14 19:43:04 UTC
I can try to pull that patch together this weekend.

-Doug-
Comment 51 Galen Charlton 2013-11-15 16:34:46 UTC
(In reply to Doug Kingston from comment #50)
> I can try to pull that patch together this weekend.

Great, thanks!
Comment 52 Doug Kingston 2013-11-17 21:20:00 UTC
Created attachment 22986 [details] [review]
QA Patch - Make daemon mode imply -z -a -b and abort on incompatible flags
Comment 53 Galen Charlton 2013-11-24 18:28:02 UTC
Pushed to master.  Thanks, Doug!  Also, this appears to be your first patch accepted into Koha.  Congratulation!
Comment 54 Martin Renvoize 2013-11-26 09:17:55 UTC
Nice work Doug, excellent follow through.
Comment 55 Fridolin SOMERS 2013-12-13 10:42:48 UTC
This patch has been pushed to 3.14.x, will be in 3.14.1

Contains 3 patches :
08c8bea Bug 6435: (follow-up) make -daemon really imply -a and -b
ae9e02a Bug 6435: (follow-up) rebuild_zebra -daemon option now smarter
e7482ec Bug 6435: Add daemon mode to rebuild_zebra.pl