Bug 5166 - Reintroduce a zebraqueue daemon, make several rebuild_zebra.pl functions a lib
Summary: Reintroduce a zebraqueue daemon, make several rebuild_zebra.pl functions a lib
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P3 enhancement (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords:
: 5165 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-08-24 16:18 UTC by Tomás Cohen Arazi
Modified: 2012-10-26 00:51 UTC (History)
11 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Initial patch (16.36 KB, patch)
2010-08-24 16:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
THE patch (58.81 KB, patch)
2011-03-02 21:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Initial patch, refactored (libs) (14.55 KB, patch)
2011-03-21 15:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Sysprefs changes (18.40 KB, patch)
2011-03-21 15:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
zebraqueue daemon scripts (21.38 KB, patch)
2011-03-21 15:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
updatedatabase.pl fix (2.39 KB, patch)
2011-03-21 16:19 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Sysprefs make 'experimental' nature explicit (27.55 KB, patch)
2011-04-04 15:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Sysprefs make 'experimental' nature explicit - again (33.05 KB, patch)
2011-04-05 10:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Fix syspref, for applying to new/enh/bug_5166 (4.43 KB, patch)
2011-04-20 15:34 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Take note of Bug 5724 (1.07 KB, patch)
2011-05-03 14:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
New zebraqueue daemon and libs (68.05 KB, patch)
2011-08-03 17:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Rebased against origin/master (41.95 KB, patch)
2011-12-21 14:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Last rebase of the patch (42.12 KB, patch)
2012-03-20 14:03 UTC, Zeno Tajoli
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2010-08-24 16:18:05 UTC
For using with the zebraqueue daemon script I propose refactoring rebuild_zebra.pl script so we can avoid spawning a new perl interpreter each run. Server load in mind.

This is an initial submit so we discuss this topic with something concrete to talk about.

Another option would be to just use a shell script.
Comment 1 Tomás Cohen Arazi 2010-08-24 16:23:37 UTC Comment hidden (obsolete)
Comment 2 Colin Campbell 2011-01-10 17:59:02 UTC
What's the interface to this code to be?
Comment 3 Tomás Cohen Arazi 2011-03-02 21:14:18 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2011-03-03 13:14:12 UTC
For testing follow the old install steps:

{{{
$ sudo ln -s ${SCRIPT_DIR}/koha-zebraqueue-ctl.sh \
     /etc/init.d/koha-zebraqueue-daemon
$ sudo update-rc.d koha-zebraqueue-daemon defaults
}}}
Comment 5 Tomás Cohen Arazi 2011-03-09 16:49:50 UTC
If you'd like to test on a dev install, run daemon like this:

KOHA_DEV_SRC=/home/tomas/Work/koha/devel/koha-community-src
KOHA_DEV_ETC=/home/tomas/koha-dev/etc/koha-conf.xml
MYUSER=<user that runs processes>

su - $MYUSER PERL5LIB=$KOHA_DEV_SRC KOHA_CONF=$KOHA_DEV_ETC $KOHA_DEV_SRC/misc/bin/zebraqueue_daemon.pl
Comment 6 Julian Maurice 2011-03-18 15:31:27 UTC
Patch does not apply.
--
Applying: Refactor rebuild_zebra.pl into a library
/home/julian/koha/versions/koha-community-git/.git/rebase-apply/patch:86: trailing whitespace.
returns the number of updated+deleted authority records 
/home/julian/koha/versions/koha-community-git/.git/rebase-apply/patch:100: trailing whitespace.
returns the number of updated+deleted biblio records 
/home/julian/koha/versions/koha-community-git/.git/rebase-apply/patch:114: trailing whitespace.
returns the number of updated+deleted authority and biblio records 
/home/julian/koha/versions/koha-community-git/.git/rebase-apply/patch:134: trailing whitespace.
returns the number of updated+deleted $record_type records 
/home/julian/koha/versions/koha-community-git/.git/rebase-apply/patch:161: trailing whitespace.
returns the number of updated+deleted $record_type records 
error: patch failed: installer/data/mysql/de-DE/mandatory/sysprefs.sql:285
error: installer/data/mysql/de-DE/mandatory/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/en/mandatory/sysprefs.sql:285
error: installer/data/mysql/en/mandatory/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/fr-FR/1-Obligatoire/unimarc_standard_systemprefs.sql:286
error: installer/data/mysql/fr-FR/1-Obligatoire/unimarc_standard_systemprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/it-IT/necessari/sysprefs.sql:271
error: installer/data/mysql/it-IT/necessari/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/pl-PL/mandatory/sysprefs.sql:283
error: installer/data/mysql/pl-PL/mandatory/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/ru-RU/mandatory/system_preferences_full_optimal_for_install_only.sql:338
error: installer/data/mysql/ru-RU/mandatory/system_preferences_full_optimal_for_install_only.sql: patch does not apply
error: patch failed: installer/data/mysql/uk-UA/mandatory/system_preferences_full_optimal_for_install_only.sql:363
error: installer/data/mysql/uk-UA/mandatory/system_preferences_full_optimal_for_install_only.sql: patch does not apply
Patch failed at 0001 Refactor rebuild_zebra.pl into a library
Comment 7 Tomás Cohen Arazi 2011-03-21 15:21:59 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2011-03-21 15:22:22 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2011-03-21 15:49:37 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 2011-03-21 16:19:03 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi 2011-03-30 11:36:48 UTC
*** Bug 5165 has been marked as a duplicate of this bug. ***
Comment 12 Chris Cormack 2011-04-01 18:39:37 UTC
Correct me if im wrong, but this doesnt seem to touch rebuild_zebra.pl at all

As such the options in the systempreferences only apply if you are running the new, (and hopefully not memory leaking zebraqueue_daemon). 

Before I can merge this into master, we would need to first change the wording in the sys prefs to make it clear that if you are running rebuild_zebra.pl as a cron job these will have no effect.

Then we need to test the daemon does not memory leak (functionality was never a problem with it, leaking memory and taking the system down was)

The branch is in new/enh/bug_5166 please could others take a look.
Comment 13 Tomás Cohen Arazi 2011-04-01 19:04:29 UTC
As-is, this code doesn't hurt any previous functionality. That's what I wanted, and why I didn't touch rebuild_zebra.pl (which is trivial to change to use the sysprefs as default, I can do it if preferred). Also I didn't port fullindex and other rebuild_zebra.pl functionalities to avoid making this a big commit, but those functions are trivial to implement too.
Comment 14 Chris Cormack 2011-04-02 06:29:04 UTC
Ah no, i think we are miscommunicating :)

I don't want it ported to rebuild_zebra.pl, it's too close to the release to do something like that.

What I do want is the words in the preferences screen to make it very clear, these system preferences are only valid if you are using the zebraqueue daemon.
So that people dont change them, and wonder why they are having no effect. 
Also Id like them to make it clear that the daemon is still expiremental, until we are sure it still isn't memory leaking.

So what I would like is a follow up patch to change the wording of the preferences page. If you need help with the words I can help out with that.
Comment 15 Tomás Cohen Arazi 2011-04-04 15:30:43 UTC Comment hidden (obsolete)
Comment 16 Chris Cormack 2011-04-04 20:56:12 UTC
Hi Tomás

What branch did you base your new patch on, it will not apply on
new/enh/bug_5166

Chris
Comment 17 Tomás Cohen Arazi 2011-04-05 10:59:59 UTC Comment hidden (obsolete)
Comment 18 Paul Poulain 2011-04-08 13:36:03 UTC
shouldn't this patch be set to "Needs signoff" again ?
Comment 20 Chris Cormack 2011-04-12 23:19:45 UTC
Pushed up into new/enh/bug_5166 and master merged into it, please test.

It doesn't touch any main functionality so if it doesnt cause any regressions it should be ok for 3.4.0

Please update the bug with your testing report
Comment 21 Tomás Cohen Arazi 2011-04-18 13:37:15 UTC
Added 2 records.
Added 2 authority records.
Modified 1 record (added an item).
Deleted 1 authority record.
Deleted 1 biblio record.

Operations completed successfully.

Just got a warning when updating authority. It actually belongs to AuthoritiesMarc.pm:

Argument "a" isn't numeric in numeric ne (!=) at /home/tomas/Work/koha/devel/koha-community-src/C4/AuthoritiesMarc.pm line 1462.

so not relevant here.
Comment 22 Tomás Cohen Arazi 2011-04-18 13:49:34 UTC
I forgot to mention that I had to change several lines (between #320 and #332) for the search to work in staff interface as the conversion to TT seems broken in this branch.
Comment 23 Tomás Cohen Arazi 2011-04-18 13:51:01 UTC
In Comment 22 I meant to say I had to change several lines IN FILE 'catalogue/results.tt' to be able to test indexing was working.
Comment 24 Chris Cormack 2011-04-18 15:20:28 UTC
The results.tt file has been changed in master, new/enh/bug_5166 needs to have master merged into it.

It is now too close to the release for this to make it into 3.4.0, it also still needs 3rd party sign off.

I have merged master in, and pushed, if you can find someone to sign off on it that would be what would be needed for it to go into master.
Comment 25 Tomás Cohen Arazi 2011-04-18 15:36:17 UTC
Julian Maurice signed-off on this bug. The only change it had was some changes in the wording of the sysprefs. Anyway, I'll ask someone on the irc to sign-off.
Comment 26 Chris Cormack 2011-04-18 15:39:46 UTC
In that case, if you can get someone to signoff on the branch now, just even noting it in the bugreport is fine. That the new/enh/bug_5166 branch works fine, with no changes needed. I will merge it for 3.4.0 as it was signed off before the string freeze.
Comment 27 Chris Cormack 2011-04-19 18:26:58 UTC
Bug found with the patch
Comment 28 Tomás Cohen Arazi 2011-04-20 15:34:56 UTC Comment hidden (obsolete)
Comment 29 Tomás Cohen Arazi 2011-05-03 14:25:36 UTC Comment hidden (obsolete)
Comment 30 Fernando L. Canizo 2011-05-03 14:36:05 UTC
Tested and everything went fine.
Comment 31 Paul Poulain 2011-08-03 13:45:51 UTC
Sorry, but the updatedatabase patch does not apply anymore, could you rebase please ?

Note also i'm confused by the patch names. I have :
0001-Bug-5166-Fix-searching.pref-also-rephrase.patch
0001-Bug-5166-Libraries-and-zebraqueue-daemon-scripts.patch
0002-Bug-5166-Fix-as-of-Bug-5724.patch
0002-Bug-5166-Sysprefs.patch
0003-Bug-5166-zebraqueue-daemon-scripts.patch
0004-Bug-5166-updatedatabase.pl-fix.patch
0005-Bug-5166-Sysprefs-make-experimental-nature-explicit-.patch

The 0001-Bug-5166-Fix-searching.pref-also-rephrase.patch and 0002-Bug-5166-Fix-as-of-Bug-5724.patch being number 6 and 7 in the patch list. In which order should we apply them ?

However, I like the patch and the idea. But unless i've missed something it's not related to "refactor rebuild_zebra.pl" because there's nothing to do with rebuild_zebra. It's more "reintroduce a zebra daemon, and provide tools that will enable us later to refactor rebuild_zebra.pl".
Note i'm happy with this idea, maybe we/you can just update the title of your bug ?
Comment 32 Tomás Cohen Arazi 2011-08-03 17:53:21 UTC Comment hidden (obsolete)
Comment 33 Tomás Cohen Arazi 2011-08-03 18:00:48 UTC
Please note the adition of:

installer/data/mysql/atomicupdate/bug_5166-zebraqueuedaemon_is_back.pl

as noted on IRC this could help QA and RM to avoid rebasing so frecquently.
Comment 34 Paul Poulain 2011-10-24 11:38:04 UTC
Updating Version : This ENH will be for Koha 3.8
Comment 35 Paul Poulain 2011-10-25 15:05:39 UTC
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Comment 36 Katrin Fischer 2011-12-21 05:29:34 UTC
The system preference management has changed since this patch was written, only one file has to be changed now. Please fix and resubmit.
Comment 37 Tomás Cohen Arazi 2011-12-21 14:00:41 UTC Comment hidden (obsolete)
Comment 38 Frédéric Demians 2011-12-25 16:12:55 UTC
I have an alternative implementation of a patch for this bug. I've
packaged it as a CPAN module for easy testing. It contains two scripts:
one for punctual indexing, and one for queued records indexing.

What is specific to this implementation:

  - No use of POE, but AnyEvent:
    http://search.cpan.org/dist/AnyEvent/
  - Moose OO classes
  - Export records for Zebra only in XML
  - No records content modification (leader, authority id, unimarc 100
    tag). As it is, it works with a lot of UNIMARC/MARC21 Koha instances
    loaded with various records types.

How to test it:

- Install the module:

  sudo cpan Koha::Contrib::Tamil

- You have two scripts installed:

  koha-index
  koha-index-daemon

- You can access their respective man pages:

  man koha-index
  man koha-index-daemon

- To reindex a whole DB, standard env variable being set:

  koha-index --verbose
  koha-index --source authority --verbose

- To start the indexing daemon, with indexing occurring every minute:

  koha-index-daemon

- To start the same daemon, with indexing occuring every 30 seconds:

  koha-index-daemon --timeout 30
Comment 39 Liz Rea 2012-02-06 18:04:41 UTC
Testing notes:

Install: 
* One *must* have their Koha environment variables set (KOHA_CONF and PERL5LIB) for the CPAN module to install.

Questions:
* How does this mingle with the packages scripts that do similar things? I understand the daemon will be a different process - what about a normal reindex? Is the koha-index --verbose process in some way superior?

Testing  notes:
I did a batch import of about 30k records - I set the timeout to 5. The daemon picked up between 80 and 200 records a tick, and never seemed to get overwhelmed. 

This was on Frederic's cpan module. 

I think it's good - we should include it. How do you want the signoff as this is a cpan module? Is there a reason to choose one of these implementations over the other? Should I test the other version as well?
Comment 40 Paul Poulain 2012-02-17 14:39:01 UTC
The script devel/misc/testbugzillapatches.pl has detected that this bug does not apply anymore, so i'm marking this bug as "does not apply".

The script is supposed to handle conflicts related to updatedatabase, but no other cases of merge conflict. So the conflict may be trivial to fix.

If it happens that the patch attached does apply cleanly, please send me an email, there's something wrong in the script. It's possible, as it's a recent script.
If the patch applies does not apply as I think, please rebase and resubmit, thanks
Comment 41 Tomás Cohen Arazi 2012-03-19 16:12:46 UTC
I'd like to note that I initiated this bug when we implemented here (UNC) a way (call it wrapper / new daemon / whatever) to achieve the sub-minute record index update our librarians required.

We've been using this in production since early 2010.

We just used the logic already present in rebuild_zebra.pl and thought it would be a fast cool feature everyone could enjoy until that zerba stuff was rewritten.

I rebased this patches for a year and a half, also adjusting some bits to mimic a diverging rebuild_zebra.pl script everytime smth changed.

We don't need this inside koha per-se, and I don't have the time to keep struggling with this patch. Ence, I retire from this bug, and would like to ask if I should close it to clean bugzilla.

Regards
To+
Comment 42 Liz Rea 2012-03-19 18:17:05 UTC
My question still remains - should we pick one of these implementations over the other?

I've been using the cpan module version for several months and let me tell you the freedom to set 5 second zebraqueue updates is awesome. This should be in Koha. 

Can we make it happen? What needs to happen?
Comment 43 Tomás Cohen Arazi 2012-03-19 18:33:17 UTC
(In reply to comment #42)
> I've been using the cpan module version for several months and let me tell
> you the freedom to set 5 second zebraqueue updates is awesome. This should
> be in Koha. 

Yeah, that's why this bug was submitted for the first time.

> My question still remains - should we pick one of these implementations over
> the other?

I think Frederic's rewrite of this is a good one. I personally dont like the idea of it being an external lib, as I think that libs should be tied to specific versions of Koha (think of db changes, we realize smth should be done other way, etc). So a reworked version, more daemon-friendly (so you can do 'update-rc.d koha-zeebraqueue-daemon default' to have it at startup), etc could make it.

What I propose is to close this bug and maybe start a new one on integrating Tamil's lib into Koha. I just abandoned my patches for this bug.
Comment 44 Chris Cormack 2012-03-19 18:49:26 UTC
I agree with this idea, integrating the Tamil code into koha
Comment 45 Zeno Tajoli 2012-03-20 10:17:00 UTC
Hi we are working to commit this patch and after that patch 7430
So we will put all in specific places.
But we need to start with this.

We are at HackFest in Marseille
Comment 46 Zeno Tajoli 2012-03-20 14:03:04 UTC
Created attachment 8357 [details] [review]
Last rebase of the patch
Comment 47 Zeno Tajoli 2012-03-20 14:13:10 UTC
I sugget to close this bug with a 'Wont fix' situation.
For who want background indexing, I will insert instruction in documentation.
See Bug 7759