This is related to: Bug 15032 - [Plack] Scripts that fork (like stage-marc-import.pl) don't work as expected Bug 1993 - Task Scheduler Needs Re-write And many others. The goal here would be to remove the background execution in real time we are doing right now and delegate the jobs to an external service (handling several workers, load balancing, etc.)
I first tried Minion but found problematic issues for us (Koha): - The mysql backend requide Mojo::mysql which requires DBD::mysql 4.042 However Jessie has 4.028 and Stretch 4.041 - We could have used the Postgres backend, but we would have require MySQL/MariaDB and Postgres, which is not acceptable. I then gave a try to RabbitMQ (https://www.rabbitmq.com/documentation.html), which is an implementation of AMQP in Erlang. If you want more information about it I let you follow the link. The result is quite good, I managed to have a working POC, adapt to our uses, in a single day. Test plan: 0. Prerequisite: % sudo apt install libanyevent-rabbitmq-perl rabbitmq-server % sudo cpanm Net::RabbitFoot # Quid of the debian package? Then make sure the RabbitMQ server is running: % sudo service rabbitmq-server status and start it if needed with % sudo service rabbitmq-server start Then create, or make sure you have a MARC Modification Template, with template_id=1 (like add new field 123$z=foo) 1. To test this patch set you should first understand how the enqueue/consume process work: % perl new_koha_job.pl # Launch it 1+ times => The job must have been enqueued (no error in the output) % perl koha_worker.pl # Will consume the 1+ tasks you enqueued Do not close this worker and start another one in another console, then enqueue more new jobs. Enjoy :) (yes it should be fun) 2. Watch the history of the jobs Hit admin/background_jobs.pl (logged in as superlibrarian) This is an interface (to improve) to the list of pending/running and finished processes 3. Use this whole stuff in a real Koha world: Play with the "Batch record modification" tool (with biblios) Notice the different in the last (report) step and click on the "View detail of the enqueued job" link Discussions: 1. Please grep TODO and FIXME in the patchset, there are lot to do/fix. 2. Even if we do not go for RabbitMQ I think we could start moving the code from pl to Koha::BackgroundJob::*pm It will be "easy" (nothing is easy, right?) to replace it with something else. 3. With this base we will want to answer different needs: * background jobs * task scheduler * reindex process (?) Going further: 1. Implement Koha::BackgroundJob for the different background jobs 2. Provide a koha-worker script to consume to jobs 3. Rewrite the task scheduler with Koha::BackgroundJob (1993) 4. Provide a configuration file to prioritize or postpone jobs (for instance no batch modification on Wednesday between 9-17) - Or maybe we won't need that (?) 5. What would be your needs? Code is at https://gitlab.com/joubu/Koha/commits/bug_22417 (as the code will evolve in the next days I will not pollute here). Switched to 'In Discussion' to collect thoughts.
And the screenshots: https://snag.gy/WVeqDf.jpg What you get once the job has been enqueued https://snag.gy/1GgnN5.jpg Detail of a non started job https://snag.gy/ySnlaF.jpg Detail of a finished job https://snag.gy/s5dF7n.jpg List of jobs
I've taken a brief look at the code and the screenshots and it's looking good.
(In reply to Jonathan Druart from comment #1) > Discussions: > <snip> > 2. Even if we do not go for RabbitMQ I think we could start moving the code > from pl to Koha::BackgroundJob::*pm > It will be "easy" (nothing is easy, right?) to replace it with something > else. I totally agree; it would nice to be as message queue implementation agnostic as possible. > Going further: > 1. Implement Koha::BackgroundJob for the different background jobs > 2. Provide a koha-worker script to consume to jobs > 3. Rewrite the task scheduler with Koha::BackgroundJob (1993) It's been a couple years since I looked at RabbitMQ. Is there a way to cancel/remove/delete a message once it is enqueued? I think this would be useful for a task scheduler. I built that into the POE-based task scheduler I built for https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662. (Actually, I also built-in the ability for in-progress jobs to be canceled in the event a big batch job was started and a person realized it was a mistake.) > 4. Provide a configuration file to prioritize or postpone jobs (for instance > no batch modification on Wednesday between 9-17) - Or maybe we won't need > that (?) Fridolin mentioned something like this to me when I was talking about https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662 with him at Kohacon18 in Portland. He pointed out that for a multi-tenant server you might not want all the different Koha instances on the server to run their background jobs at the same time due to restraints like performance, rate-limiting, connection limits, etc. Off the top of my head, I don't have a solution for this one though, although in the case of https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662 I use separate task schedulers for every Koha instance on the server. In this case, we could potentially use 1 RabbitMQ server, and X number of workers... although we'd have to have a way of setting the KOHA_CONF for each task. That might be the best way to handle multi-tenant scenarios. Alternatively, I think we'd need to have unique queues for each Koha instance and workers for those unique queues. Requires a bit of thought in any case. > 5. What would be your needs? > Ideally, it would be nice to replace my home-made task scheduler in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662 with this one. I like that mine can add/start/pause/stop/remove tasks, but I haven't seen a mainstream task scheduler that is capable of doing that, so that might be too difficult to do here.
I have added a commit to add the ability to cancel a job.
I just tried it and it works like a charm! Great job Jonathan! I think just to polish this functionality would be great start and big progress from what we have now.
(In reply to Jonathan Druart from comment #5) > I have added a commit to add the ability to cancel a job. Thanks, Jonathan!
(In reply to Josef Moravec from comment #6) > I think just to polish this functionality would be great start and big > progress from what we have now. Actually, I think that's a really good point. I think in the community we sometimes have a tendency to debate/discuss things to death... which stifles innovation. I think it could be good to get something polished into Koha (maybe just 1 background job using RabbitMQ to start) and then if that is working well then we add more background jobs. I suppose it might be more efficient to have it fully specified before implementing in Koha rather than doing iterative development but... -- Oh, but I think my concerns about multi-tenant environments are important. At a glance, this implementation seems like it would only allow 1 Koha instance talk to 1 RabbitMQ instance. But I think in practice there are many people who run multiple Koha instances on the same server and it would be good if they could share 1 RabbitMQ instance.
Do we really need the background jobs ? Or should we refactor these scripts in the direction of AJAX API calls in JavaScript ?
(In reply to Marcel de Rooy from comment #9) > Do we really need the background jobs ? Or should we refactor these scripts > in the direction of AJAX API calls in JavaScript ? I am not sure if this would also allow reimplementing the scheduler with scheduled reports? In my understanding some jobs might not be triggered by the GUI.
(In reply to Katrin Fischer from comment #10) > (In reply to Marcel de Rooy from comment #9) > > Do we really need the background jobs ? Or should we refactor these scripts > > in the direction of AJAX API calls in JavaScript ? > > I am not sure if this would also allow reimplementing the scheduler with > scheduled reports? In my understanding some jobs might not be triggered by > the GUI. OK That's true. I was focused on the forking scripts.
(In reply to Marcel de Rooy from comment #11) > (In reply to Katrin Fischer from comment #10) > > (In reply to Marcel de Rooy from comment #9) > > > Do we really need the background jobs ? Or should we refactor these scripts > > > in the direction of AJAX API calls in JavaScript ? > > > > I am not sure if this would also allow reimplementing the scheduler with > > scheduled reports? In my understanding some jobs might not be triggered by > > the GUI. > > OK That's true. I was focused on the forking scripts. IMHO, in either case we need a way to throttle concurrency and server loads. So a task queue is needed. My only doubt is if we should really model the task queue with a message queue. I tried the same Jonathan tried but using AnyEvent::Task and the results where similar, and it only required a unix socket to communicate client and server processes. My suggestion was to use zeromq. But lets see how this goes. Specially in the integration step. I agree with David that this of course would require a single RabbitMQ server so questions: Is there a concept of namespaces? can any Koha instance access other instance's messages?
(In reply to Tomás Cohen Arazi from comment #12) > I agree with David that this of course would require a single RabbitMQ > server so questions: Is there a concept of namespaces? can any Koha instance > access other instance's messages? I really would like you guys to apply, read and try the patches, read the code, then discuss :) There are a lot of FIXME and TODO. This one answers your question: 16 sub connect { 17 my ( $self ); 18 my $conn = Net::RabbitFoot->new()->load_xml_spec()->connect( 19 host => 'localhost', # TODO Move this to KOHA_CONF 20 port => 5672, 21 user => 'guest', 22 pass => 'guest', 23 vhost => '/', 24 ); 25 26 return $conn; 27 } So different vhost, user and pass can be used.
There is also: "Going further: 2. Provide a koha-worker script to consume the jobs" So one worker per instance.
(In reply to Jonathan Druart from comment #13) > (In reply to Tomás Cohen Arazi from comment #12) > > I agree with David that this of course would require a single RabbitMQ > > server so questions: Is there a concept of namespaces? can any Koha instance > > access other instance's messages? > > I really would like you guys to apply, read and try the patches, read the > code, then discuss :) > There are a lot of FIXME and TODO. > > This one answers your question: > > 16 sub connect { > 17 my ( $self ); > 18 my $conn = Net::RabbitFoot->new()->load_xml_spec()->connect( > 19 host => 'localhost', # TODO Move this to KOHA_CONF > 20 port => 5672, > 21 user => 'guest', > 22 pass => 'guest', > 23 vhost => '/', > 24 ); > 25 > 26 return $conn; > 27 } > > So different vhost, user and pass can be used. I haven't tried the patches but I have read your code, and that's why I've voiced my concerns about multi-tenancy. There wasn't enough information there to understand, so that's why I asked the questions. Your reply almost answers the question, but not quite, so now I'm just reading about RabbitMQ. If I understand correctly, each Koha instance would get its own RabbitMQ "vhost", and within each "vhost" there would be named queues (which at the moment are hard-coded but could be configurable I suppose). Cool. That removes my concerns about multi-tenancy. Thanks for that, Jonathan. (Maybe in your example you could use something like "vhost => '/kohademo'" or maybe include some code comments just so it's more obvious how it works.)
(In reply to Tomás Cohen Arazi from comment #12) > My only doubt is if we should really model the task queue with a message > queue. I tried the same Jonathan tried but using AnyEvent::Task and the > results where similar, and it only required a unix socket to communicate > client and server processes. My suggestion was to use zeromq. But lets see > how this goes. Specially in the integration step. Yeah, for #10662, I used POE::Component::JobQueue and a Unix socket. It was very effective, although my bespoke implementation had a number of limitations. I figured allowing TCP sockets also might be a good idea just to allow for more distributed architectures. When I was working on #10662, I was considering both RabbitMQ and ZeroMQ, but ended up doing my own job queue. I'm not necessarily sold on RabbitMQ yet, but I figure if Jonathan is willing to do it, I'm happy to see how this goes too. I'm not willing to put in the time to make it happen :/.
(In reply to David Cook from comment #15) > If I understand correctly, each Koha instance would get its own RabbitMQ > "vhost", and within each "vhost" there would be named queues (which at the > moment are hard-coded but could be configurable I suppose). I imaged one koha-worker script per Koha instance, with as well one vhost. The queues would be named 'batch_record_modification', 'manage-marc-import', 'batch_item_modification', etc.
TODO, remove that: Bug 15032: UpdateDB entry + # $dbh->do( "ALTER TABLE biblio ADD COLUMN badtaste int" ); Bug 22417: Add the ability to cancel a job + sleep(5); :)
I was thinking about this work this morning. I'm really busy at the moment, but still think this is a good thing to be working on.
I played a little with this patchset, and also with Minion. Some remarks: - Net::RabbitFoot says it's compatible with version 0-8 of the AMQP specification. RabbitMQ website says it implements version 0-9-1. Are we sure these two are compatible ? - Net::RabbitFoot is quite easy to package for Debian stretch/stable (all dependencies are already packaged - I did not try on oldstable) - If rabbitmq-server is not started, new_koha_job.pl and koha_worker.pl do not work. Is there a way to enqueue job when rabbitmq-server is not started ? - If koha_worker.pl is not started, and I enqueue jobs, and restart rabbitmq-server and then start koha_worker.pl, the worker does nothing. Potential loss of jobs here ? Now for the Minion part, - the mysql backend requires a recent version of Minion (which requires a more recent version of Mojolicious than the one packaged in debian.k-c.org). It might be complicated to package. - the sqlite backend however have some versions that can be packaged and can work with our version of Mojolicious (requires packaging Minion 6.04, Mojo::SQLite 2.002, and Minion::Backend::SQLite 0.009) - it works similarly to the rabbitmq code (a worker that run continuously, some code to enqueue jobs, where all jobs are stored in database) but doesn't require to have something (like rabbitmq-server) listening all the time to work. The worker just fetches queued jobs from its backend when it starts (and also regularly while it's running) - from what I have seen, it requires less code (see for example: https://mojolicious.io/blog/2018/12/10/minion-stands-alone/) - workers can be "paused" (meaning they won't accept new jobs, but active jobs will continue to run) I'm a little concerned about the potential loss of jobs with the RabbitMQ option. Is there any way to avoid this problem ?
(In reply to Julian Maurice from comment #20) > I'm a little concerned about the potential loss of jobs with the RabbitMQ > option. Is there any way to avoid this problem ? Using Monit to make sure the service is always up? :) But indeed we should return an error if the server has not been reached, and so the job not enqueued.
(In reply to Jonathan Druart from comment #21) > (In reply to Julian Maurice from comment #20) > > I'm a little concerned about the potential loss of jobs with the RabbitMQ > > option. Is there any way to avoid this problem ? > > Using Monit to make sure the service is always up? :) > > But indeed we should return an error if the server has not been reached, and > so the job not enqueued. That solves one problem, but not the other: when RabbitMQ already has enqueued jobs but stops before koha_worker.pl had the chance to process them.
But then the job will be in the DB table, so we will not lose it, right?
(In reply to Jonathan Druart from comment #23) > But then the job will be in the DB table, so we will not lose it, right? If we're worried about losing messages, why not look at RabbitMQ persistence? https://www.rabbitmq.com/persistence-conf.html
Thanks Jonathan for the great job done. I have been testing the development, it works correctly and it seems to me a very good idea to use RabbitMQ as a queue manager. As I had told you by email, we have to do a development focused on the reports, so that they can be executed regardless of AT, and also can be scheduled reports that require user parameters at the time of execution. Because the reports should be able to be programmed, the way to work with RabbitMQ would be through two possibilities: * Use version 3.5.3 or later of RabbitMQ. The one I have installed with Debian Jessie is 3.3.5. From version 3.5.3 you can use a plugin for delayed messages, so that these can be consumed in the future, and not immediately, which is how RabbitMQ works. I do not know how Net::RabbitFoot will behave in this case ... * Use an intermediate table (or the background_jobs table, with a new column called exec_on) for future messages, and have a producer (cronjob) to send the messages to RabbitMQ at the time they need to be consumed. If nobody raises objections to the use of RabbitMQ, I can investigate the above options, and make a POC for scheduled reports.
(In reply to Juan Romay Sieira from comment #25) > * Use version 3.5.3 or later of RabbitMQ. The one I have installed with > Debian Jessie is 3.3.5. From version 3.5.3 you can use a plugin for delayed > messages, so that these can be consumed in the future, and not immediately, > which is how RabbitMQ works. I do not know how Net::RabbitFoot will behave > in this case ... > This sounds like it's probably not an option due to the version issue. > * Use an intermediate table (or the background_jobs table, with a new column > called exec_on) for future messages, and have a producer (cronjob) to send > the messages to RabbitMQ at the time they need to be consumed. > If you're going to use a cronjob as a producer for future messages, maybe it's worthwhile to use a cronjob as a producer for all messages. That way there's just 1 method for enqueuing messages. My only reluctance to use a cronjob is that it relies on a system administrator setting it up properly, and it's challenging to know from the web interface if it has been set up correctly, so the user experience could suffer greatly. How would users know that the cronjob is running and their task will actually be run? As a result, it seems better to run a task scheduler service, but then that gets us back to using existing tools like Minion, Celery, etc. I suppose we could write our own task scheduler though. That reminds me of something that Fridolin said to me at Kohacon18 though. From a vendor/system administrator perspective, we don't necessarily want libraries to be in-charge of their own scheduling. For instance, say a vendor is running 30 Koha instances with a shared database server, and the librarians at each of the 30 Koha libraries all schedule a very intensive report to run at 7pm. Or say they schedule a task that does API calls, and then an upstream API server rejects all the calls after the 10th due to rate limiting by domain. I mention this because I already wrote my own task queue/task scheduler (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10662&attachment=85224), and Fridolin at BibLibre pointed out that since each instance ran their own task scheduler which could schedule a task for 7pm, they could cause these problems. I suppose in this case the timers would just be for the enqueuing of tasks. The dequeuing and execution would still be handled by RabbitMQ, and I suppose if you ran with a low number of workers you'd be less likely to overwhelm your systems. And if you needed better performance, you could add workers and add database servers for the report scenario. The rate limiting scenario is trickier though. I don't know how to solve that one. My only ideas for that involve complex home-baked solutions. I don't know the best practice for that one. Anyway, mostly just playing devil's advocate and trying to think of edge cases.
(In reply to David Cook from comment #26) > Anyway, mostly just playing devil's advocate and trying to think of edge > cases. I suppose the advantage of the cronjob is that system administrators would control the timing of it. I suppose there could be a warning on the page saying that the timing of tasks scheduled for the future would be up to the discretion of the system administrator? Lately, I've been wondering how we can modernize Koha, so that it can run in large distributed environments... but also on low budget single server systems. Being all things to all people...
Hello, Rabbit is pretty useful for Koha in my eyes. Now we can fight with some limitation because message_que is "one piece" If some message is in the que and waiting for send isn't possible delete just one or group of them. So fragmentation based on RabbitMQ can be great solution. But this bring many new possibillities and better services to borrowers and stuff. I vote for this implementation. May I help with testing or something else?
I won't be at the Marseille hackfest, but maybe some folk will think about working on this there next month?
So, getting back to this (one year after the previous try). Please read this comment, the different commit, then try the patchset. I have changed some stuffs, in order to make everybody (hopefully) happy. I removed the dependency to Net::RabbitFoot, in order to not depend on non-packaged stuffs, and I decided to take a slightly different direction. This patch can be considered ready for testing. I have investigated a bit what could be possible, and picked STOMP [1]: """ STOMP is the Simple (or Streaming) Text Orientated Messaging Protocol. STOMP provides an interoperable wire format so that STOMP clients can communicate with any STOMP message broker to provide easy and widespread messaging interoperability among many languages, platforms and brokers. """ There is an actively developed Perl library, Net::Stomp, packaged for debian. I think it could be a very good first step, as we could have a dependency on a small server at first, and sysop could then use more powerful/scalable/etc server depending on their needs. However I sticked to RabbitMQ for the server in my tests, and it worked great. Something else worth to be noted: STOMP does not provide a 'namespace' parameter that can be specified at connection time, unlike Net::RabbitFoot. But there is a 'destination' on send/receive (actually when subscribing), that seems to do the same job. There is a new patch set (see the same remote branch, previous one is pushed to old/20200225_bug_22417), with different follow-ups adding and fixing several things (better error handling , fix batch authority modification, etc.) Test plan: 0. Prerequisite: % apt install rabbitmq-server # server % rabbitmq-plugins enable rabbitmq_stomp # Enagle the STOMP plugin % sudo service rabbitmq-server status # Make sure the RabbitMQ server is running % sudo service rabbitmq-server start # start it if needed % apt install libnet-stomp-perl # install the client lib % cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh % cp debian/scripts/koha-worker /usr/bin/ % koha-worker --start kohadev # start the background jobs worker % tail -f /var/log/rabbitmq/* # take a look at the look to understand what's going on % Execute the updatedatabase entry to create the new DB table Then create, or make sure you have a MARC Modification Template, with template_id=1 (like add new field 123$z=foo) 1. To test this patch set you should first understand how the enqueue/consume process work: % perl new_koha_job.pl # Launch it 1+ times => The job must have been enqueued (no error in the output) % perl koha_worker.pl # Will consume the 1+ tasks you enqueued Do not close this worker and start another one in another console, then enqueue more new jobs. Use the following bash loop to enqueue lot of jobs: % for i in {1..100}; do perl new_koha_job.pl; done Enjoy :) (yes it should be fun) 2. Watch the history of the jobs Hit admin/background_jobs.pl (logged in as superlibrarian) This is an interface of the list of pending/running and finished processes 3. Use this whole stuff in a real Koha world: Play with the "Batch record modification" tool (with biblios or authorities) Notice the different in the last (report) step and click on the "View detail of the enqueued job" link Discussions: 1. Please grep TODO and FIXME in the patchset, there are lot to do/fix. 2. Even if we do not go for STOMP I think we could start moving the code from pl to Koha::BackgroundJob::*pm It will be "easy" (nothing is easy, right?) to replace it with something else. 3. With this base we will want to answer different needs: * background jobs * task scheduler * reindex process (?) Going further: 1. Implement Koha::BackgroundJob for the different background jobs 2. Rewrite the task scheduler with Koha::BackgroundJob (1993) 3. Provide a configuration file to prioritize or postpone jobs (for instance no batch modification on Wednesday between 9-17) - Or maybe we won't need that (?) 4. Server-side, there is the possibility to configure a lot of things, like users, destinations, ssl, etc. By default guest/guest has a localhost access only. Code is at https://gitlab.com/joubu/Koha/commits/bug_22417 [1] https://stomp.github.io/ https://stomp.github.io/stomp-specification-1.2.html
Going to try to make some time to try this out next week. For the OAI-PMH harvester, I'm going to want the user to be able to enqueue messages for starting/stopping downloading. Downloader workers will need to be able to enqueue messages for other downloader workers (for recurring tasks with slightly altered parameters each time). Downloader workers will also need to be able to enqueue messages for import workers. I'll think about those logistics later, but it should be doable with a few topics/queues in the message broker.
Hi, I played again, 1 year later, with this patchset and also with Minion. The STOMP && RabbitMQ solution looks very powerful, but I think that it is a little bit too much for our needs. I made a POC with Minion for comparison. https://gitlab.com/jajm/Koha/-/commits/bug/22417-minion There are 2 commits, there's a test plan in each commit message. About the packaging problems mentioned in comment 1 > I first tried Minion but found problematic issues for us (Koha): > - The mysql backend requide Mojo::mysql which requires DBD::mysql 4.042 > However Jessie has 4.028 and Stretch 4.041 This is still true, but buster is out for a while now and has a more recent version of DBD::mysql, so it should not be a problem to package Mojo::mysql and Minion::Backend::mysql. Older distributions could use the SQLite backend which works well too.
(In reply to Julian Maurice from comment #32) > Hi, > > I played again, 1 year later, with this patchset and also with Minion. > > The STOMP && RabbitMQ solution looks very powerful, but I think that it is a > little bit too much for our needs. > > I made a POC with Minion for comparison. > https://gitlab.com/jajm/Koha/-/commits/bug/22417-minion > There are 2 commits, there's a test plan in each commit message. So you need an additional DB. 2 tables to store the jobs (one for our data, to keep track of them, one internal for minion). I do not think STOMP is "very powerful", I picked it for its simplicity. Having the RabbitMQ option will let sysadmins decide the flexibility they want. Back to In Discussion then.
(In reply to Jonathan Druart from comment #33) > So you need an additional DB. 2 tables to store the jobs (one for our data, > to keep track of them, one internal for minion). What ? No. You can use the same database, and you do not have to keep track of jobs manually. Minion does that for you.
(In reply to Jonathan Druart from comment #33) > I do not think STOMP is "very powerful", I picked it for its simplicity. > Having the RabbitMQ option will let sysadmins decide the flexibility they > want. STOMP is very simple indeed. But combined with RabbitMQ it becomes a powerful solution for dispatching messages. And that's not a bad thing of course, but I think it adds a useless layer of complexity in our case.
(In reply to Julian Maurice from comment #34) > (In reply to Jonathan Druart from comment #33) > > So you need an additional DB. 2 tables to store the jobs (one for our data, > > to keep track of them, one internal for minion). > > What ? No. You can use the same database, and you do not have to keep track > of jobs manually. Minion does that for you. Does it allow additional data? Will you manage to write the dashboard I provided with my patches? When I did my different tries bug 15032 it was not possible (not Minion).
(In reply to Jonathan Druart from comment #36) > Does it allow additional data? Yes, there is a 'note' field for this purpose https://mojolicious.org/perldoc/Minion/Job#note > Will you manage to write the dashboard I provided with my patches? > When I did my different tries bug 15032 it was not possible (not > Minion). Here are some screenshots of the admin UI packaged with Minion: https://snipboard.io/n0O6lf.jpg https://snipboard.io/QNJrLC.jpg https://snipboard.io/tiq5dI.jpg
(In reply to Julian Maurice from comment #35) > STOMP is very simple indeed. But combined with RabbitMQ it becomes a > powerful solution for dispatching messages. And that's not a bad thing of > course, but I think it adds a useless layer of complexity in our case. I am willing to try both Minion and RabbitMQ (and I keep telling myself I will try them soon in my personal time since I'm staying home due to the pandemic). But I want to say first that I don't think we'd be adding a useless layer of complexity with RabbitMQ. Provided the implementation is well-architected, I think message queues are an integral part of scaling up an application and improving interoperability. Using a language agnostic protocol like STOMP, it could be easier to implement higher performance workers in other languages. For instance, maybe we'd want to implement some workers in Golang, which makes concurrent programming very easy with its implementation of user threads as goroutines balanced against kernel threads depending on resource demands. For my work on a OAI-PMH harvester, I have been thinking of rewriting it in something like Golang instead of Perl, and then hooking into Koha using a standard message queue (instead of my bespoke POE-based implementation) and/or APIs. (Honestly though my Perl version is quite high performance in terms of downloads but the bottleneck is importing into Koha. That's still slower than I'd like. I occasionally think about how to optimally implement a scalable high performance MARCXML record importer. But that's a whole other topic really.) That said, RabbitMQ is a new dependency, and I think our collective fear of new dependencies has kept us from implementing a message queue previously. I think we've arrived at a point where we *have* to have some kind of job/message queue though. Anyway, sorry for the ramble. It's just a topic in which I have a strong interest. I'm going to put it at the top of my personal to try pile.
(As a side note, I am looking at packaging (as RPMs) newer versions of DBD::Pg and DBI to provide access to new PostgreSQL versions for older OSes for a different project, and it doesn't seem too challenging.)
Ok I'm finally trying this out! (I should be Netflixing or chilling or something, but I am really keen to give this a go, especially after a day of talking about message queues for other projects. Plus I'm always saying we need to modernize, so I really want to help out Jonathan with all his efforts on this one, and show that it's not for nothing.) My steps: 0a. cd ~/git/koha-testing-docker/ 0b. sh run.sh (my own script to for invoking docker-compose with a custom file) 1. winpty docker exec -it koha_koha_1 bash 2. cd koha 3. git remote add joubu https://gitlab.com/joubu/Koha.git 4. git fetch joubu 5. git checkout -b rabbitmq joubu/bug_22417 #The following could probably be handled using https://hub.docker.com/_/rabbitmq 6. apt install rabbitmq-server 7. rabbitmq-plugins enable rabbitmq_stomp 8. service rabbitmq-server start 9. service rabbitmq-server status 10. apt install libnet-stomp-perl 11. cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh 12. cp debian/scripts/koha-worker /usr/bin/ 13. koha-worker --start kohadev 14. koha-shell kohadev -c installer/data/mysql/updatedatabase.pl 15. restart_all 16. Log into http://localhost:8081/ 17. Go to Tools > MARC modification templates 18. Make new template 18b. Name: test 19. Make new action for "test" template 19b. Add new field "856" "u" "https://koha-community.org" 19c. Description: "RabbitMQ test" 20. koha-shell kohadev 21. perl new_koha_job.pl Output: [x] Enqueuing BatchUpdateBiblio mmtid=1 with biblionumber=1,2,3 22. exit 23. Go to http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl 24. Confirm that the job has completed 25. Go to http://localhost:8081/cgi-bin/koha/tools/batch_record_modification.pl 26. Enter list of record numbers: 1, 2, 3, 4, 5 27. Select "test" template 28. Click "Continue" 29. Click "Modify selected records" 30. See message "The job has been enqueued! It will be processed as soon as possible (FIXME - well, it could depend on a config?)" 31. Click "View detail of the enqueued job" 32. Confirm job has status of "finished" 33. Click "Return to the job list" (or "New batch record modification") Great work, Jonathan! The test works beautifully. I'll take a peek at the code now to look for some of those FIXME and TODO messages you mention.
git diff ff4c85109f043c4086c9348e5e88638cc5a7ebd5~1..HEAD TODOs: 1. Agree raising exceptions is better than returning null 2. A new "manage_background_jobs" permission might make sense for managing supervisor permissions, but I figure whoever enqueued the job should be able to check on its progress, so this might not make sense. We should manage background job permissions based on their other permissions, although that gets tricky. I suppose we could pass the background job type, validate it against a list (and translate it to an existing permission), and then put it into flagsrequired. If there is no background job type passed, then we assume it's all, and then we could use "manage_background_jobs" as the permission which would allow a supervisor/superlibrarian to view all jobs at once? Alternatively, admin/background_jobs.tt is for supervisors/superlibrarians only, and each tool implements its own interface for tracking tasks. That might be necessary in any case, as the existing functionality isn't very librarian friendly. While "Background Jobs" make sense to developers, I don't think librarians will necessarily understand the concept, nor should they need to. FIXMEs: Too many to comment on right now, plus would be easier to comment on if you uploaded a patchset to Bugzilla. I think we might have too much specific/tightly-coupled logic in Koha/BackgroundJob.pm. I notice too there is some "enqueue" copy/paste code between Koha/BackgroundJob/BatchUpdateBiblio.pm and Koha/BackgroundJob/BatchUpdateAuthority.pm, which could be put into a base/parent class. I think it would make sense to put the "process" code into something like "Koha/Worker/BatchUpdateBiblio.pm", and to use a "Koha/Worker/Base.pm" type class to handle the messaging aspect for the workers. I could provide more feedback but it is late and I need to get up early.
I am tempted to try re-engineering https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662 using STOMP and RabbitMQ, as I would really like to use a standard message queue rather than my bespoke implementation. I would also be interested in using plugins to provide a Koha CRUD web interface for managing STOMP messages for specific types of tasks. For instance, I could provide a plugin for sending add/start/stop/update/remove messages to an OAI-PMH harvester. As the sysadmin, I'd need to run up the harvester in the background first, but that would be OK. It would be more so for a proof of concept than anything at first. Anyway definitely bed time now...
I am keen to get Koha using RabbitMQ, so I'm wondering a bit about the shortest path for us to do that. I am tempted to start making Koha plugins that use RabbitMQ as an experiment. That might make it easier to decide on a tried and tested way to proceed with master Koha code. Plus, plugins are well suited to batch/long-running work that RabbitMQ would help with. (Honestly, we have some local customizations that I want to change to use RabbitMQ anyway, so I think that I'm heading in that direction with or without the community anyway, but I'd like to share that work and knowledge.) Why not add Net::Stomp as an optional dependency for Koha? And then maybe make some experimental features that can be switched on using system preferences? (That is to say, it might be better to experiment for a bit, rather than trying to completely replace existing components.)
I've also been thinking about the separate needs for a message queue and a task scheduler. For instance, "Bug 1993 - Task Scheduler Needs Re-write" and an OAI-PMH harvester would both need a task scheduler to make sure tasks run at schedule times. However, a Staged MARC Import doesn't need to be scheduled. It just needs to be run ASAP in the background. (Same case for batch modifications, label creation, and many other long-running tasks.) That being said, for scheduled reports (like in "Bug 1993 - Task Scheduler Needs Re-write"), the scheduled task itself might be to enqueue a message into the message queue. (For instance, let's say 20 librarians schedule their SQL reports to run at 6pm. At 6pm, those 20 SQL reports should be put into a queue, and then 1+ report workers can do the work and record the results. They then queue an email job, and emails go out with the results.) (Arguably email processing could be a scheduled task, as you may or may not want emails going out from the library in the middle of the night.)
Thinking out my plugin idea for Bug 10662 made me think how useful it would be to have RabbitMQ support built into Koha (via STOMP), but it made me wonder how you'd provide the workers in the plugin. I suppose a person could just bundle it in with the plugin code, but the plugin code is "web" code. Might not matter depending on how the web server is set up I suppose.
Clap Clap Clap
I'm impressed by the results, Jonathan. I really sorry I didn't spend time on this before. It deserved it for sure. I will probably repeat myself, but in my opinion what we need is: - A task_queue table - A daemon that looks for things to do, and spawns children processes to do the tasks, mark them as completed or failed, handle retries, etc That's what basically Minion does. This implementation: - Sets a message broker - Has a table to keep track of tasks - Sends a message for each task that is enqueued - Has a daemon that looks for those messages and runs the task What we are doing is modelling a task queue with a message queue, from which we use (probably) its scheduling features, and the 'retry' logic perhaps? I'm not in a situation in which I can provide an alternative right now, so I wouldn't want to block this right now. So will try to add my constructive two cents: - I would like to know what ideas you might have for maintaining the 'tasks' catalog, specially when it comes to plugins (i.e. the mapping between background_job.type and the method that has to be run. - I expected to see an event-driven implementation of the koha_worker.pl loop, as messages will arrive and we should react to them instead of polling. Maybe it is a limitation from Net::STOMP, I saw there's AnyEvent::STOMP which is event-driven. Maybe the 'blocking and waiting for a frame' behaviour from Net::STOMP::receive_frame works similarly... worth checking. - I would like to see OO in Koha::BackgroundJob but this is minor abviously I will organise my work so I can spend some hours on redoing this using AnyEvent::Task::Server and AnyEvent::Task::Client as I wrote a POC a lot of time ago. GOOD JOB Jonathan, it is obvious you spent lots of time on this and you put the bar very high.
(In reply to Tomás Cohen Arazi from comment #47) > I'm impressed by the results, Jonathan. I really sorry I didn't spend time > on this before. It deserved it for sure. > Ditto. I would love to see more interest in this one, as it could be such a valuable addition to Koha. I don't know why more people don't seem interested. Not sure if they're just uninterested because they're not familiar with the technologies. > I will probably repeat myself, but in my opinion what we need is: > > - A task_queue table > - A daemon that looks for things to do, and spawns children processes to do > the tasks, mark them as completed or failed, handle retries, etc > This is similar to the model that I've used in the past (for Bug 10662 and some of my fun projects), and it has pros and cons. Personally, I am interested in a model where we create a task scheduler daemon that Koha connects to via a HTTP API, and the task scheduler daemon publishes to the message broker when it's time to run a task. It has pros and cons too: FORKING: - One pro is that it closely couples the task scheduler to Koha, which makes it very easy to *do things*. - One con is that it can only use 1 machine's resources, which reduces its availability and scalability. It also means you're probably running 1 task scheduler per Koha instance, which could increase management/maintenance complexity. - One pro is that it's very easy to increase/decrease limits on forking child processes. - One pro is that it would be easier to use plugins to do background jobs, as the task scheduler would just need to load the code. WORKERS: - One pro is that it loosely coupled the task scheduler to Koha, which means it is a discrete modular unit of code that can be developed and tested independently of the rest of the Koha codebase. - One con is that being loosely coupled makes it harder to do things. It means using more APIs instead of giving direct database access and internal (ie Perl) API access to workers. - One pro is that a sysadmin has a lot of control for increasing/decreasing the number of worker processes, and where they run. (This lends itself to containerization.) - One pro is that it means workers can be written in any programming language. Tasks which require high efficiency could have workers written in a more high performing language. - One con is that it increases complexity. Managing worker processes is harder than forking new child processes. Supervisord could be useful for this though. This is a solved problem. - One con is that plugins wouldn't be able to introduce worker processes. (Slowly, in my free time, I'm developing a Koha plugin which will connect to a Golang-based task scheduler using a HTTP API which runs OAI-PMH harvest jobs using goroutines. The downside is that a sysadmin would have to manage the Golang task scheduler. It wouldn't work just out of the box after loading the Koha plugin.)
(In reply to Tomás Cohen Arazi from comment #47) > - I would like to know what ideas you might have for maintaining the 'tasks' > catalog, specially when it comes to plugins (i.e. the mapping between > background_job.type and the method that has to be run. I am really interested in this too. > - I expected to see an event-driven implementation of the koha_worker.pl > loop, as messages will arrive and we should react to them instead of > polling. Maybe it is a limitation from Net::STOMP, I saw there's > AnyEvent::STOMP which is event-driven. Maybe the 'blocking and waiting for a > frame' behaviour from Net::STOMP::receive_frame works similarly... worth > checking. As you say, "blocking and waiting for a frame" is event-driven behaviour, so koha_worker.pl is already event-driven. Net::STOMP and AnyEvent::STOMP both use a select-based event loop. (AnyEvent can use other event loops apparently, but you have to load them. Plus the performance gained using a more sophisticated event loop is likely marginal for our use cases.) > I will organise my work so I can spend some hours on redoing this using > AnyEvent::Task::Server and AnyEvent::Task::Client as I wrote a POC a lot of > time ago. > That looks interesting! Can I suggest that you use a TCP socket rather than a Unix socket? For my POE-based task scheduler in Bug 10662 (which was similar to this but included timers for task scheduling) I used a Unix socket and it worked fine, but it meant my scheduler has to be on the same machine; in hindsight, I wish I had used a TCP socket. With a TCP socket, it would make Dockerization easier and could make production deployments easier (depending on how the workers are done). Actually, how do you see AnyEvent::Task fitting in with RabbitMQ or task scheduling? The AnyEvent::Task::Server would sit in the background... would your koha_worker.pl be the RabbitMQ consumer and AnyEvent::Task::Client?
We use RabbitMQ for some other projects, and I think I'm going to start looking at using it with some of our local Koha extensions. I don't have a timeline. It's mostly just going to be when I have some free time, but happy to report back on how it goes, lessons learned, etc. Tomas, I'm also interested in using RabbitMQ with Koha plugins. I think I agree now with the general model that you suggested. My koha-plugin-oaipmh-import plugin would be able to make use of RabbitMQ, so I'd be keen to test it out on the plugin side. I suppose the minimum we'd need in Koha is the following: 1) A RabbitMQ server 2) A background Koha worker (which can invoke Koha plugins) The plugin worker could write results to its own result store (ie a plugin controlled database table). A core (ie non plugin) result store is probably a good idea but not an immediate requirement I would think...
(In reply to David Cook from comment #50) > We use RabbitMQ for some other projects, and I think I'm going to start > looking at using it with some of our local Koha extensions. > With my own code, I've added a local tool to tools-home.pl, and I have that add a JSON message to RabbitMQ with Net::Stomp. I have a background worker using Net::Stomp to get the JSON message, do the long-running task, and then email a user with the result. It does the bare minimum of what I need it to do, and I'll be looking to roll it out soon as an experimental feature to select Koha instances. Both the web app and daemon worker rely on configuration in koha-conf.xml to know not only the RabbitMQ server address, username, and password, but also what queues to use. (I rather use a YAML file for this, but koha-conf.xml already has caching support, so it's so easy to use.) Things to think about: - Installing RabbitMQ and plugins are easy, but you need enable the stomp plugin and create virtual hosts as the RabbitMQ administrator... which is only possible if they're running on the same machine - Technically, all Koha instances could use the root vhost, and we could prefix the queue names. (I'm going to be running into this issue when I roll out my experimental work more widely.) - Flow control for the background worker. Should it do 1 task at a time, or should it fork off workers? If it forks workers, you can easily exhaust your system's resources if you don't have any artificial limits, but you can also do more work. (Alternatively, we could create the facility to run X background workers, so sysadmins would determine the limits.) - How best to store the results of background tasks? Should it be in 1 central store, or should it be more task specific? Most of those are sysadmin type questions except for the last one. For the last one, I'm thinking task specific might be best, especially when it comes to background tasks done by Koha Plugins.
Oh and in the JSON message I specify "class" and "method" (much like the Koha::Plugin::Handler). It also provides "args". So it loads the "class", and runs the "method" with the "args". It's pretty simple. If I were to record results, I'd probably have a "result_callback_method" or something too.
Created attachment 105542 [details] [Prosentient] Create experimental support for RabbitMQ-based background tasks This patch includes a koha-mq-scheduler service which creates a mq_scheduler.pl daemon, which listens to RabbitMQ queues. This patch also includes an audit-authorities.pl web script, which sends a message to RabbitMQ. The mq_scheduler.pl daemon loads Koha::Prosentient::MQ::AuditAuthorities.pm, and runs a class method which does a background task to audit bib-auth linkages. Koha community work: https://bugs.koha-community.org/bugzilla3//show_bug.cgi?id=22417
Created attachment 105543 [details] [review] [Local Prosentient] Create experimental support for RabbitMQ-based background tasks This patch includes a koha-mq-scheduler service which creates a mq_scheduler.pl daemon, which listens to RabbitMQ queues. This patch also includes an audit-authorities.pl web script, which sends a message to RabbitMQ. The mq_scheduler.pl daemon loads Koha::Prosentient::MQ::AuditAuthorities.pm, and runs a class method which does a background task to audit bib-auth linkages. Koha community work: https://bugs.koha-community.org/bugzilla3//show_bug.cgi?id=22417
At Tomas's request, I've attached a patch I've done for a local customization to use RabbitMQ. It's very simple. I don't have any time-based "task scheduling" per se. It's just FIFO. The mq_scheduler.pl script is problematic in a number of ways, but it's just an experiment. Early testing is good. You submit a form on the Koha web UI, it sends a message to RabbitMQ, the mq_scheduler.pl receives the message from RabbitMQ, the mq_scheduler.pl loads a module and runs some code (which in this case also sends an email), and then it marks the message complete/acknowledged in RabbitMQ. This patch does require a person to run "add_vhost" and "set_permissions" manually using rabbitmqctl on the host running RabbitMQ. However, as I'm defining the queues in koha-conf.xml, the root vhost could be used, and the queues could be prefixed with __DB_NAME__. At this point, I'm fairly indifferent to the method. I like vhosts for the logical separation of the messages, but if it's not practical for a package installation, then maybe we should just use the prefixed queues. The systemd service is *very* package installation friendly. We'd need to update the debian/* files accordingly, but it would be so easy. I haven't done that yet, as I'm testing on non-package installations first.
Can we dedicate this bug report to the base idea, then create bug report on top of it for experiment? I'd like to avoid noise for testers and keep the comment list easy to follow.
(In reply to Jonathan Druart from comment #56) > Can we dedicate this bug report to the base idea, then create bug report on > top of it for experiment? I'd like to avoid noise for testers and keep the > comment list easy to follow. Sounds like a plan, boss man [U+1F44D]
Remote branch rebased against master, and adjusted.
(In reply to Jonathan Druart from comment #56) > Can we dedicate this bug report to the base idea, then create bug report on > top of it for experiment? I'd like to avoid noise for testers and keep the > comment list easy to follow. I agree with that. As discussed by other means, my only concerns are: - should we make it opt-in at this stage (i.e. keep the current scripts, but have a config/syspref that defines if the rabbitmq + daemon should be used). As it is packaged in the supported distros/versions it is not *that* important but worth discussing. - I don't agree with pulling the rabbitmq-server package automatically on a Debian install. We don't do that with mysql-server/mariadb-server and we should follow that approach. i.e. on the install notes, there will be an entry, before installing koha-common, which will say 'RabbitMQ' and will explain that if you already have one running, you point to it when calling koha-create (bug 25674) and otherwise what command to run for installing it.
(In reply to Tomás Cohen Arazi from comment #59) > (In reply to Jonathan Druart from comment #56) > > Can we dedicate this bug report to the base idea, then create bug report on > > top of it for experiment? I'd like to avoid noise for testers and keep the > > comment list easy to follow. > > I agree with that. > > As discussed by other means, my only concerns are: > - should we make it opt-in at this stage (i.e. keep the current scripts, but > have a config/syspref that defines if the rabbitmq + daemon should be used). > As it is packaged in the supported distros/versions it is not *that* > important but worth discussing. We could, but really I don't think it's worth the effort. We will have to maintain 2 different codes that do the same thing, and will slow down for sure the move for other scripts. > - I don't agree with pulling the rabbitmq-server package automatically on a > Debian install. We don't do that with mysql-server/mariadb-server and we > should follow that approach. i.e. on the install notes, there will be an > entry, before installing koha-common, which will say 'RabbitMQ' and will > explain that if you already have one running, you point to it when calling > koha-create (bug 25674) and otherwise what command to run for installing it. Yes sure, I agree. I will implement that as soon as this is PQA.
(In reply to Tomás Cohen Arazi from comment #59) > As discussed by other means, my only concerns are: > - should we make it opt-in at this stage (i.e. keep the current scripts, but > have a config/syspref that defines if the rabbitmq + daemon should be used). > As it is packaged in the supported distros/versions it is not *that* > important but worth discussing. With my local experiments, I've made it opt-in, so that I can manually switch over and evaluate whether my approach to the message queue is working well enough. I check to see if the "Net::Stomp" module is available and if the koha-conf.xml file has the configuration I need. That works well. > - I don't agree with pulling the rabbitmq-server package automatically on a > Debian install. We don't do that with mysql-server/mariadb-server and we > should follow that approach. i.e. on the install notes, there will be an > entry, before installing koha-common, which will say 'RabbitMQ' and will > explain that if you already have one running, you point to it when calling > koha-create (bug 25674) and otherwise what command to run for installing it. I agree. While we've decided to run rabbitmq-server locally on each server, it is a reasonable choice to use external RabbitMQ servers.
(In reply to Jonathan Druart from comment #60) > Yes sure, I agree. I will implement that as soon as this is PQA. Could you squash your commits and then post a patch here? I have directly fetched from your Gitlab during testing, but it does make testing and patch evaluation more difficult.
(In reply to Jonathan Druart from comment #60) > Yes sure, I agree. I will implement that as soon as this is PQA. The latest work still seems like work-in-progress to me? - Connection details and credentials are still hard-coded into Koha::BackgroundJob, and there is no way of passing in a virtual host (which we might not use out of the box, but it would be good to build in the support) - For instance, koha_worker.pl should be moved to "./misc/bin/koha_worker.pl". - I was thinking the "namespace" for the queues should use the database name instead, since non-Debian installs may or may not have correctly set memcached_namespace. - I don't think there's any service to start up koha_worker.pl? - I'm curious why koha_worker.pl doesn't use the /queue prefix (as suggested by https://www.rabbitmq.com/stomp.html). I think it works without it, as I have tested your patches, but I'm curious. - Could we use a "/" instead of a "-" for the queue destination. I think that would be more conventional. - Koha::BackgroundJob loads C4::MarcModificationTemplates and C4::Biblio but doesn't use them. I'd really like to see this code be plugin friendly from Day 1, which I think could be done with some updates to koha_worker.pl and Koha::BackgroundJob. At the moment, koha_worker.pl and Koha::BackgroundJob are hard-coded with 2 job_types. It wouldn't be difficult to move the job_types somewhere more configurable.
(In reply to David Cook from comment #62) > (In reply to Jonathan Druart from comment #60) > > Yes sure, I agree. I will implement that as soon as this is PQA. > > Could you squash your commits and then post a patch here? > If you squash your commits and post a patch here, I'd be happy to collaborate on this work. As you know, I've already got a similar implementation running locally for 1 Koha instance, and I am very keen to move forward with a community implementation instead. I also have a Koha Plugin in development which I would love to use with a message queue. Between Jonathan, Tomas, and David, I'm sure we can make this happen soon.
(In reply to David Cook from comment #63) > (In reply to Jonathan Druart from comment #60) > > Yes sure, I agree. I will implement that as soon as this is PQA. > > The latest work still seems like work-in-progress to me? Yes and no. It's ready for testing. I will address QA comment once this will hit the QA process. We will certainly deal with other additions on separate bug reports. > - Connection details and credentials are still hard-coded into > Koha::BackgroundJob, and there is no way of passing in a virtual host (which > we might not use out of the box, but it would be good to build in the > support) > - For instance, koha_worker.pl should be moved to > "./misc/bin/koha_worker.pl". It's in misc/background_jobs_worker.pl the koha_worker.pl script is part of a "DO NOT PUSH" patch and is there to understand what's going on. Please read the test plan from comment 30. > - I was thinking the "namespace" for the queues should use the database name > instead, since non-Debian installs may or may not have correctly set > memcached_namespace. Yes, it's in the comment: # This namespace is wrong, it must be a vhost instead. # But to do so it needs to be created on the server => much more work when a new Koha instance is created. # Also, here we just want the Koha instance's name, but it's not in the config... # Picking a random id (memcached_namespace) from the config That will be addressed on bug 25674. > - I don't think there's any service to start up koha_worker.pl? There is a patch titled "Add debian script koha-worker". > - I'm curious why koha_worker.pl doesn't use the /queue prefix (as suggested > by https://www.rabbitmq.com/stomp.html). I think it works without it, as I > have tested your patches, but I'm curious. It's also in Net::Stomp's POD, so I guess it's a good pattern to follow. I will submit a follow-up. > - Could we use a "/" instead of a "-" for the queue destination. I think > that would be more conventional. Agreed. > - Koha::BackgroundJob loads C4::MarcModificationTemplates and C4::Biblio but > doesn't use them. Yes, leftover. Thanks! > I'd really like to see this code be plugin friendly from Day 1, which I > think could be done with some updates to koha_worker.pl and > Koha::BackgroundJob. At the moment, koha_worker.pl and Koha::BackgroundJob > are hard-coded with 2 job_types. It wouldn't be difficult to move the > job_types somewhere more configurable. It won't from Day 1. I really need a sign from the community that we agree on a first move. Make it more complex now is not a good idea. Once we agreed on that I promise to move all the different background jobs we have in a short term. That will show us some pattern that is repeated and must be refactored. For instance when I rebased yesterday I had to fix a conflict with bug 18127 (see patch "Restore the 'add to list' feature"). It highlights an interesting feature that must be implemented: a post-process hook to display something specific to the job's report. I hard-coded it for now, but that would be something great to implement in the next steps, especially for the plugins. (In reply to David Cook from comment #64) > (In reply to David Cook from comment #62) > > (In reply to Jonathan Druart from comment #60) > > > Yes sure, I agree. I will implement that as soon as this is PQA. > > > > Could you squash your commits and then post a patch here? > > > > If you squash your commits and post a patch here, I'd be happy to > collaborate on this work. I would prefer to keep the history. Checking out a remote branch should not prevent you to collaborate, it's even more easier than attaching the patches from here :)
Branch updated: - 2 use statements removed (commit amended) - New commit to send/receive message to/from /queue Note that /queue/$namespace/$job_type does not work, I kept the dash: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue destination
I would like my signature to be added to this work. I think there are a couple areas we need to work on: - Tasks classes organization (I don't really like them much) - Plugins: how they fit here Those two can be dealt with later, and I'm sure it is better to have this pushed ASAP to address any integration issues sooner than later.
(In reply to Tomás Cohen Arazi from comment #67) > I'm sure it is better to have this > pushed ASAP to address any integration issues sooner than later. Sounds good enough to me. I think that the most important thing is to get the Net::Stomp dependency added, and a configuration mechanism so that Koha code can get connection details for a RabbitMQ instance. With that in place, we should be able to refine and polish the overall task queue feature incrementally. As you say, Tomas, the sooner we get this merged into Koha, the better. (By extension, I'm already locally using RabbitMQ with Koha, but I'm using a custom block in koha-conf.xml and custom update to debian/control to do that on the Koha side. It would be great for those to be provided by upstream. That would free me up to focus on my custom RabbitMQ consumer and my custom Koha producer code. Once the task queue feature is optimized, I could replace most of that custom work too, other than whatever "driver" is being used in the Koha RabbitMQ consumer.)
Created attachment 107385 [details] [review] Bug 22417: UpdateDB entry TODO: - Add the FK on borrowernumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107386 [details] [review] Bug 22417: DBIX Schema changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107387 [details] [review] Bug 22417: Add Koha::BackgroundJob[s] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107388 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateBiblio Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107389 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateAuthority This is not ready yet! Note that this is too close to Koha::BackgroundJob::BatchUpdateBiblio, we will want to refactor bit of code. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107390 [details] [review] Bug 22417: [DO NOT PUSH] Add simple worker and add_job scripts Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107391 [details] [review] Bug 22417: Add GUI to watch the progress of jobs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107392 [details] [review] Bug 22417: Adapt the batch_record_modification tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107393 [details] [review] Bug 22417: Add the ability to cancel a job Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107394 [details] [review] Bug 22417: Fix borrowernumber values Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107395 [details] [review] Bug 22417: Add debian script koha-worker Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107396 [details] [review] Bug 22417: Switch to STOMP apt install rabbitmq-server service rabbitmq-server start rabbitmq-plugins enable rabbitmq_stomp apt install libnet-stomp-perl cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh cp debian/scripts/koha-worker /usr/bin/ koha-worker --start kohadev tail -f /var/log/rabbitmq/* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107397 [details] [review] Bug 22417: Handle errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107398 [details] [review] Bug 22417: Try to display pending jobs - only work if worker not started ie. no subscription yet This needs to be fixed (if possible), or removed from the patchset Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107399 [details] [review] Bug 22417: Fix the batch authority tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107400 [details] [review] Bug 22417: Restore the concept of namespace Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107401 [details] [review] Bug 22417: [DO NOT PUSH] Add sleep to notice the progress Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107402 [details] [review] Bug 22417: Add a note about the existence of the DB row Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107403 [details] [review] Bug 22417: [DO NOT PUSH] Adjust koha_worker.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107404 [details] [review] Bug 22417: Restore the 'add to list' feature This feature has been added recently by bug 18127. It highlights the need of a post processing hook. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107405 [details] [review] Bug 22417: Add the new permission manage_background_jobs QA: Please answer the question in admin/background_jobs.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 107406 [details] [review] Bug 22417: Send message to /queue This cannot be: /queue/$namespace/$job_type I got: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue destination So keeping the dash in $namespace-$job_type Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Jonathan Druart from comment #90) > Created attachment 107406 [details] [review] [review] > Bug 22417: Send message to /queue > > This cannot be: > /queue/$namespace/$job_type > > I got: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue > destination > > So keeping the dash in $namespace-$job_type > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> This might be explained by https://www.rabbitmq.com/stomp.html#d. Looks like a person could possibly use '/koha_kohadev%ddbatch_biblio_record_modification', but "$namespace-$job_type" might be easier to read for developers.
(In reply to Jonathan Druart from comment #89) > Created attachment 107405 [details] [review] [review] > Bug 22417: Add the new permission manage_background_jobs > > QA: Please answer the question in admin/background_jobs.pl > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I can't 100% recall how background jobs are implemented in the web UI, but I find admin/background_jobs.pl to be a challenging topic. In theory, any module (eg circ, acquisitions, etc) could use a background job, so shouldn't the UI be in the source module rather than in the admin module? (If that already is the case and I'm mistaken, then maybe just leave this script with "manage_background_jobs".)
Created attachment 107481 [details] [review] Bug 22417: Remove record_type from BatchUpdate* We don't need to pass it to the job, they each know which record type they are dealing with
Created attachment 107482 [details] [review] Bug 22417: (follow-up) Fix the batch authority tool
(In reply to David Cook from comment #92) > (In reply to Jonathan Druart from comment #89) > > Created attachment 107405 [details] [review] [review] [review] > > Bug 22417: Add the new permission manage_background_jobs > > > > QA: Please answer the question in admin/background_jobs.pl > > > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> > > I can't 100% recall how background jobs are implemented in the web UI, but I > find admin/background_jobs.pl to be a challenging topic. > > In theory, any module (eg circ, acquisitions, etc) could use a background > job, so shouldn't the UI be in the source module rather than in the admin > module? (If that already is the case and I'm mistaken, then maybe just leave > this script with "manage_background_jobs".) What is the "source module"? I think that a user with manage_background_jobs should be able to access and manage (cancel, view, etc.) any jobs (done) However anyone who started a job should be allowed to manage this job (not done yet), but should we force a minimum permission or authentication is enough? If we agree on that it can be implemented easily.
(In reply to Jonathan Druart from comment #95) > What is the "source module"? > Oh I mean the module where the source of the call is made. So in this case, "Staged MARC for Import" would be in the Tools module. But in the Circulation module you could upload an Offline Circulation module, and queue up a job to do that import in the background. It seems to me that the person should be able to review their job within the module that they made it. > I think that a user with manage_background_jobs should be able to access and > manage (cancel, view, etc.) any jobs (done) > However anyone who started a job should be allowed to manage this job (not > done yet), but should we force a minimum permission or authentication is > enough? > > If we agree on that it can be implemented easily. I'm not sure I understand 100% what you're saying. I think that I agree that a high level administrator should be able to manage all/any jobs. So manage_background_jobs.pl probably should have a particular/specific admin permission I would think.
I'm finding the number of patches to be a bit intimidating, so I made this little git diff of changed files: Koha/BackgroundJob.pm Koha/BackgroundJob/BatchUpdateAuthority.pm Koha/BackgroundJob/BatchUpdateBiblio.pm Koha/BackgroundJobs.pm Koha/Schema/Result/BackgroundJob.pm admin/background_jobs.pl debian/docs/koha-common.xml debian/koha-common.init debian/scripts/koha-create debian/scripts/koha-disable debian/scripts/koha-functions.sh debian/scripts/koha-remove debian/scripts/koha-worker installer/data/mysql/atomicupdate/bug_15032.perl installer/data/mysql/kohastructure.sql installer/data/mysql/userpermissions.sql koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt koha_worker.pl misc/background_jobs_worker.pl new_koha_job.pl tools/batch_record_modification.pl From a staff interface perspective, it looks like the main things to test are admin/background_jobs.pl and tools/batch_record_modification.pl? From a sysadmin perspective, it looks like it would be a good idea to test /etc/init.d/koha-common, koha-create, koha-disable, koha-remove, and koha-worker. I'm not seeing any usage for "koha_worker.pl" or "new_koha_job.pl". Are these dev scripts that should be removed?
Can we change $namespace in Koha/BackgroundJob.pm and misc/background_jobs_worker.pl from "memcached_namespace" to "database"? I think this is a more robust choice for Koha instances that have been upgraded over time (since there is no guarantee that they even have "memcached_namespace" even set in their koha-conf.xml). Happy for this to be done in a separate bug report. I figure we really should get #22417 into master ASAP, and we can improve it with other bug reports?
(In reply to David Cook from comment #96) > (In reply to Jonathan Druart from comment #95) > > I think that a user with manage_background_jobs should be able to access and > > manage (cancel, view, etc.) any jobs (done) > > However anyone who started a job should be allowed to manage this job (not > > done yet), but should we force a minimum permission or authentication is > > enough? > > > > If we agree on that it can be implemented easily. > > I'm not sure I understand 100% what you're saying. I think that I agree that > a high level administrator should be able to manage all/any jobs. So > manage_background_jobs.pl probably should have a particular/specific admin > permission I would think. That's how it works already, there is a manage_background_jobs permission. The question is about a "normal" user (without that permission) who enqueued a new job. (In reply to David Cook from comment #97) > I'm finding the number of patches to be a bit intimidating, so I made this > little git diff of changed files: Then squash them locally :) > From a staff interface perspective, it looks like the main things to test > are admin/background_jobs.pl and tools/batch_record_modification.pl? > > From a sysadmin perspective, it looks like it would be a good idea to test > /etc/init.d/koha-common, koha-create, koha-disable, koha-remove, and > koha-worker. > > I'm not seeing any usage for "koha_worker.pl" or "new_koha_job.pl". Are > these dev scripts that should be removed? "Bug 22417: [DO NOT PUSH] Add simple worker and add_job scripts" Those are for tests and won't be pushed.
(In reply to Jonathan Druart from comment #99) > The question is about a "normal" user (without that permission) who enqueued > a new job. Ah thanks for the clarification. I imagine just being authenticated should be sufficient in that case.
What's the status of this bug report? It's signed off, but is it waiting for QA or is it still a work in progress?
Created attachment 107682 [details] [review] Bug 22417: Let owner of a job see the progress A owner of a job should be able to see the progress of this job.
(In reply to David Cook from comment #101) > What's the status of this bug report? It's signed off, but is it waiting for > QA or is it still a work in progress? It's "Signed Off" and so ready for QA. Why are you asking?
(In reply to Jonathan Druart from comment #103) > It's "Signed Off" and so ready for QA. Why are you asking? Just wondering if there was anything else I could do to help move this along. You also emailed once about wanting this to be Signed Off even though it was already Signed Off, so was just wondering if I was missing something.
You can still add your signed-off-by stamp on the patch set to tell QA you agree with it.
(In reply to Jonathan Druart from comment #105) > You can still add your signed-off-by stamp on the patch set to tell QA you > agree with it. Ok I'll keep that in mind. I don't 100% agree with all the current code, but I really do want to see this get into 20.11.
Consider reading my (short) comment https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=1993#c54 It might be better to have Koha code enqueue to a destination of "/koha_kohadev_schedule" and let the message contain the exact details. Then a scheduler (in this case it can just schedule for right now) will take that message and send it (now or in the future at a designated time) to the job-specific queue "/koha_kohadev_batch_biblio_record_modification", which will be read by workers subscribed to that queue. I don't know how big of a change it would be at this point, but it might add some future proofing for when we do have a task scheduler...
Used this test plan for server setup: 1. apt install rabbitmq-server 2. service rabbitmq-server start 3. rabbitmq-plugins enable rabbitmq_stomp 4. apt install libnet-stomp-perl 5. cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh 6. cp debian/scripts/koha-worker /usr/bin/ 7. koha-worker --start kohadev 8. tail -f /var/log/rabbitmq/* Used this test plan for Staff Interface: 1. Go to http://localhost:8081/cgi-bin/koha/tools/batch_record_modification.pl 2. Create template at http://localhost:8081/cgi-bin/koha/tools/marc_modification_templates.pl which will add a 900$a field of "MQ_TEST" to the records I'm going to modify 3. Go back to http://localhost:8081/cgi-bin/koha/tools/batch_record_modification.pl 4. Choose newly created template and use a list of record numbers including 1-10. Complete following actions. 5. Got this error message: Cannot enqueue this job. (The error was: {UNKNOWN}: Failed to connect: Error connecting to localhost:61613: Connection refused at /usr/share/perl5/Net/Stomp.pm line 27.; giving up at /usr/share/perl5/Net/Stomp.pm line 27. at /kohadevbox/koha/Koha/BackgroundJob.pm line 61 . See the Koha logfile for more information). 6. service rabbitmq-server restart 7. Repeat Step 4 8. Go to http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl?op=view&id=2 9. Got error: DBIx::Class::Schema::source(): Can't find source for BackgroundJob at /kohadevbox/koha/Koha/Objects.pm line 543 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
Ah, it looks like I should've ran "restart_all" as well. My bad. 10. Go to http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl?op=view&id=3 11. See Progress 1/10 12. Refresh page after a few seconds 13. See Progress 10/10 14. http://localhost:8081/cgi-bin/koha/admin/background_jobs.pl 15. See list of finished jobs Hmm... I'm not seeing any changes to my files... I'll try a less obscure field like 500$a
Ok I see my change when I use 500$a. Beautiful. I'm happy to sign off on these patches. There is one odd thing and it's that I'm trying "rabbitmqctl list_queues" but I'm getting an error: Error: unable to connect to node rabbit@e507f05ef96a: nodedown Hostname mismatch: node "rabbit@kohadevbox" believes its host is different. Please ensure that hostnames resolve the same way locally and on "rabbit@kohadevbox" I'm going to look at that for a minute...
Ah, the root user in koha-testing-docker has a HOSTNAME=e507f05ef96a in my case. This works: root@kohadevbox:koha(bug_22417)$ HOSTNAME=kohadevbox rabbitmqctl list_queues Listing queues ... koha_kohadev-batch_authority_record_modification 0 koha_kohadev-batch_biblio_record_modification 0 batch_biblio_record_modification 0 I'm curious about that last queue there, but not a blocker.
Created attachment 108023 [details] [review] Bug 22417: UpdateDB entry TODO: - Add the FK on borrowernumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108024 [details] [review] Bug 22417: DBIX Schema changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108025 [details] [review] Bug 22417: Add Koha::BackgroundJob[s] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108026 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateBiblio Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108027 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateAuthority This is not ready yet! Note that this is too close to Koha::BackgroundJob::BatchUpdateBiblio, we will want to refactor bit of code. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108028 [details] [review] Bug 22417: [DO NOT PUSH] Add simple worker and add_job scripts Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108029 [details] [review] Bug 22417: Add GUI to watch the progress of jobs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108030 [details] [review] Bug 22417: Adapt the batch_record_modification tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108031 [details] [review] Bug 22417: Add the ability to cancel a job Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108032 [details] [review] Bug 22417: Fix borrowernumber values Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108033 [details] [review] Bug 22417: Add debian script koha-worker Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108034 [details] [review] Bug 22417: Switch to STOMP apt install rabbitmq-server service rabbitmq-server start rabbitmq-plugins enable rabbitmq_stomp apt install libnet-stomp-perl cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh cp debian/scripts/koha-worker /usr/bin/ koha-worker --start kohadev tail -f /var/log/rabbitmq/* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108035 [details] [review] Bug 22417: Handle errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108036 [details] [review] Bug 22417: Try to display pending jobs - only work if worker not started ie. no subscription yet This needs to be fixed (if possible), or removed from the patchset Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108037 [details] [review] Bug 22417: Fix the batch authority tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108038 [details] [review] Bug 22417: Restore the concept of namespace Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108039 [details] [review] Bug 22417: [DO NOT PUSH] Add sleep to notice the progress Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108040 [details] [review] Bug 22417: Add a note about the existence of the DB row Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108041 [details] [review] Bug 22417: [DO NOT PUSH] Adjust koha_worker.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108042 [details] [review] Bug 22417: Restore the 'add to list' feature This feature has been added recently by bug 18127. It highlights the need of a post processing hook. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108043 [details] [review] Bug 22417: Add the new permission manage_background_jobs QA: Please answer the question in admin/background_jobs.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108044 [details] [review] Bug 22417: Send message to /queue This cannot be: /queue/$namespace/$job_type I got: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue destination So keeping the dash in $namespace-$job_type Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108045 [details] [review] Bug 22417: Remove record_type from BatchUpdate* We don't need to pass it to the job, they each know which record type they are dealing with Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108046 [details] [review] Bug 22417: (follow-up) Fix the batch authority tool Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 108047 [details] [review] Bug 22417: Let owner of a job see the progress A owner of a job should be able to see the progress of this job. Signed-off-by: David Cook <dcook@prosentient.com.au>
(In reply to David Cook from comment #108) > Used this test plan for server setup: > 1. apt install rabbitmq-server > 2. service rabbitmq-server start > 3. rabbitmq-plugins enable rabbitmq_stomp > 4. apt install libnet-stomp-perl > 5. cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh > 6. cp debian/scripts/koha-worker /usr/bin/ > 7. koha-worker --start kohadev > 8. tail -f /var/log/rabbitmq/* > 5. Got this error message: > Cannot enqueue this job. (The error was: {UNKNOWN}: Failed to connect: > Error connecting to localhost:61613: Connection refused at > /usr/share/perl5/Net/Stomp.pm line 27.; giving up at > /usr/share/perl5/Net/Stomp.pm line 27. at > /kohadevbox/koha/Koha/BackgroundJob.pm line 61 . See the Koha logfile for > more information). > 6. service rabbitmq-server restart Yes I noticed that the last time I tried. It needs a restart after we enable the stomp plugin.
(In reply to David Cook from comment #111) > Ah, the root user in koha-testing-docker has a HOSTNAME=e507f05ef96a in my > case. > > This works: > > root@kohadevbox:koha(bug_22417)$ HOSTNAME=kohadevbox rabbitmqctl list_queues > Listing queues ... > koha_kohadev-batch_authority_record_modification 0 > koha_kohadev-batch_biblio_record_modification 0 > batch_biblio_record_modification 0 > > I'm curious about that last queue there, but not a blocker. maybe you tried the test script? new_koha_job.pl Thanks for the signoff!
Oh, I just remembered something else that should be addressed. The Koha::BackgroundJob->connect method should use configurable options rather than hard-coded default values. (Maybe it would even be OK to use the hard-coded default values in the absence of configured values for usability...) I would also suggest adding "host" to the the $stomp->connect call. Even if we default to "/", it would be nice to allow sysadmins to determine what virtual host to use in RabbitMQ, and it would cost us nothing but a line or two of code/config. (Note that doing this will help the koha-testing-docker and koha-docker projects to run RabbitMQ in a separate container.)
See bug 25674.
(In reply to Jonathan Druart from comment #140) > See bug 25674. Considering I wrote the majority of the content on bug 25674, I'm not 100% sure what you're implying. I'm guessing that you're saying we can fix that later. I won't disagree on that.
At this point, are we just waiting for the QA team to look at this report?
(In reply to David Cook from comment #142) > At this point, are we just waiting for the QA team to look at this report? See comment 103 and comment 104
(In reply to Jonathan Druart from comment #143) > (In reply to David Cook from comment #142) > > At this point, are we just waiting for the QA team to look at this report? > > See comment 103 and comment 104 [U+1F923] It's been over 2 weeks since those comments. I figured that its status as an RM priority would bump it up the QA queue, so I decided it wouldn't hurt to check in again.
(In reply to David Cook from comment #144) > I figured that its status as an RM priority would bump it up the QA queue Me too!
(In reply to Jonathan Druart from comment #145) > (In reply to David Cook from comment #144) > > I figured that its status as an RM priority would bump it up the QA queue > > Me too! Me too? This one isn't for me tho.
(In reply to Katrin Fischer from comment #146) > Me too? This one isn't for me tho. Is that due to any particular reason? I'm happy to provide more information if that will help.
This looks really great! There are some rough edges that will need polished, but that doesn't necessarily need to happen right now on this bug. A few notes: * Why are koha_worker.pl and new_koha_job.pl in root dir? * Why does new_koha_job.pl use Koha::BackgroundJob::BatchUpdateBiblio but not Koha::BackgroundJob::BatchUpdateAuthority? Is this needed * use Scalar::Util qw( weaken ); not used I think at this point, the focus could be on cleaning up the ui and not refactoring the internals until they need to be refactored. The most glaring issue is this message presented to the user: The job has been enqueued! It will be processed as soon as possible (FIXME - well, it could depend on a config?)
I'll let Jonathan respond to the notes, but I agree with your comment, Kyle. I figure with that little UI tidy up, it should be pretty much good to go.
Looking here too now
(In reply to Marcel de Rooy from comment #150) > Looking here too now Yay!!! :D
(In reply to Kyle M Hall from comment #148) > * Why are koha_worker.pl and new_koha_job.pl in root dir? If I understand correctly, you are referring to scripts from a DO NOT PUSH patch.
Created attachment 109270 [details] [review] Bug 22417: Remove useless use statement
Created attachment 109271 [details] [review] Bug 22417: Remove FIXME in the template
(In reply to Kyle M Hall from comment #148) > This looks really great! There are some rough edges that will need polished, > but that doesn't necessarily need to happen right now on this bug. > > A few notes: > > * Why are koha_worker.pl and new_koha_job.pl in root dir? > > * Why does new_koha_job.pl use Koha::BackgroundJob::BatchUpdateBiblio but > not Koha::BackgroundJob::BatchUpdateAuthority? Is this needed As Marcel said, those 2 scripts are for testing only. They won't be pushed. > * use Scalar::Util qw( weaken ); not used > > I think at this point, the focus could be on cleaning up the ui and not > refactoring the internals until they need to be refactored. > > The most glaring issue is this message presented to the user: > The job has been enqueued! It will be processed as soon as possible (FIXME - > well, it could depend on a config?) Fixed.
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 :)
(In reply to Marcel de Rooy from comment #156) > 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? I believe this is to prevent two workers from grabbing the same job. There may be other solutions, such as row locking. I'm sure others are more well versed in this subject than I am. > 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? I think a separate conf makes the most sense, as workers are per-server and not per-instance, right? > # 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 ? I see nothing wrong with what's being done here. The memcached_namespace is just another way of saying 'instance name' really. > 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. Unless they are expected to diverge more over time. I don't think this is a blocker, but could be a followup bug. > 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 :) Also, many minor QA script failures that need to be cleaned up.
Thanks Marcel and Kyle for having a look at this! (In reply to Marcel de Rooy from comment #156) > 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? I am not sure I understand what you are suggesting. Do you mean misc/background_jobs_worker.pl could watch the DB table and so we could remove the RabbitMQ dependency? > 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? If we need to tweak the configuration it could go into koha-conf. As I said in a comment (Koha::BackgroundJob->enqueue), specific vhosts need to be defined server-side. Here we just want to have it working out of the box. We will have to focus specifically on that depending on the needs we found once it's pushed. > # 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 As Kyle said, it does not really matter for now. We just need an ID. > Not sure btw if using the term namespace is confusing here; or just a queue > prefix ? Maybe you are right but it does not matter much :) > 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. Yes, maybe. The idea was to be flexible in case we need it. When we will have more job moved to this mechanism we will know what can be refactored. Also I think it's good to have more code in the module than in the controller scripts. > installer/data/mysql/atomicupdate/bug_15032.perl > Wrong bug number Indeed, it's actually from 15032. It has a long history :) > diff koha_worker.pl misc/background_jobs_worker.pl > Why did you add koha_worker.pl? We already run the misc script ? new_koha_job.pl and koha_worker.pl won't be pushed. They are there for testing purpose only. > No tests anywhere [yet]? Ok :) For point. My (very) bad tentative to justify would be that existing code was not covered by tests either. Argument rejected. I could try to write some tests for the 2 ->process method, but I don't think ->enqueue or ->connect can be tested easily.
> For point. Fair point!
(In reply to Jonathan Druart from comment #158) > (In reply to Marcel de Rooy from comment #156) > > 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? > > I am not sure I understand what you are suggesting. > Do you mean misc/background_jobs_worker.pl could watch the DB table and so > we could remove the RabbitMQ dependency? No, that is not my suggestion here. But this argument is about its architecture. We use a message queue for a more specific goal. This has translated into the actual code making it harder to widen the scope again. Later on, I want to add messages (no tasks) in another area but what if I dont want to go via BackgroundJob? The last few sentences actually ask for a justification why send we the same data to both a table and the message queue. What is in this specific case the added value of the message queue? If we only use it for this specific case, it looks like much overhead for what it brings. If the justification is in the wider use of a message queue, lets translate that to its design.
(In reply to Marcel de Rooy from comment #160) > No, that is not my suggestion here. But this argument is about its > architecture. We use a message queue for a more specific goal. This has > translated into the actual code making it harder to widen the scope again. > Later on, I want to add messages (no tasks) in another area but what if I > dont want to go via BackgroundJob? I had the same thought. Locally, I'm already using RabbitMQ for some experimental features, and I have a Koha::MQ class that takes care of connections, configuration, etc. I use Koha::MQ in my Web UI to send a fire and forget message to a background service. It's a very light implementation. > The last few sentences actually ask for a justification why send we the same > data to both a table and the message queue. What is in this specific case > the added value of the message queue? If we only use it for this specific > case, it looks like much overhead for what it brings. If the justification > is in the wider use of a message queue, lets translate that to its design. You need the database table to track the task. The database table allows you to see its progress, store results, etc. Otherwise, you're just doing a fire and forget (like I do above), which is problematic, as there's no way of tracking the task to see if it succeeded or what its output was. But the message queue is more efficient and scalable than polling the database. With the message queue, workers use a select(2) syscall to wait until they have data to read on the socket connection they have to the message broker. They don't consume CPU time. They just sleep. They're only woken up when they have a message from the message broker. When polling the database, you're generating steady network and file I/O, so then you tune the polling time to something that doesn't impact your system too much, which takes time/energy/experience and is ultimately slower than the message queue. Plus, if you were polling the database, you'd only be able to have 1 process doing that. You couldn't have 5 processes polling the database. So then your process polling the database becomes a de facto message broker anyway, and then you're re-inventing the wheel (and spokes).
(In reply to David Cook from comment #161) > With the message queue, workers use a select(2) syscall to wait > until they have data to read on the socket connection they have to the > message broker. That is workers that are using Net::Stomp, as it uses a select-based event loop. There's no reason a person couldn't have workers (ie queue consumers) written using different event systems. I think with AnyEvent::STOMP you can specify your own event loop (like the more efficient epoll), or even use a different programming language all together.
(In reply to David Cook from comment #161) Thanks for replying. > Locally, I'm already using RabbitMQ for some experimental features, and I > have a Koha::MQ class that takes care of connections, configuration, etc. We will need such a class too when extending this dev. So the only question is: when will we add it? > But the message queue is more efficient and scalable than polling the > database. Good point.
David answered perfectly to the need of the insert in the table. I will answer about the design then, Koha::BackgroundJob is coming from bug 15032, written more than 3 years ago. The idea here is to provide a fix for the background jobs to make all the controllers work under plack (and so fix the different bugs linked with bug 15032). Nothing else, but it let the ground open to other enhancements. I don't think it's a good idea to block this development more. IMO we really need it into master to get new ideas on top of it. I think we could easily provide a follow-up bug report renaming background job with task queue (DB table, scripts and modules).
(In reply to Jonathan Druart from comment #164) > I don't think it's a good idea to block this development more. IMO we really > need it into master to get new ideas on top of it. I think we could easily > provide a follow-up bug report renaming background job with task queue (DB > table, scripts and modules). We can push it, but the comments show that work needs to be done. Furthermore we should not use two standards: smaller devs are blocked for arbitrary, picky reasons, but large things can go in just as-is putting aside constructice criticism.
(In reply to Jonathan Druart from comment #164) > David answered perfectly to the need of the insert in the table. > > I think we could easily > provide a follow-up bug report renaming background job with task queue (DB > table, scripts and modules). Which was not the main point at all.
As I explained in the main commit message, this patch is a first step. The discussions highlighted the need of a message queue for other uses I had not in mind (like plugins). I am not asking for any privileges here, and I don't have any personal interests in this feature, but I feel like it's something very important for the project. If you really think it should be redone and it's not acceptable as it (because the design is wrong and will block further enhancements) then we should not let it go as it. I am willing to help anybody who would like to improve it. I am resetting to NQA as the patches do not have a PQA stamp.
(In reply to Jonathan Druart from comment #167) > I am resetting to NQA as the patches do not have a PQA stamp. I will add my signoff line in the expectation that things like improving configuration and future extension for other MQ use will be handled later on.
Created attachment 109319 [details] [review] Bug 22417: UpdateDB entry TODO: - Add the FK on borrowernumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109320 [details] [review] Bug 22417: DBIX Schema changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109321 [details] [review] Bug 22417: Add Koha::BackgroundJob[s] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109322 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateBiblio Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109323 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateAuthority This is not ready yet! Note that this is too close to Koha::BackgroundJob::BatchUpdateBiblio, we will want to refactor bit of code. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109324 [details] [review] Bug 22417: [DO NOT PUSH] Add simple worker and add_job scripts Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109325 [details] [review] Bug 22417: Add GUI to watch the progress of jobs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109326 [details] [review] Bug 22417: Adapt the batch_record_modification tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109327 [details] [review] Bug 22417: Add the ability to cancel a job Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109328 [details] [review] Bug 22417: Fix borrowernumber values Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109329 [details] [review] Bug 22417: Add debian script koha-worker Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109330 [details] [review] Bug 22417: Switch to STOMP apt install rabbitmq-server service rabbitmq-server start rabbitmq-plugins enable rabbitmq_stomp apt install libnet-stomp-perl cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh cp debian/scripts/koha-worker /usr/bin/ koha-worker --start kohadev tail -f /var/log/rabbitmq/* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109331 [details] [review] Bug 22417: Handle errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109332 [details] [review] Bug 22417: Try to display pending jobs - only work if worker not started ie. no subscription yet This needs to be fixed (if possible), or removed from the patchset Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109333 [details] [review] Bug 22417: Fix the batch authority tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109334 [details] [review] Bug 22417: Restore the concept of namespace Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109335 [details] [review] Bug 22417: [DO NOT PUSH] Add sleep to notice the progress Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109336 [details] [review] Bug 22417: Add a note about the existence of the DB row Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109337 [details] [review] Bug 22417: [DO NOT PUSH] Adjust koha_worker.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109338 [details] [review] Bug 22417: Restore the 'add to list' feature This feature has been added recently by bug 18127. It highlights the need of a post processing hook. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109339 [details] [review] Bug 22417: Add the new permission manage_background_jobs QA: Please answer the question in admin/background_jobs.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109340 [details] [review] Bug 22417: Send message to /queue This cannot be: /queue/$namespace/$job_type I got: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue destination So keeping the dash in $namespace-$job_type Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109341 [details] [review] Bug 22417: Remove record_type from BatchUpdate* We don't need to pass it to the job, they each know which record type they are dealing with Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109342 [details] [review] Bug 22417: (follow-up) Fix the batch authority tool Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109343 [details] [review] Bug 22417: Let owner of a job see the progress A owner of a job should be able to see the progress of this job. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109344 [details] [review] Bug 22417: Remove useless use statement Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109345 [details] [review] Bug 22417: Remove FIXME in the template Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
QA script complains about missing POD, missing TT filters, and spelling. Also Net::Stomp is missing in cpanfile FAIL Koha/BackgroundJob.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL Koha/BackgroundJob/BatchUpdateAuthority.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL Koha/BackgroundJob/BatchUpdateBiblio.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL Koha/BackgroundJobs.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL Koha/Schema/Result/BackgroundJob.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL admin/background_jobs.pl FAIL forbidden patterns forbidden pattern: Script permissions is authnotrequired => 0, it could be correct for an OPAC script if it is was you really want error (bug 24663) (line 42) OK debian/docs/koha-common.xml OK installer/data/mysql/kohastructure.sql OK koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc FAIL koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt FAIL filters missing_filter at line 27 ( <div><i class="fa fa-exclamation error"></i>Cannot retrieve pending jobs ([% m.error %])</div>) missing_filter at line 107 ( Bibliographic record <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% m.biblionumber | uri %]">[% m.biblionumber | html %]</a> has not been modified. An error occurred on modifying it.[% IF m.error %] ([% m.error %])[% END %].) missing_filter at line 125 ( Authority record <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% m.authid | uri %]">[% m.authid | html %]</a> has not been modifi ed. An error occurred on modifying it[% IF m.error %] ([% m.error %])[% END %].) missing_filter at line 149 ( There is [% pending_jobs.size %] pending jobs on the server: [% pending_jobs.join(', ') %].) missing_filter at line 149 ( There is [% pending_jobs.size %] pending jobs on the server: [% pending_jobs.join(', ') %].) FAIL spelling resizeable ==> resizable OK koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt OK koha_worker.pl OK misc/background_jobs_worker.pl OK new_koha_job.pl OK tools/batch_record_modification.pl
Created attachment 109350 [details] [review] Bug 22417: Add missing POD and html filters Also remove "authnotrequired => 0,"
Created attachment 109351 [details] [review] Bug 22417: Add new Net::Stomp dependency
Actually the spelling error is more than that. 'resizeable' is used as an option of window.open, but the correct option is 'resizable', so it must be fixed. https://developer.mozilla.org/fr/docs/Web/API/Window/open All other QA script errors seem to be fixed by the last two patches.
Created attachment 109355 [details] [review] Bug 22417: Fix spelling resizeable vs resizable For the record, codespell does not raise the problem within ktd with 1.17.1 but appears with 1.14
I've opened bug 26324 for the other occurrence of resizeable we have in the codebase.
(In reply to Marcel de Rooy from comment #168) > I will add my signoff line in the expectation that things like improving > configuration and future extension for other MQ use will be handled later on. Cheers, Marcel! I'll certainly be contributing patches once this main patch set is pushed. I have ideas and enthusiasm.
All errors from QA script are gone :) One minor annoyance: when you apply all patches except those with "DO NOT PUSH", it fail at some point: Application de Bug 22417: UpdateDB entry Application de Bug 22417: DBIX Schema changes Application de Bug 22417: Add Koha::BackgroundJob[s] Application de Bug 22417: Add Koha::BackgroundJob::BatchUpdateBiblio Application de Bug 22417: Add Koha::BackgroundJob::BatchUpdateAuthority Application de Bug 22417: Add GUI to watch the progress of jobs Application de Bug 22417: Adapt the batch_record_modification tool Application de Bug 22417: Add the ability to cancel a job Utilisation de l'information de l'index pour reconstruire un arbre de base... A new_koha_job.pl Retour à un patch de la base et fusion à 3 points... CONFLIT (modification/suppression) : new_koha_job.pl supprimé dans HEAD et modifié dans Bug 22417: Add the ability to cancel a job. Version Bug 22417: Add the ability to cancel a job de new_koha_job.pl laissée dans l'arbre. error: Échec d'intégration des modifications. le patch a échoué à 0001 Bug 22417: Add the ability to cancel a job It may be good to do a little cleanup on these patches, and maybe squash followup patches ?
Created attachment 109390 [details] [review] Bug 22417: Remove $dbh in Koha::BackgroundJob::* Also remove unused $job_type var
Created attachment 109391 [details] [review] Bug 22417: Add tests Finally!
Created attachment 109392 [details] [review] Bug 22417: UpdateDB entry TODO: - Add the FK on borrowernumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109393 [details] [review] Bug 22417: DBIX Schema changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109394 [details] [review] Bug 22417: Add Koha::BackgroundJob[s] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109395 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateBiblio Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109396 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateAuthority This is not ready yet! Note that this is too close to Koha::BackgroundJob::BatchUpdateBiblio, we will want to refactor bit of code. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109397 [details] [review] Bug 22417: Add GUI to watch the progress of jobs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109398 [details] [review] Bug 22417: Adapt the batch_record_modification tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109399 [details] [review] Bug 22417: Add the ability to cancel a job Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109400 [details] [review] Bug 22417: Fix borrowernumber values Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109401 [details] [review] Bug 22417: Add debian script koha-worker Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109402 [details] [review] Bug 22417: Switch to STOMP apt install rabbitmq-server service rabbitmq-server start rabbitmq-plugins enable rabbitmq_stomp apt install libnet-stomp-perl cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh cp debian/scripts/koha-worker /usr/bin/ koha-worker --start kohadev tail -f /var/log/rabbitmq/* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 22417: Remove useless use statement Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109403 [details] [review] Bug 22417: Handle errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109404 [details] [review] Bug 22417: Try to display pending jobs - only work if worker not started ie. no subscription yet This needs to be fixed (if possible), or removed from the patchset Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109405 [details] [review] Bug 22417: Fix the batch authority tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 22417: Add a note about the existence of the DB row Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 22417: (follow-up) Fix the batch authority tool Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109406 [details] [review] Bug 22417: Restore the concept of namespace Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109407 [details] [review] Bug 22417: Restore the 'add to list' feature This feature has been added recently by bug 18127. It highlights the need of a post processing hook. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109408 [details] [review] Bug 22417: Add the new permission manage_background_jobs QA: Please answer the question in admin/background_jobs.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109409 [details] [review] Bug 22417: Send message to /queue This cannot be: /queue/$namespace/$job_type I got: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue destination So keeping the dash in $namespace-$job_type Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109410 [details] [review] Bug 22417: Remove record_type from BatchUpdate* We don't need to pass it to the job, they each know which record type they are dealing with Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109411 [details] [review] Bug 22417: Let owner of a job see the progress A owner of a job should be able to see the progress of this job. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109412 [details] [review] Bug 22417: Remove FIXME in the template Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 109413 [details] [review] Bug 22417: Add missing POD and html filters Also remove "authnotrequired => 0,"
Created attachment 109414 [details] [review] Bug 22417: Add new Net::Stomp dependency
Created attachment 109415 [details] [review] Bug 22417: Fix spelling resizeable vs resizable For the record, codespell does not raise the problem within ktd with 1.17.1 but appears with 1.14
Created attachment 109416 [details] [review] Bug 22417: Remove $dbh in Koha::BackgroundJob::* Also remove unused $job_type var
Created attachment 109417 [details] [review] Bug 22417: Add tests Finally!
Created attachment 109418 [details] [review] Bug 22417: [DO NOT PUSH] Add simple worker and add_job scripts
I squashed some patches (the easy ones), more squashes could be done but changes will have to be split (so more work and not much added value IMO). I put the 2 test scripts on a separate commit (for history). Julian, do you want to have another look?
Sorry, there is a new error from QA script: FAIL t/lib/Koha/BackgroundJob/BatchTest.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 Once it is fixed, I think it safe to change status back to Passed QA, since 2 QAers already added their signoff
I removed the POD for this test module on purpose, I don't think it makes much sense to have POD there, we already have similar POD in the 3 other modules.
I have 2 questions: We need apt install rabbitmq-server rabbitmq-plugins enable rabbitmq_stomp service rabbitmq-server restart we can have rabbitmq-server inside control.in, but what about the 2 other commands? Should we have them part of the install process or in koha-common (like we try to guess if the plugin is enabled, otherwise we enable it and restart rabbitmq)? Also, shouldn't we add a rabbitmq status indicator on the about page?
(In reply to Jonathan Druart from comment #236) > I have 2 questions: > > We need > apt install rabbitmq-server > rabbitmq-plugins enable rabbitmq_stomp > service rabbitmq-server restart > > we can have rabbitmq-server inside control.in, but what about the 2 other > commands? > > Should we have them part of the install process or in koha-common (like we > try to guess if the plugin is enabled, otherwise we enable it and restart > rabbitmq)? > I think that we should probably treat rabbitmq setup the same way we treat mysql installation; that is, as a separate step. The Koha server(s) should only be concerned with the network connection to RabbitMQ. The sysadmin should be responsible for configuring RabbitMQ apart from that. (Same day, we might change our mind about that, in that case I'd say probably require Koha to use the RabbitMQ REST API (plugin). While it's not necessarily an out-of-the-box solution, I think it is a reasonable one.) > Also, shouldn't we add a rabbitmq status indicator on the about page? +1 I would really like this
(In reply to David Cook from comment #237) > I think that we should probably treat rabbitmq setup the same way we treat > mysql installation; that is, as a separate step. > To that end, I think that we should "suggest" rather than "require" rabbitmq-server in the debian/control file. When running koha-testing-docker, we'll have a separate container for rabbitmq. We won't have it installed locally. Production deployments may also have a 2+ instance RabbitMQ cluster (using either round robin DNS or a load balancer or both). Those won't be local to the Koha server(s) either. So Koha can only interact with RabbitMQ over a network.
MySQL/MariaDB is very standard, RabbitMQ is not... I am far from understanding the whole discussion here, but from the use cases and "need" for this I have heard about, I wonder if this can be really kept an optional feature. And if it turns mandatory, are we adding a new hurdle to installing Koha if we keep it all separate for the sysadmin to deal with?
It would be great if RabbitMQ was made optional, perhaps by adding a very basic task queue implementation (for instance with only 1 worker allowed to avoid many problems of a task queue) that will be used if RabbitMQ is not configured ? But that would still require sysadmins to add a "service" (systemd service, start/stop script, ...) for the worker, so there would be manual steps anyway. Is there a way to make background jobs work automatically on the next upgrade ?
(In reply to Julian Maurice from comment #240) > It would be great if RabbitMQ was made optional, perhaps by adding a very > basic task queue implementation (for instance with only 1 worker allowed to > avoid many problems of a task queue) that will be used if RabbitMQ is not > configured ? > But that would still require sysadmins to add a "service" (systemd service, > start/stop script, ...) for the worker, so there would be manual steps > anyway. > > Is there a way to make background jobs work automatically on the next > upgrade ? There is the a start/stop script provided with the debian package (debian/scripts/koha-worker). It will start the daemon like we do with other services. There is no extra steps needed for that part. The only bit to make it works is the 3 lines I pasted earlier: apt install rabbitmq-server # done with the deb dependency rabbitmq-plugins enable rabbitmq_stomp service rabbitmq-server restart And maybe that could be a postinst task (?)
Created attachment 109474 [details] [review] Bug 22417: Add rabbitmq status indicator on the about page
(In reply to Jonathan Druart from comment #241) > (In reply to Julian Maurice from comment #240) > > It would be great if RabbitMQ was made optional, perhaps by adding a very > > basic task queue implementation (for instance with only 1 worker allowed to > > avoid many problems of a task queue) that will be used if RabbitMQ is not > > configured ? > > But that would still require sysadmins to add a "service" (systemd service, > > start/stop script, ...) for the worker, so there would be manual steps > > anyway. > > > > Is there a way to make background jobs work automatically on the next > > upgrade ? > > There is the a start/stop script provided with the debian package > (debian/scripts/koha-worker). It will start the daemon like we do with other > services. There is no extra steps needed for that part. What about non-package installs ? It's probably not possible to automate everything, but a minimal systemd service file for the worker would be a good start I think. So when you update/install Koha you just have to copy the file to /etc/systemd/system and adjust paths/users/.. It has certainly helped me in the past with other softwares I installed. Can be done in another bug report, but what do you think about this ?
Created attachment 109475 [details] [review] Bug 22417: Add rabbitmq status indicator on the about page
Created attachment 109476 [details] [review] Bug 22417: Remove list of pending jobs This was not accurate as it was only retrieving jobs for batch_biblio_record_modification We will need to improve that later if needed
Created attachment 109477 [details] [review] Bug 22417: Process the jobs even if the message broker is not reachable
This last patch adds the ability to process the jobs even if the message broker is not installed/configured correctly. I am not sure it is a good idea, we will want to make the message broker mandatory at some point anyway (for messages not in the DB for instance). And so we are only postponing the problem.
(In reply to Julian Maurice from comment #243) > (In reply to Jonathan Druart from comment #241) > > (In reply to Julian Maurice from comment #240) > > > It would be great if RabbitMQ was made optional, perhaps by adding a very > > > basic task queue implementation (for instance with only 1 worker allowed to > > > avoid many problems of a task queue) that will be used if RabbitMQ is not > > > configured ? > > > But that would still require sysadmins to add a "service" (systemd service, > > > start/stop script, ...) for the worker, so there would be manual steps > > > anyway. > > > > > > Is there a way to make background jobs work automatically on the next > > > upgrade ? > > > > There is the a start/stop script provided with the debian package > > (debian/scripts/koha-worker). It will start the daemon like we do with other > > services. There is no extra steps needed for that part. > > What about non-package installs ? > It's probably not possible to automate everything, but a minimal systemd > service file for the worker would be a good start I think. So when you > update/install Koha you just have to copy the file to /etc/systemd/system > and adjust paths/users/.. It has certainly helped me in the past with other > softwares I installed. > Can be done in another bug report, but what do you think about this ? Yes, that could be a good idea!
(In reply to Jonathan Druart from comment #247) > This last patch adds the ability to process the jobs even if the message > broker is not installed/configured correctly. > > I am not sure it is a good idea, we will want to make the message broker > mandatory at some point anyway (for messages not in the DB for instance). > And so we are only postponing the problem. IMO we should not rely on the connection status (RabbitMQ is reachable or not) but instead on the configuration (RabbitMQ is configured or not). For instance, if configuration for RabbitMQ exists in $KOHA_CONF, use that, and if it is not reachable then die. And if there is no configuration, use the "dumb" worker system. That would allow people to just use the dumb system if it works for them, and if not sufficient they can "upgrade" by installing and configuring RabbitMQ
(In reply to Julian Maurice from comment #249) > (In reply to Jonathan Druart from comment #247) > > This last patch adds the ability to process the jobs even if the message > > broker is not installed/configured correctly. > > > > I am not sure it is a good idea, we will want to make the message broker > > mandatory at some point anyway (for messages not in the DB for instance). > > And so we are only postponing the problem. > > IMO we should not rely on the connection status (RabbitMQ is reachable or > not) but instead on the configuration (RabbitMQ is configured or not). > For instance, if configuration for RabbitMQ exists in $KOHA_CONF, use that, > and if it is not reachable then die. And if there is no configuration, use > the "dumb" worker system. > That would allow people to just use the dumb system if it works for them, > and if not sufficient they can "upgrade" by installing and configuring > RabbitMQ If we feel that RabbitMQ is not ready for primary processes like the batch record modify in this patch set, we should remove that part. I would not promote a hybrid solution that falls back to code without MQ. It should just be stable enough to operate on and seen as an essential component.
(In reply to Julian Maurice from comment #249) > IMO we should not rely on the connection status (RabbitMQ is reachable or > not) but instead on the configuration (RabbitMQ is configured or not). > For instance, if configuration for RabbitMQ exists in $KOHA_CONF, use that, > and if it is not reachable then die. And if there is no configuration, use > the "dumb" worker system. > That would allow people to just use the dumb system if it works for them, > and if not sufficient they can "upgrade" by installing and configuring > RabbitMQ So far there is no configuration, the default parameters are used and hardcoded in the connection method. (In reply to Marcel de Rooy from comment #250) > If we feel that RabbitMQ is not ready for primary processes like the batch > record modify in this patch set, we should remove that part. I would not > promote a hybrid solution that falls back to code without MQ. It should just > be stable enough to operate on and seen as an essential component. I am against removing background jobs like batch record modify, it's the main point of this first step.
(In reply to Marcel de Rooy from comment #250) > If we feel that RabbitMQ is not ready for primary processes like the batch > record modify in this patch set, we should remove that part. I would not > promote a hybrid solution that falls back to code without MQ. It should just > be stable enough to operate on and seen as an essential component. It's not about stability. I'm sure RabbitMQ is stable enough for this. It's about not adding another mandatory dependency when we don't need it. Do I really need a message queue if I run 1 background job per day ? Of course a MQ will be useful to some. I'm not advocating against it. I'm only saying a fallback would be nice for those who only run jobs occasionally. Also the upgrade path would be simpler ("just make sure the script worker.pl is always running and you're good")
(In reply to Jonathan Druart from comment #251) > So far there is no configuration, the default parameters are used and > hardcoded in the connection method. That should probably be changed if, as someone already mentioned, one want to install rabbitmq on another server, or in a different docker container, ...
(In reply to Julian Maurice from comment #252) > (In reply to Marcel de Rooy from comment #250) > > If we feel that RabbitMQ is not ready for primary processes like the batch > > record modify in this patch set, we should remove that part. I would not > > promote a hybrid solution that falls back to code without MQ. It should just > > be stable enough to operate on and seen as an essential component. > > It's not about stability. I'm sure RabbitMQ is stable enough for this. > It's about not adding another mandatory dependency when we don't need it. Do > I really need a message queue if I run 1 background job per day ? > Of course a MQ will be useful to some. I'm not advocating against it. I'm > only saying a fallback would be nice for those who only run jobs > occasionally. > Also the upgrade path would be simpler ("just make sure the script worker.pl > is always running and you're good") So you think the last patch is the (kind of) way to go? (In reply to Julian Maurice from comment #253) > (In reply to Jonathan Druart from comment #251) > > So far there is no configuration, the default parameters are used and > > hardcoded in the connection method. > > That should probably be changed if, as someone already mentioned, one want > to install rabbitmq on another server, or in a different docker container, > ... Yes, see bug 25674 ;)
I hope that the last patch is not the design pattern for introduction of a MQ in Koha. No blocker for me, just amazement ;)
Hi Marcel, > I would not promote a hybrid solution that falls back to code without MQ Ok, but why ? > I hope that the last patch is not the design pattern for introduction of a MQ in Koha Ok, but why ? Why would it be such a terrible design to have the ability to run background jobs without a message queue ?
(In reply to Julian Maurice from comment #256) > > I would not promote a hybrid solution that falls back to code without MQ > Ok, but why ? Duplication of code. Maintenance, bugs etc. > > I hope that the last patch is not the design pattern for introduction of a MQ in Koha > Ok, but why ? See above. > Why would it be such a terrible design to have the ability to run background > jobs without a message queue ? Actually not. As you say, we could run without MQ. But since we want a message queue anyway, this could be a nice first start. What I do not like, is writing two blocks: code with MQ en code without.
(In reply to Marcel de Rooy from comment #257) > Actually not. As you say, we could run without MQ. But since we want a > message queue anyway, this could be a nice first start. That's a thing that is still not clear for me. For what other purpose do we need a message queue ? > What I do not like, is writing two blocks: code with MQ en code without. I do not like it either, but compared to forcing hundreds of users to install a new dependency, it doesn't look too bad.
(In reply to Julian Maurice from comment #258) > (In reply to Marcel de Rooy from comment #257) > > Actually not. As you say, we could run without MQ. But since we want a > > message queue anyway, this could be a nice first start. > That's a thing that is still not clear for me. For what other purpose do we > need a message queue ? For now: all background jobs, and a task scheduler. But that can be extended to other things if we want (like indexing, sending emails, etc.). With a configuration system we could add the capacity to define rules like "execute all the background jobs that does XXX", or "don't execute background jobs during opening hours". Also we could have several workers, each doing something specific, and have them running on a dedicated server. > > What I do not like, is writing two blocks: code with MQ en code without. > I do not like it either, but compared to forcing hundreds of users to > install a new dependency, it doesn't look too bad. The idea was to not reinvent the wheel and use one solution that demonstrated its effectiveness. We could write our own server if we wish. In this implementation (there have been others previously) I decided to make things as simple as possible.
(In reply to Jonathan Druart from comment #259) > (In reply to Julian Maurice from comment #258) > > (In reply to Marcel de Rooy from comment #257) > > > Actually not. As you say, we could run without MQ. But since we want a > > > message queue anyway, this could be a nice first start. > > That's a thing that is still not clear for me. For what other purpose do we > > need a message queue ? > > For now: all background jobs, and a task scheduler. I'm curious about how we would implement a task scheduler with a message queue. Would that be with something like https://github.com/rabbitmq/rabbitmq-delayed-message-exchange ? > But that can be extended to other things if we want (like indexing, sending > emails, etc.). Indexing and sending emails can be implemented as a background job too. How do these tasks differ from a task like BatchUpdateBiblio ? > With a configuration system we could add the capacity to define rules like > "execute all the background jobs that does XXX", or "don't execute > background jobs during opening hours". Also we could have several workers, > each doing something specific, and have them running on a dedicated server. The ability to have multiple workers is really great, but how will you run them on another server ? You would need to have the whole Koha code duplicated on this other server, right ? > > > What I do not like, is writing two blocks: code with MQ en code without. > > I do not like it either, but compared to forcing hundreds of users to > > install a new dependency, it doesn't look too bad. > > The idea was to not reinvent the wheel and use one solution that > demonstrated its effectiveness. We could write our own server if we wish. > In this implementation (there have been others previously) I decided to make > things as simple as possible. Do you mean our own message broker ? Because I certainly do not wish that. All I'm suggesting is to make the dependency to RabbitMQ (or any other MQ) optional, for small installations that won't benefit from it.
(In reply to Katrin Fischer from comment #239) > MySQL/MariaDB is very standard, RabbitMQ is not... I am far from > understanding the whole discussion here, but from the use cases and "need" > for this I have heard about, I wonder if this can be really kept an optional > feature. And if it turns mandatory, are we adding a new hurdle to installing > Koha if we keep it all separate for the sysadmin to deal with? I wasn't saying it should be optional. I was saying that it must not be installed locally on the same server as Koha. It makes no operational sense. A new hurdle is also just a reality of building a better performing system. I'm happy to provide user-friendly documentation on the Wiki so that it's simple to install and configure RabbitMQ. It's really not difficult at all.
(In reply to Julian Maurice from comment #243) > What about non-package installs ? > It's probably not possible to automate everything, but a minimal systemd > service file for the worker would be a good start I think. So when you > update/install Koha you just have to copy the file to /etc/systemd/system > and adjust paths/users/.. It has certainly helped me in the past with other > softwares I installed. > Can be done in another bug report, but what do you think about this ? As I've noted elsewhere, I'm already using RabbitMQ with Koha locally, and I use a systemd template service definition, which makes it really easy to set up individual systemd units per Koha instance. I'm happy to contribute a community friendly version of this. But I think we can do that as a high-priority follow-up.
(In reply to Julian Maurice from comment #260) > I'm curious about how we would implement a task scheduler with a message > queue. Would that be with something like > https://github.com/rabbitmq/rabbitmq-delayed-message-exchange ? > Jonathan has previously suggested that plugin (https://www.rabbitmq.com/blog/2015/04/16/scheduling-messages-with-rabbitmq/). It could work. Task schedulers are a passion of mine, and there are also other ways of doing them with a message queue. Typically, you'll see to-purpose software like Celery (https://docs.celeryproject.org/en/stable/index.html) or BigBen (https://github.com/walmartlabs/bigben) that provide task schedulers that use message queues. I've also studied the source code of Perl libraries like AnyEvent, POE, Mojo::IOLoop, etc as well as the native implementation of timers in the Golang programming language. Ultimately, task schedulers are all about running a process (or a thread) that sleeps until a task is scheduled to run. It then triggers some kind of event. In Golang or Perl, that's whatever function you defined. In Celery or BigBen, it will be to send a message through the message queue to worker processes. I've written my own task schedulers in Perl and in Golang, and I'm happy to provide one for Koha later. It's not required right now, but I know what to do to implement one. (In reply to Julian Maurice from comment #260) > The ability to have multiple workers is really great, but how will you run > them on another server ? You would need to have the whole Koha code > duplicated on this other server, right ? > Not necessarily. If we write code well, we could write workers that could run on standalone servers/containers. We just need to get out of the monolith mindset. This would possibly involve building up the HTTP API further, which is a goal we already have anyway. For Koha packages, we could also package that worker code into distinct packages, which makes it easy to install on other servers/containers. I plan to do more work on this front in the future for Koha. (In reply to Julian Maurice from comment #260) > Do you mean our own message broker ? Because I certainly do not wish that. > All I'm suggesting is to make the dependency to RabbitMQ (or any other MQ) > optional, for small installations that won't benefit from it. Except even small installations *will* benefit from using a message queue. Right now, Koha - even when running Plack - depends on CGI. Using RabbitMQ provides a standard way of breaking our remaining dependency on CGI scripts for long-running tasks. As time passes, more and more of Koha will use the message queue as well. There are many long-running jobs, which would benefit from being placed into the background. This change is about building capacity and improving performance. Koha is about being all things to all people, and really all people need asynchronous processing, especially as users demand smoother user experiences.
(In reply to Katrin Fischer from comment #239) > MySQL/MariaDB is very standard, RabbitMQ is not... I just want to add that message queues like RabbitMQ, ActiveMQ, Amazon SQS, etc are common in the software world. Both ActiveMQ and Amazon SQS have been around for over 15 years. They're not uncommon in the library software world either. Both DSpace and Fedora Commons come with embedded ActiveMQ servers. (Since those are Java apps, they have the advantage of being able to embed Java-based apps like ActiveMQ and Solr.) Using Plack and RabbitMQ actually helps Koha to catch-up with other (open source) library systems.
I promise this is my last comment for now ;). I ask that people trust Tomas, Jonathan, and myself. Only the Batch Record Modification tool has been altered so far. That's not a tool critical to daily library operations. If there are issues, we can sort them out in the future with no major disruptions to library operations. If there are concerns about installations and dependencies, then I'm happy to help people with the practical aspect of that and hopefully show that they have no reason to be concerned. I run Koha from packages on Debian and Ubuntu, I run Koha from source install on OpenSuse, and I run Koha from git install on OpenSuse. I also have already implemented RabbitMQ for every one of those Koha instances, and have a custom Koha web tool which uses RabbitMQ to send messages to a custom Koha worker that is set up using a Systemd unit file. I acknowledge that I'm passionate and experienced, so maybe I'm not thinking about what it will be like for an inexperienced and more tentative Koha installer, but there really is no reason to fear RabbitMQ. Essentially, you install it, and then you forget about it. It just works. (And if you're really stuck, I'm happy to field any questions.)
OK just one more comment... Why don't we create a metapackage for koha like "koha-standalone" or "koha-small", which requires MySQL, Elasticsearch, and RabbitMQ (and any other external dependencies which should be run on other servers but don't have to be for a small instance). That could offer an easy setup for small Koha libraries, while leaving koha-common for more experienced and well-resourced Koha libraries and Koha support providers? (Side note: Once we have added RabbitMQ and eliminated our dependence on CGI, and maybe packaged static assets separately, we won't have to include Apache with Koha anymore either. The Apache dependency could be added to that metapackage for novice users, but most of us could replace it with Nginx or HAProxy or some other reverse proxy/load balancer.) In my mind, the koha-common package is going to become smaller over time, as we'll need fewer dependencies running on the same server as the Starman application server. So maybe the best compromise is to retain a common/core package and then have a more comprehensive metapackage which also includes that common/core package. That should make everyone happy?
(In reply to David Cook from comment #263) > Right now, Koha - even when running Plack - depends on CGI. Using RabbitMQ > provides a standard way of breaking our remaining dependency on CGI scripts > for long-running tasks. I am not following this argument. We are not introducing a MQ to eliminate CGI. Imo this is really another topic, not in the scope of this report. > As time passes, more and more of Koha will use the message queue as well. > There are many long-running jobs, which would benefit from being placed into > the background. > This change is about building capacity and improving performance. So we have an architectural reason for a MQ. And the question is: do we want an optional introduction needing additional fallback code, or dont we postpone making it mandatory (since it will be one day)?
(In reply to Marcel de Rooy from comment #267) > > As time passes, more and more of Koha will use the message queue as well. > > There are many long-running jobs, which would benefit from being placed into > > the background. > > This change is about building capacity and improving performance. > > So we have an architectural reason for a MQ. And the question is: do we want > an optional introduction needing additional fallback code, or dont we > postpone making it mandatory (since it will be one day)? My point was that we can run jobs in background without a message queue and without adding new dependencies. But since everyone want a message queue anyway, please ignore my comments.
(In reply to Marcel de Rooy from comment #267) > (In reply to David Cook from comment #263) > > Right now, Koha - even when running Plack - depends on CGI. Using RabbitMQ > > provides a standard way of breaking our remaining dependency on CGI scripts > > for long-running tasks. > > I am not following this argument. We are not introducing a MQ to eliminate > CGI. Imo this is really another topic, not in the scope of this report. The background job processes are running in CGI mode % grep ProxyPass debian/templates/apache-shared-intranet-plack.conf > > As time passes, more and more of Koha will use the message queue as well. > > There are many long-running jobs, which would benefit from being placed into > > the background. > > This change is about building capacity and improving performance. > > So we have an architectural reason for a MQ. And the question is: do we want > an optional introduction needing additional fallback code, or dont we > postpone making it mandatory (since it will be one day)? Once it's pushed, koha-testing-docker will want to run it on a separate container for sure.
(In reply to David Cook from comment #262) > (In reply to Julian Maurice from comment #243) > > What about non-package installs ? > > It's probably not possible to automate everything, but a minimal systemd > > service file for the worker would be a good start I think. So when you > > update/install Koha you just have to copy the file to /etc/systemd/system > > and adjust paths/users/.. It has certainly helped me in the past with other > > softwares I installed. > > Can be done in another bug report, but what do you think about this ? > > As I've noted elsewhere, I'm already using RabbitMQ with Koha locally, and I > use a systemd template service definition, which makes it really easy to set > up individual systemd units per Koha instance. > > I'm happy to contribute a community friendly version of this. > > But I think we can do that as a high-priority follow-up. I've opened bug 26363 and assigned it to you ;)
(In reply to Jonathan Druart from comment #269) > The background job processes are running in CGI mode > % grep ProxyPass debian/templates/apache-shared-intranet-plack.conf I know. But it is not the main point of this discussion. > Once it's pushed, koha-testing-docker will want to run it on a separate > container for sure. Yes, I already asked for that. But that does not tell us if we should handle Koha without MQ (where Julian is asking for).
(In reply to Marcel de Rooy from comment #271) > (In reply to Jonathan Druart from comment #269) > > The background job processes are running in CGI mode > > % grep ProxyPass debian/templates/apache-shared-intranet-plack.conf > > I know. But it is not the main point of this discussion. This patchset is about moving these scripts out of CGI mode (bug 15032).
(In reply to Jonathan Druart from comment #272) > (In reply to Marcel de Rooy from comment #271) > > (In reply to Jonathan Druart from comment #269) > > > The background job processes are running in CGI mode > > > % grep ProxyPass debian/templates/apache-shared-intranet-plack.conf > > > > I know. But it is not the main point of this discussion. > > This patchset is about moving these scripts out of CGI mode (bug 15032). We're talking different languages here. Never mind.
Completely other observation: Just noticing on my test server that process beam.smp used quite a bit CPU at some point while there was nothing going on in terms of background processes. Googling beam.smp brings you right to such things. Reason for being more cautious?
(In reply to Marcel de Rooy from comment #274) > Completely other observation: Just noticing on my test server that process > beam.smp used quite a bit CPU at some point while there was nothing going on > in terms of background processes. Googling beam.smp brings you right to such > things. Reason for being more cautious? Looks like it's coming from the client. Did you play with rabbitmq "manually" or only via the Koha interface?
(In reply to Jonathan Druart from comment #275) > (In reply to Marcel de Rooy from comment #274) > > Completely other observation: Just noticing on my test server that process > > beam.smp used quite a bit CPU at some point while there was nothing going on > > in terms of background processes. Googling beam.smp brings you right to such > > things. Reason for being more cautious? > > Looks like it's coming from the client. Did you play with rabbitmq > "manually" or only via the Koha interface? Not exactly sure anymore. Nothing noteworthy, I guess.
(In reply to Marcel de Rooy from comment #274) > Completely other observation: Just noticing on my test server that process > beam.smp used quite a bit CPU at some point while there was nothing going on > in terms of background processes. Googling beam.smp brings you right to such > things. Reason for being more cautious? I'm running RabbitMQ in production on several hosts. Looking at beam.smp right now on an 8 CPU machine, it's steadily using maybe 1% of 1 CPU, although this week it's used a grand total of about 60 minutes of CPU time. I could look into this later but I'm not concerned.
(In reply to David Cook from comment #277) > I'm running RabbitMQ in production on several hosts. Looking at beam.smp > right now on an 8 CPU machine, it's steadily using maybe 1% of 1 CPU, > although this week it's used a grand total of about 60 minutes of CPU time. > > I could look into this later but I'm not concerned. Keep in mind too that polling a database for tasks will use system resources as well. Nothing comes for free.
Created attachment 110150 [details] [review] Bug 22417: Update packaging files
(In reply to Mason James from comment #279) > Created attachment 110150 [details] [review] [review] > Bug 22417: Update packaging files hi Jonathan i've tested a built package here... https://gitlab.com/mjames/ktd-test-pkgs/-/jobs/741583709 $ apt-get -qy install koha-common Setting up koha-common (20.06.00-37~git+20200916083530.1c1076af-1) ... The following plugins have been configured: rabbitmq_stomp Applying plugin configuration to rabbit@runner-z3wu8uu--project-20149864-concurrent-0... The following plugins have been enabled: rabbitmq_stomp set 1 plugins. Offline change; changes will take effect at broker restart. Restarting RabbitMQ Messaging Server: rabbitmq-server. $ rabbitmq-plugins list Configured: E = explicitly enabled; e = implicitly enabled | Status: * = running on rabbit@runner-z3wu8uu--project-20149864-concurrent-0 |/ ... [ ] rabbitmq_sharding 3.7.8 [ ] rabbitmq_shovel 3.7.8 [ ] rabbitmq_shovel_management 3.7.8 [E*] rabbitmq_stomp 3.7.8 [ ] rabbitmq_top 3.7.8 [ ] rabbitmq_tracing 3.7.8 [ ] rabbitmq_web_stomp_examples 3.7.8 Job succeeded
Where are we at with this one now?
Created attachment 111091 [details] [review] Bug 22417: UpdateDB entry TODO: - Add the FK on borrowernumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111092 [details] [review] Bug 22417: DBIX Schema changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111093 [details] [review] Bug 22417: Add Koha::BackgroundJob[s] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111094 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateBiblio Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111095 [details] [review] Bug 22417: Add Koha::BackgroundJob::BatchUpdateAuthority This is not ready yet! Note that this is too close to Koha::BackgroundJob::BatchUpdateBiblio, we will want to refactor bit of code. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111096 [details] [review] Bug 22417: Add GUI to watch the progress of jobs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111097 [details] [review] Bug 22417: Adapt the batch_record_modification tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111098 [details] [review] Bug 22417: Add the ability to cancel a job Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111099 [details] [review] Bug 22417: Fix borrowernumber values Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111100 [details] [review] Bug 22417: Add debian script koha-worker Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111101 [details] [review] Bug 22417: Switch to STOMP apt install rabbitmq-server service rabbitmq-server start rabbitmq-plugins enable rabbitmq_stomp apt install libnet-stomp-perl cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh cp debian/scripts/koha-worker /usr/bin/ koha-worker --start kohadev tail -f /var/log/rabbitmq/* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 22417: Remove useless use statement Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111102 [details] [review] Bug 22417: Handle errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111103 [details] [review] Bug 22417: Try to display pending jobs - only work if worker not started ie. no subscription yet This needs to be fixed (if possible), or removed from the patchset Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111104 [details] [review] Bug 22417: Fix the batch authority tool Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 22417: Add a note about the existence of the DB row Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 22417: (follow-up) Fix the batch authority tool Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111105 [details] [review] Bug 22417: Restore the concept of namespace Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111106 [details] [review] Bug 22417: Restore the 'add to list' feature This feature has been added recently by bug 18127. It highlights the need of a post processing hook. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111107 [details] [review] Bug 22417: Add the new permission manage_background_jobs QA: Please answer the question in admin/background_jobs.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111108 [details] [review] Bug 22417: Send message to /queue This cannot be: /queue/$namespace/$job_type I got: '/koha_kohadev/batch_biblio_record_modification' is not a valid queue destination So keeping the dash in $namespace-$job_type Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111109 [details] [review] Bug 22417: Remove record_type from BatchUpdate* We don't need to pass it to the job, they each know which record type they are dealing with Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111110 [details] [review] Bug 22417: Let owner of a job see the progress A owner of a job should be able to see the progress of this job. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111111 [details] [review] Bug 22417: Remove FIXME in the template Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111112 [details] [review] Bug 22417: Add missing POD and html filters Also remove "authnotrequired => 0," Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111113 [details] [review] Bug 22417: Add new Net::Stomp dependency Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111114 [details] [review] Bug 22417: Fix spelling resizeable vs resizable For the record, codespell does not raise the problem within ktd with 1.17.1 but appears with 1.14 Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111115 [details] [review] Bug 22417: Remove $dbh in Koha::BackgroundJob::* Also remove unused $job_type var Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111116 [details] [review] Bug 22417: Add tests Finally! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111117 [details] [review] Bug 22417: [DO NOT PUSH] Add simple worker and add_job scripts Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111118 [details] [review] Bug 22417: Add rabbitmq status indicator on the about page Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111119 [details] [review] Bug 22417: Remove list of pending jobs This was not accurate as it was only retrieving jobs for batch_biblio_record_modification We will need to improve that later if needed Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111120 [details] [review] Bug 22417: Process the jobs even if the message broker is not reachable Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 111121 [details] [review] Bug 22417: Update packaging files Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 20.11, thanks to everybody involved!
Created attachment 111238 [details] [review] Bug 22417: Add koha-worker to koha-common.install Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Jonathan Druart from comment #314) > Created attachment 111238 [details] [review] [review] > Bug 22417: Add koha-worker to koha-common.install > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master.
Created attachment 111239 [details] [review] Bug 22417: Add exec flag on .t file
(In reply to Jonathan Druart from comment #316) > Created attachment 111239 [details] [review] [review] > Bug 22417: Add exec flag on .t file Pushed to master.
Created attachment 111245 [details] [review] Bug 22417: Remove batch_record_modification from the non-plack list Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Yahoo!
(In reply to Marcel de Rooy from comment #274) > Completely other observation: Just noticing on my test server that process > beam.smp used quite a bit CPU at some point while there was nothing going on > in terms of background processes. Googling beam.smp brings you right to such > things. Reason for being more cautious? Same report on IRC with a possible solution: < kohaputti> seems to be around 4-10% on my machine more or less constantly < kohaputti> yes, it is beam.smp < kohaputti> maybe there is some configration we could change < kohaputti> (4-10% on one core) < kohaputti> just installed the rabbitmq stuff because I noticed reset_all needed it < kohaputti> I have the latest version of Koha master as of ~1 hour < kohaputti> lots of sched_yield() syscalls < kohaputti> when I look with strace < kohaputti> https://www.rabbitmq.com/runtime.html#cpu-reduce-idle-usage < kohaputti> adding RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS="+sbwt none" to the env vars seemed to do the trick < kohaputti> now the load is around 1-2% constantly < Joubu> which version of rabbitmq? which os? < kohaputti> seems to be debian 9 (from koha-testing-docker) < kohaputti> version of rabbitmq-server is 3.6.6-1 Under D11 (koha-testing-docker), RabbitMQ version: 3.8.9 * Without the config modification I see it around 4.5% rabbitmq 1148 4.5 0.5 3227132 86044 ? Sl 13:10 0:37 /usr/lib/erlang/erts-11.1/bin/beam.smp -W w -K true -A 64 -MBas ageffcbf -MHas ageffcbf -MBlmbcs 512 -MHlmbcs 512 -MMmcs 30 -P 1048576 -t 5000000 -stbt db -zdbbl 128000 -B i -- -root /usr/lib/erlang -progname erl -- -home /var/lib/rabbitmq -- -pa -noshell -noinput -s rabbit boot -boot start_sasl -lager crash_log false -lager handlers [] * With the config modification: % echo 'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS="+sbwt none +sbwtdcpu none +sbwtdio none"' >> /etc/rabbitmq/rabbitmq-env.conf % service rabbitmq-server restart It's around 3% rabbitmq 2646 2.9 0.5 3223728 84124 ? Sl 13:25 0:06 /usr/lib/erlang/erts-11.1/bin/beam.smp -W w -K true -A 64 -MBas ageffcbf -MHas ageffcbf -MBlmbcs 512 -MHlmbcs 512 -MMmcs 30 -P 1048576 -t 5000000 -stbt db -zdbbl 128000 -sbwt none -sbwtdcpu none -sbwtdio none -B i -- -root /usr/lib/erlang -progname erl -- -home /var/lib/rabbitmq -- -pa -noshell -noinput -s rabbit boot -boot start_sasl -lager crash_log false -lager handlers [] Not changed much in my case.
(In reply to Jonathan Druart from comment #318) > Created attachment 111245 [details] [review] [review] > Bug 22417: Remove batch_record_modification from the non-plack list > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master!
(In reply to Jonathan Druart from comment #320) > Same report on IRC with a possible solution: > < kohaputti> seems to be around 4-10% on my machine more or less constantly > < kohaputti> yes, it is beam.smp > < kohaputti> maybe there is some configration we could change > < kohaputti> (4-10% on one core) > < kohaputti> just installed the rabbitmq stuff because I noticed reset_all > needed it > < kohaputti> I have the latest version of Koha master as of ~1 hour > < kohaputti> lots of sched_yield() syscalls > < kohaputti> when I look with strace > < kohaputti> https://www.rabbitmq.com/runtime.html#cpu-reduce-idle-usage > < kohaputti> adding RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS="+sbwt none" to the > env vars seemed to do the trick > < kohaputti> now the load is around 1-2% constantly > < Joubu> which version of rabbitmq? which os? > < kohaputti> seems to be debian 9 (from koha-testing-docker) > < kohaputti> version of rabbitmq-server is 3.6.6-1 > I'm using Ubuntu 18.04 and RabbitMQ 3.6.10-1ubuntu0.1 in production. I think Erlang is version 1:20.2.2+dfsg-1ubuntu2 (as per erlang-base)... and erts 9.2. (I think this might be the relevant documentation: http://erlang.org/documentation/doc-9.2/doc/). Environment: USER=rabbitmqHOME=/var/lib/rabbitmqOLDPWD=/ERL_LIBS=/usr/lib/rabbitmq/plugins:/usr/lib/rabbitmq/lib/rabbitmq_server-3.6.10/pluginsEMU=beamPROGNAME=erlRABBITMQ_CONFIG_FILE=/etc/rabbitmq/rabbitmqBINDIR=/usr/lib/erlang/erts-9.2/binLOGNAME=rabbitmqJOURNAL_STREAM=9:2636PATH=/usr/lib/erlang/erts-9.2/bin:/usr/lib/erlang/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/binINVOCATION_ID=e123641da5f6417888a333ac9927f11cLANG=en_AU.UTF-8ROOTDIR=/usr/lib/erlangERL_MAX_ETS_TABLES=50000PWD=/var/lib/rabbitmq Process: /usr/lib/erlang/erts-9.2/bin/beam.smp -W w -A 128 -P 1048576 -t 5000000 -stbt db -zdbbl 32000 -K true -B i -- -root /usr/lib/erlang -progname erl -- -home /var/lib/rabbitmq -- -pa /usr/lib/rabbitmq/lib/rabbitmq_server-3.6.10/ebin -noshell -noinput -s rabbit boot -sname rabbit@server -boot start_sasl -kernel inet_default_connect_options [{nodelay,true}] -sasl errlog_type error -sasl sasl_error_logger false -rabbit error_logger {file,"/var/log/rabbitmq/rabbit@server.log"} -rabbit sasl_error_logger {file,"/var/log/rabbitmq/rabbit@server-sasl.log"} -rabbit enabled_plugins_file "/etc/rabbitmq/enabled_plugins" -rabbit plugins_dir "/usr/lib/rabbitmq/plugins:/usr/lib/rabbitmq/lib/rabbitmq_server-3.6.10/plugins" -rabbit plugins_expand_dir "/var/lib/rabbitmq/mnesia/rabbit@server-plugins-expand" -os_mon start_cpu_sup false -os_mon start_disksup false -os_mon start_memsup false -mnesia dir "/var/lib/rabbitmq/mnesia/rabbit@server" -kernel inet_dist_listen_min 25672 -kernel inet_dist_listen_max 25672 Over 2.5 days, it's used about 1hr 10 minutes of CPU time for processing 0 messages. CPU usage varies but it's more so in the 1-2% range. -- But... using strace I do see a tonne of sched_yield() syscalls. Kind of interesting reading Linus Torvald's 2020 indictment of sched_yield (https://www.realworldtech.com/forum/?threadid=189711&curpostid=189752), and a rude 2016 defense of sched_yield by someone http://erlang.org/pipermail/erlang-questions/2016-March/088232.html. I've tried adding the following to /etc/rabbitmq/rabbitmq-env.conf as per https://www.rabbitmq.com/runtime.html#cpu-reduce-idle-usage but it caused RabbitMQ to fail to start up: SERVER_ADDITIONAL_ERL_ARGS="+sbwt none +sbwtdcpu none +sbwtdio none" However, the server would start with the following: SERVER_ADDITIONAL_ERL_ARGS="+sbwt none" Now I'm not seeing any sched_yield syscalls via strace. I do still see a lot of clock_gettime, timerfd_settime, epoll_wait (with no timeout) and futex. Not sure why it's checking the time so much. It looks to me like Erlang isn't actually sleeping the schedulers, despite it saying it puts schedulers to sleep if +sbwt is none. I'm guessing the clock_gettime is used to monitor each thread to make sure it's not trying to do too much, which is a far cry from sleeping... But overall, I'd say CPU usage is about 0-1% now. It may be worth noting that RabbitMQ says "The runtime assumes that it does not share CPU resources with other tools or tenants. When that's the case, the scheduling mechanism used can become very inefficient and result in significant (up to several orders of magnitude) latency increase for certain operations. This means that in most cases colocating RabbitMQ nodes with other tools or applying CPU time slicing is highly discouraged and will result in suboptimal performance." at https://www.rabbitmq.com/runtime.html#cpu-contention Overall, I'm still not concerned.
Reading through http://erlang.org/pipermail/erlang-questions/2015-September/085799.html took me to https://github.com/erlang/otp/blob/master/erts/lib_src/pthread/ethr_event.c#L65-L161 There are the futex and monotonic time gets I reckon. And maybe the scheduler event loop: https://github.com/erlang/otp/blob/master/erts/emulator/beam/erl_process.c#L3347 I tried adding "+scl false" to see if that would reduce the load generated while waiting, but no change. I'm half tempted to use "+S" to limit the number of schedulers created, which seems equivalent to Go's GOMAXPROCS, but I doubt it's worth fiddling too much. This is a topic that I've encountered when writing my own schedulers and reading about Go's scheduler(s). Unless you sleep, you're going to generate some load, as you're constantly checking if you have work to do. Of course, if you sleep, then you create a chance for latency. Although I think most schedulers check for I/O and timers, and (micro) sleep if there's no I/O, no timer, and no work to do. If you do a strace gainst Apache httpd, you'll see that it's using a select(null, null, null, 1) as an alternative to sleep (like the one mentioned in Perl at https://perldoc.perl.org/functions/select). An alternative to select would be to use epoll (which Erlang does use although with a 0 second timeout), but it seems like Erlang instead does some other work while waiting for work. I'm not an expert on programming language internals especially not ones with focuses on concurrency. Anyway, I don't think RabbitMQ using some system resources is a big drama.
As we all know, I'm insatiably curious. Looks like the switches +sbwtdcpu, +sbwtdio, +swtdcpu, and +swtdio were added in Erts 10.0, but I think we're using Erts 9.2 (at least in Ubuntu 18.04). Although it looks like +sbwt should apply in lieu of them anyway I think. I'm actually half-tempted to email RabbitMQ folks about this...
(In reply to David Cook from comment #323) > Anyway, I don't think RabbitMQ using some system resources is a big drama. Keep in mind that some libraries might not have the luxury of sufficient funds to allocate the resources that most of us do have. If the load is just the result of the waiting/polling, the thing is really badly programmed ;)
(In reply to Marcel de Rooy from comment #325) > Keep in mind that some libraries might not have the luxury of sufficient > funds to allocate the resources that most of us do have. > That is true. I would argue that Elasticsearch requires resources, but we still have Zebra for less well resourced libraries. Technically speaking, Koha doesn't require RabbitMQ. Rather, it just requires a message queue that can use the STOMP protocol. So we could always recommend RabbitMQ for well resourced libraries, but provide/recommend a smaller alternative. A quick Google suggests ocamlmq as a lightweight option. Apache ActiveMQ is embedded in DSpace and Fedora Commons I believe, but we could use a standalone instance. Here's a list of STOMP compliant servers: https://stomp.github.io/implementations.html > If the load is just the result of the waiting/polling, the thing is really > badly programmed ;) I think that's a bit presumptuous. In the Koha community, I think that we should endeavour to be open-minded about technology. After chatting with some RabbitMQ folk and briefly studying Erlang, it appears that the Erlang BEAM virtual machine uses "busy waiting" to stay in a state of readiness, so that it's able to start new work as quickly as possible. This helps RabbitMQ to achieve high throughput on heavy workloads. So I wouldn't say that it is "really badly programmed". Rather, it's a deliberate programming choice designed for high performance computing (HPC). I think that we should be careful not to mistake our own local perspectives for absolute truth. That being said, I'm not the most trusting person, and don't like to accept what people tell me without some evidence. While using "+sbwt none" does reduce the CPU usage and reduce the number of syscalls, it appears to me that when using RabbitMQ 3.6.10-1ubuntu0.1 with the Erlang Run-Time System (ERTS) 9.2 on Ubuntu 18.04, the Erlang schedulers are still "busy waiting", despite what the RabbitMQ and Erlang documentation say. I'm able to demonstrate that using "top" and "strace". Someone on the RabbitMQ listserv pointed out that I was using outdated tech, and https://www.rabbitmq.com/install-debian.html does point out that the software in the Debian and Ubuntu repositories is often quite out-of-date (just like Zebra) and suggests installing from their own sources. I'd be interested to see if newer versions of RabbitMQ and Erlang do as much busy waiting. I might do some quick Docker-based experiments to see if anything obvious jumps out.
I just remembered that I have a RabbitMQ on an openSUSE Leap 15.1 server. It's running RabbitMQ 3.7.14 with Erlang 20.3.8.15 and Erts 9.3.3.6 with 12 CPUs. I'm not using +sbwt on that system, but my constant CPU usage is 0%. The 1_scheduler thread is still doing a constant stream of syscalls (epoll_pwait, clock_gettime, futex), but I don't see it reflected in CPU usage via top. The "aux" thread is also doing something a little bit but rarely. Looking back again at Ubuntu 18.04 with RabbitMQ 3.6.10 with Erlang 20.2.2 and Erts 9.2 with 8 CPUs, I can see that all the scheduler threads are spinning a bit. I'm curious if it's due to Erlang/Erts/RabbitMQ differences, or configuration differences. I'm going to study the configuration options a bit. -- As for Docker... I'm running rabbitmq:3.6.10 and CPU usage is typically 0% and occasionally spikes to 2% on a 2 CPU system. It's not as frequent as the Ubuntu system but more frequent than the openSUSE system. I'd say on openSUSE it stays at 0% for 10 seconds before moving up to maybe 0.333% for .5 seconds. On Ubuntu 18.04, it sits at 0% but every 1-2 seconds it'll jump up to 2%. On Docker rabbitmq:3.6.10, it looks like it goes up to 2% every 2-10 seconds. -- Trying Docker rabbitmq:3.8.9... it goes to 2% CPU every every 2-5 seconds it seems. Like openSUSE 3.7.14, it seems to be 1_scheduler thread that is doing the majority of the spinning. When I use strace for it, I do see a huge number of sched_yield syscalls and a much smaller number of futex syscalls. I'm going to try using "+sbwt none" on another container and see how it goes. (I'm also capturing the CLI conf to see if there's anything obvious there.)
Actually trying that new Docker image with the following: RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS="+sbwt none +sbwtdcpu none +sbwtdio none" That definitely got rid of the sched_yield syscalls. Lots of futex calls from 1_scheduler thread. The aux thread is doing some other work. Based on https://www.rabbitmq.com/runtime.html#thread-stats, I'd say that's probably doing things like managing timers and doing other management work.
Ok some numbers... The Ubuntu 18.04 container running RabbitMQ 3.8.9 used 3x less CPU time while idling compared to a Ubuntu 18.04 virtual machine running RabbitMQ 3.6.10. The openSUSE Leap 15.1 virtual machine running RabbitMQ 3.7.14 used 2x less CPU time while idling compared to the Ubuntu 18.04 container. So, my recommendation would be the same as the people from RabbitMQ, and that is that it would be best to install the latest version of RabbitMQ available, as that's probably going to result in the best performance. I don't feel like meddling with my Ubuntu 18.04 virtual machine today, but I might have a look at that, although I'm not concerned with the small amount of CPU usage/CPU time it's using, so I might not bother. Watching the beam.smp process over time, even on the Ubuntu 18.04 virtual machine, it's using a max of 2% CPU for fractions of a second every few seconds. It's a small amount of CPU time over a fairly large amount of real time. It's not competing with other processes for CPU. It's not like a process that is sitting constantly at 100% or even 50%. That said, maybe I should have a look at koha-testing-docker one more time...
Ok I'm using the vanilla koha-testing-docker on Debian 9 (stretch), so I have RabbitMQ 3.6.6 with Erlang 19.2.1 and Erts 8.2.1. Very old now... Even without "+sbwt none", my usage is sitting at 0% and spiking to 2% every 1-5 real seconds (typically more like every 5 seconds). It's actually using less CPU time than my Ubuntu 18.04 VM. Note that I'm using koha-testing-docker via Docker Desktop in Windows, so my Docker host is actually a Hyper-V virtual machine. Popping "SERVER_ADDITIONAL_ERL_ARGS="+sbwt none"" into /etc/rabbitmq/rabbitmq-env.conf and restarting the rabbitmq server... Yeah I'd say a 2% spike every 5 seconds for a fraction of a second. Going into "top" and pressing "H", I see "1_scheduler", "2_scheduler", and "aux" threads very occasionally having a little burst. But it's by no means constant. It's actually idling very nicely.
Thx David for your thorough investigations. And note that my 'badly programmed' statement is really in an if-block embedded :) We never reach that point IF the condition is not true..
In light of Jonathan's "Koha 20.11 release dates" email, how are we all going in regards to the packaging files? Do we really want rabbitmq-server to be a required dependency for the packaged Koha? Did we decide that koha-common was going to be the "comprehensive/full" Koha? For the time being, I'm OK running RabbitMQ collocated with Koha, but in the long-term I'm going to want to run it on a separate server. I suppose at this point any changes in terms of packaging and configuration could be handled in the future... Overall, I'm excited to see this change go in.
Btw, for those concerned about RabbitMQ CPU usage, I have been going down the rabbit hole a bit in the RabbitMQ and Erlang worlds. Gerhard Lazu from Pivotal has added some comments on the erlang-questions listserv: http://erlang.org/pipermail/erlang-questions/2020-October/100035.html Based on his message I think the CPU usage that I am seeing is probably due to stats collection. I'm waiting to hear back from him, but I think the CPU usage I have seen could be reduced by changing the stats collection interval from 5 seconds to say 30-60 seconds as per https://www.rabbitmq.com/networking.html#tuning-for-large-number-of-connections-cpu-footprint
(In reply to David Cook from comment #332) > In light of Jonathan's "Koha 20.11 release dates" email, how are we all > going in regards to the packaging files? > > Do we really want rabbitmq-server to be a required dependency for the > packaged Koha? > > Did we decide that koha-common was going to be the "comprehensive/full" > Koha? > > For the time being, I'm OK running RabbitMQ collocated with Koha, but in the > long-term I'm going to want to run it on a separate server. > > I suppose at this point any changes in terms of packaging and configuration > could be handled in the future... David, that would be awesome if you could summarize the discussion we had "privately" between some of us and ask koha-devel their opinion/continue the discussion there.
(In reply to Jonathan Druart from comment #334) > David, that would be awesome if you could summarize the discussion we had > "privately" between some of us and ask koha-devel their opinion/continue the > discussion there. I'll do my best!
As we get closer to release time, are we sure that we want to require rabbitmq-server in the debian/control file? It seems to me that it would be best to suggest it but not require it. If people do have issues with RabbitMQ, it would be better if they can relocate the RabbitMQ server off the Koha server.
new feature will not be backported to 20.05.x
(In reply to David Cook from comment #336) > As we get closer to release time, are we sure that we want to require > rabbitmq-server in the debian/control file? It seems to me that it would be > best to suggest it but not require it. > > If people do have issues with RabbitMQ, it would be better if they can > relocate the RabbitMQ server off the Koha server. hi David, i think a better option is to 'recommend' the rabbitmq package this means the package gets installed by default unless specified - and the package can also be removed if required
(In reply to Mason James from comment #338) > hi David, i think a better option is to 'recommend' the rabbitmq package > > this means the package gets installed by default unless specified - and the > package can also be removed if required I'm happy to update Bug 26741 along those lines. When you say "unless specified", are you referring to using a preference file in /etc/apt/preferences.d?
(In reply to David Cook from comment #339) > (In reply to Mason James from comment #338) > > hi David, i think a better option is to 'recommend' the rabbitmq package > > > > this means the package gets installed by default unless specified - and the > > package can also be removed if required > > I'm happy to update Bug 26741 along those lines. > > When you say "unless specified", are you referring to using a preference > file in /etc/apt/preferences.d? hi, no no, i mean installing koha-common using... $ sudo apt install --no-install-recommends koha-common
(In reply to Mason James from comment #340) > hi, no no, i mean installing koha-common using... > > $ sudo apt install --no-install-recommends koha-common Mmm, I like that, although does that apply only to the koha-common package, or does it cascade down to koha-common's dependencies too?
Recently, I have been noticing issues on a Ubuntu 18.04 server where RabbitMQ has been running. Every few days, there would be CPU soft locks and the server would hang. Restarting it would resolve the problem for a few days before it would happen again. After disabling RabbitMQ, the problem appears to have gone away. This suggests the problem was with RabbitMQ running collocated on the same server as Koha (with 100+ Starman processes running). Now that version of RabbitMQ was from the Ubuntu 18.04 apt repositories, and it is fairly old. The RabbitMQ website's own recommendation is to use newer versions of RabbitMQ as per https://www.rabbitmq.com/install-debian.html. I will be trying https://bintray.com/rabbitmq/debian/rabbitmq-server shortly.
(In reply to David Cook from comment #342) > The RabbitMQ website's own recommendation is to use newer versions of > RabbitMQ as per https://www.rabbitmq.com/install-debian.html. > > I will be trying https://bintray.com/rabbitmq/debian/rabbitmq-server shortly. A modified version of https://www.rabbitmq.com/install-debian.html#apt-bintray-quick-start: curl -fsSL https://github.com/rabbitmq/signing-keys/releases/download/2.0/rabbitmq-release-signing-key.asc | sudo apt-key add - apt-get install apt-transport-https /etc/apt/sources.list.d/rabbitmq.list: deb https://dl.bintray.com/rabbitmq-erlang/debian bionic erlang deb https://dl.bintray.com/rabbitmq/debian bionic main apt-get update apt-get install rabbitmq-server
I'm going to be testing out this version of RabbitMQ over the next few weeks on a few different servers. Note that I have no issues running an older version of RabbitMQ on openSUSE Leap 15.1.
(In reply to David Cook from comment #344) > I'm going to be testing out this version of RabbitMQ over the next few weeks > on a few different servers. > > Note that I have no issues running an older version of RabbitMQ on openSUSE > Leap 15.1. No issues running this on a backup server. Just started running the latest RabbitMQ on a prod Ubuntu 18.04 as per the aforementioned installation steps. I am using "+sbwt none" and very little CPU usage. I could add those other anti-busy wait arguments, but I'm not too concerned at the moment.
By the way, AWS have just launched a managed RabbitMQ service: https://aws.amazon.com/blogs/aws/amazon-mq-update-new-rabbitmq-message-broker-service/ I haven't tried it but people might find that useful, especially if/when Bug 26742 is pushed.
Anyone having seen the rabbitmq-server failing to startup in a Docker container after a forced stop of the container ?
(In reply to Marcel de Rooy from comment #347) > Anyone having seen the rabbitmq-server failing to startup in a Docker > container after a forced stop of the container ? Nope not at all. Can you provide a screenshot maybe?
(In reply to David Cook from comment #348) > (In reply to Marcel de Rooy from comment #347) > > Anyone having seen the rabbitmq-server failing to startup in a Docker > > container after a forced stop of the container ? > > Nope not at all. Can you provide a screenshot maybe? No. It would just freeze at trying to start the service.