[Koha-bugs] [Bug 24027] Adding multiple items is slow

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24027

--- Comment #35 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
I don't know you, Andrew, and I think your patches are doing a good job (well
spotted the bottleneck, the loop design issue, etc. Well done!). But I think it
is time for all of us to think more in the long term.

ModZebra (in the Elasticsearch case) is fetching the MARC record so it gets
indexed by ES. The main problem here is that this is not an async call, but a
blocking one. It doesn't hurt Zebra because in that case we have task queue and
a script that takes care of the queue.

I think it is the way to go: have a task queue and leverage on that.

That said, I never liked that we call ModZebra from Koha::Item->store, and
while I understand the reasons for that and agree to just move forward and fix
later, I wouldn't add even more things related to ModZebra to the method
signature like this. We would just be polluting things even more.

To put it clear, my POV is that this post-success-actions (i.e. actions that
take place if storing the new/updated Item is successful) should be something
that happens in the controller code. If Koha::Item->store didn't call ModZebra
by itself, I'd do something like:

    try {
        # This is just fiction code
        while ( my $item = $items_to_store->next ) {
            $item->store;
        }
    }
    catch {
        handle_exception($_);
    }
    finally {
      if (@_) {
        notify_the_error();
      }
      else {
        # No error
        if (ES) {
            trigger_es_reindex( $item->biblionumber );
        }
        elsif (Zebra) {
            ModZebra(...);
        }
        else {
            Koha::Exception::WTF->throw();
        }
      }
    };

So, I understand the optimization, I fear from my QA POV that we are
introducing even more code that we will need to remove, in the place we
wouldn't want to. And this is always the excuse we use for not doing things
right!

If I was to fix this, I would look into moving the code for ES from plain
triggering the indexing, into using zebraqueue (probably renaming some things
while on it).

I hope this discussion doesn't discourage you, Andrew! I don't want to block
this if the QA team thinks we could live with this.

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