Bug 11081

How does this compare to bug 6435?

> How does this compare to bug 6435?

It's a new implementation of indexing: lightweight, surclassable in
order to accommodate SolR, not mixing indexing and data sanitizing,
quicker (equivalent to -xml option). It's a true event driven script,
not sleep(ed) one. It may also have to do with Koha code base
refactoring.

From my perspective, rebuild_zebra.pl has grown too far, doing to much
things.

Take a look at:

  http://wiki.koha-community.org/wiki/Background_indexing_with_Zebra

See also bug 8773

Patch applied cleanly, go forth and signoff

I tested your indexer a couple of years back.  I guess if Moose related deps are around now, this can get in.  I liked running what I had back then when testing.  It was nice to catalogue and have things show up quickly.

I wonder if any of the issues relating to Bug 11078 would be handled by this implementation.

Also, if your implementaion is a long running daemon, do you have any stats on the longest uptime of your production site.

Maybe Robin can comment about if this implementation is package friendly. Multiple database instances handled, etc.

> I tested your indexer a couple of years back.  I guess if Moose related deps
> are around now, this can get in.  I liked running what I had back then when
> testing.  It was nice to catalogue and have things show up quickly.

rebuild_zebra.pl daemon mode provide that today. See my response to M. Tompsett
to find reasons why this enhancement may be of interest in the medium term.

> I wonder if any of the issues relating to Bug 11078 would be handled by this
> implementation.

For me bug 11078 is a non-issue. If you do a full re-indexing, you HAVE to stop
the indexing daemon, or you have to do it when nobody will update
biblio/authortiy records.

> Also, if your implementaion is a long running daemon, do you have any stats
> on the longest uptime of your production site.

~1 year. It is stopped when Koha instances are upgraded.

> Maybe Robin can comment about if this implementation is package friendly.
> Multiple database instances handled, etc.

See bug 8773.

Trying to test

Some conflicts, dependencies installed.

When runnin I found:
misc/bin/koha-index --verbose
Exporting records to update
Can't call method "config" on an undefined value at /home/bgkriegel/kohaclone/C4/Context.pm line 640.

Am I doing something wrong?

Some strange POD errors

 FAIL   Koha/Indexer/Daemon.pm
   OK     critic
   OK     forbidden patterns
   FAIL   pod
                *** ERROR: Unknown command 'attr'  in file Koha/Indexer/Daemon.pm
   OK     valid

 FAIL   Koha/Indexer/Indexing.pm
   OK     critic
   OK     forbidden patterns
   FAIL   pod
                *** ERROR: Unknown command 'method'  in file Koha/Indexer/Indexing.pm
                *** ERROR: Unknown command 'HEAD1'  in file Koha/Indexer/Indexing.pm
   OK     valid

(In reply to Bernardo Gonzalez Kriegel from comment #7)
> Trying to test

Thanks!

> Some conflicts, dependencies installed.

Fixed on the new attached patch.

> When runnin I found:
> misc/bin/koha-index --verbose
> Exporting records to update
> Can't call method "config" on an undefined value at
> /home/bgkriegel/kohaclone/C4/Context.pm line 640.

Fixed.

> Am I doing something wrong?
> 
> Some strange POD errors

Fixed.

I stay available for fixing any issue you may encounter.

Didn't apply cleanly for me.  Can you please rebase?

Created attachment 46176 [details] [review]
Bug 11081: Port Koha::Contrib::Tamil indexer into Koha code base

Add two scripts for indexing: koha-index and koha-index-daemon.

Documentation:

  perldoc koha-index
  perldoc koha-index-daemon

New dependencies:

  MooseX::Getopt
  MooseX::RW
  AnyEvent::Processor

Available as a Debian package: libmoosex-getopt-perl

Test plan:

- Apply the patch
- Install dependencies, if necessary
- Reindex a whole biblio catalog:
  koha-index --verbose
- Reindex a whole authorities catalog:
  koha-index --source authority --verbose
- Modify 1 biblio record, and index it:
  koha-index --select queue
- Modifiy 1 authority record, and index it:
  koha-index --select queue --source authority
- Run indexing daemon, with a 15s timeout:
  koha-index-daemon --timeout 15
- Modify a biblio record: wait 15s, and see
- Modify an authority record: wait 15s, and see

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Works well, followed test plan without problems

One comment: perhaps an option for koha-index-daemon
to work quietly, without printing information.

No koha-qa errors, but new files don't have license,
fixed in followup

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 46177 [details] [review]
Bug 11081: (followup) Add license information

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

Rebased, and revert to Passed QA status.

Pushed to Master - Should be in the May 2016 release. Thanks!

After the discussion on bug 11190, I thought we all agreed that Moo must be used, instead of Moose.

Time::Progress and String::RewritePrefix are listed in the deps, but don't look to be used from this patch.

Created attachment 48126 [details] [review]
Bug 11081: (followup) Rebuild debian/control

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Brendan and Galen, this patch should update the deps. I put it here just in case you didn't rebuild the control file yet.

Pushed the followup - Thanks!  Should be in the May 2016 release.

What's the difference to the existing Indexer and this one?

Also... I am confused about readding Moose too (comment#20)

(In reply to Jonathan Druart from comment #20)
> After the discussion on bug 11190, I thought we all agreed that Moo must be
> used, instead of Moose.

That was my understanding

(In reply to Chris Cormack from comment #26)
> (In reply to Jonathan Druart from comment #20)
> > After the discussion on bug 11190, I thought we all agreed that Moo must be
> > used, instead of Moose.
> 
> That was my understanding

Same here.

(In reply to Tomás Cohen Arazi from comment #27)
> (In reply to Chris Cormack from comment #26)
> > (In reply to Jonathan Druart from comment #20)
> > > After the discussion on bug 11190, I thought we all agreed that Moo must be
> > > used, instead of Moose.
> > 
> > That was my understanding
> 
> Same here.

hmm...  Should I revert this until it's not using the Moose?  Thanks.

(In reply to Brendan Gallagher from comment #28)
> (In reply to Tomás Cohen Arazi from comment #27)
> > (In reply to Chris Cormack from comment #26)
> > > (In reply to Jonathan Druart from comment #20)
> > > > After the discussion on bug 11190, I thought we all agreed that Moo must be
> > > > used, instead of Moose.
> > > 
> > > That was my understanding
> > 
> > Same here.
> 
> hmm...  Should I revert this until it's not using the Moose?  Thanks.

I think so.

(In reply to Tomás Cohen Arazi from comment #29)
> (In reply to Brendan Gallagher from comment #28)
> > (In reply to Tomás Cohen Arazi from comment #27)
> > > (In reply to Chris Cormack from comment #26)
> > > > (In reply to Jonathan Druart from comment #20)
> > > > > After the discussion on bug 11190, I thought we all agreed that Moo must be
> > > > > used, instead of Moose.
> > > > 
> > > > That was my understanding
> > > 
> > > Same here.
> > 
> > hmm...  Should I revert this until it's not using the Moose?  Thanks.
> 
> I think so.

My understanding was that we would not use Moose for CGI scripts as the startup cost is too high for Moose compared to Moo. However, we this is not a CGI script but a daemon, so the startup cost for using Moose is not a serious factor.

What are the consequences for the packages? Are the new dependencies available in Debian? My understanding is that although they are marked optional they will have to be included?

(In reply to Katrin Fischer from comment #31)
> What are the consequences for the packages? Are the new dependencies
> available in Debian? My understanding is that although they are marked
> optional they will have to be included?

That is correct. I have checked and all the dependancies are already packaged for Debian 8 ( Jessie ).

(In reply to Kyle M Hall from comment #32)
> (In reply to Katrin Fischer from comment #31)
> > What are the consequences for the packages? Are the new dependencies
> > available in Debian? My understanding is that although they are marked
> > optional they will have to be included?
> 
> That is correct. I have checked and all the dependancies are already
> packaged for Debian 8 ( Jessie ).

Looks like all the new dependencies for indexer are already provided by Debian for Wheezy and Jessie, or are provided by our own Debian repo.

It's still adding yet another dependency ... and quite a heavy one. I haven't looked up the meeting logs yet, so not sure where we left the discussion in the end, but I think this is worth taking a second look.

This patch set has been reverted in master. It has been decided that we shall not use Moose in Koha. Please modify this patch set to use Moo instead, and it should make it back in!

Too bad. It won't be possible to port this to use Moo in place of Moose. This code rely on Moose functionalities that aren't available with Moo.