[Koha-bugs] [Bug 22417] Add a task queue

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

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

--- Comment #156 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
QA Comment:
Great to see us moving to a message queue in Koha ! Nice start. And surely
things to discuss ;)
The UI changes are not really the most relevant imo. We could start as an
experimental feature too if needed (in that case we should not touch batch
mod).
But we should add imo some configuration options (see below, setting up vhost,
user, etc.)
And perhaps even more important and already commented on a bit, we are adding a
task queue here with a message queue. Obviously, we could send other stuff into
the MQ; so this might become a problem later on.
And sounding a bit nasty, but not meant to be: We are saving the data into a
background job table as well as sending it to MQ. Aren't we being redundant
here? Actually, it looks like our background job could just hook to the
background jobs table and process what is left there. So what is the real
benefit here in this implementation of the MQ exactly?


Detailed comments:
my $stomp = Net::Stomp->new( { hostname => 'localhost', port => '61613' } );
hardcoded port number / we should probably also let rabbitmq run in its own
container, so replace localhost too: move to koha-conf
No disconnect ? Memory issues when not doing so?
$stomp->connect( { login => 'guest', passcode => 'guest' } );
hmm; isnt this the place for vhosts to come in ? just as vhost, a user should
have been created? / hardcoded: move to koha-conf or separate conf?

# Picking a random id (memcached_namespace) from the config
my $namespace = C4::Context->config('memcached_namespace');
Shouldnt we configure a koha vhost during install (koha-create) and indeed pick
that from koha-conf or so ?
Why not use config/database instead? Normally it looks like koha_instance
Not sure btw if using the term namespace is confusing here; or just a queue
prefix ?

Add Koha::BackgroundJob::BatchUpdateBiblio
Not sure if we need that.
The crux is here only four lines:
            my $record = C4::Biblio::GetMarcBiblio({ biblionumber =>
$biblionumber });
            C4::MarcModificationTemplates::ModifyRecordWithTemplate( $mmtid,
$record );
            my $frameworkcode = C4::Biblio::GetFrameworkCode( $biblionumber );
            C4::Biblio::ModBiblio( $record, $biblionumber, $frameworkcode );
The rest of the code is more or less overhead which perhaps better could be
moved to the general module. The module should care for iterating too. So if we
could pass a sub or method to a general process method along with a params hash
here including biblionumber and marc_template_id, it might be enough? Similar
remarks for enqueue. Would reduce a number of submodules.
So we should enqueue: Koha::Backgroundjob->new({ type => 'batch_record_mod',
... })->enqueue({ mmtid => $mmtid, records => $records  })
And process with Koha::Backgroundjobs->process({ type => ... })  where the
process method iterates over the records and associates type with e.g.
Koha::Biblios::batch_update [four lines above] etc.

installer/data/mysql/atomicupdate/bug_15032.perl
Wrong bug number

diff koha_worker.pl misc/background_jobs_worker.pl
Why did you add koha_worker.pl? We already run the misc script ?

No tests anywhere [yet]? Ok :)

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