Bug 33044 - BackgroundJob enqueue does not return the job id if rabbit is unreachable
Summary: BackgroundJob enqueue does not return the job id if rabbit is unreachable
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-22 18:08 UTC by Nick Clemens
Modified: 2023-12-13 17:05 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.04,22.05.11,21.11.19


Attachments
Bug 33044: Return job id if there is no connection to rabbit (1.19 KB, patch)
2023-02-22 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33044: Return job id if there is no connection to rabbit (1.24 KB, patch)
2023-03-02 19:40 UTC, David Nind
Details | Diff | Splinter Review
Bug 33044: Return job id if there is no connection to rabbit (1.47 KB, patch)
2023-03-03 08:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2023-02-22 18:08:21 UTC
Some sites elect to use the worker without rabbit

This largely works, however, when a job is enqueued we don't return the job_id unless our connection was successful - this leads to the enqueued result page not providing a correct link, even though the job will be processed by DB polling

To recreate:
1 - In KTD: sudo service rabbitmq-server stop
2 - sudo koha-worker --stop kohadev; sudo koha-worker --queue long_tasks --stop kohadev
3 - In staff client enqueue a batch item modification - note the link to view the enqueued job has no id in the url and takes you to the main background jobs viewer
Comment 1 Nick Clemens 2023-02-22 18:15:14 UTC
Created attachment 147178 [details] [review]
Bug 33044: Return job id if there is no connection to rabbit

This patch simply ensures we returnt he job id even if we don't send the
job to rabbit

To test:
1 - In KTD: sudo service rabbitmq-server stop
2 - sudo koha-worker --stop kohadev; sudo koha-worker --queue long_tasks --stop kohadev
3 - In staff client enqueue a batch item modification - note the link to view the enqueued job has no id in the url and takes you to the main background jobs viewer
4 - Apply patch
5 - Restart all
6 - Enqueue a new batch item modification
7 - Confirm the link works and has the correct id
Comment 2 David Nind 2023-03-02 19:40:48 UTC
Created attachment 147670 [details] [review]
Bug 33044: Return job id if there is no connection to rabbit

This patch simply ensures we returnt he job id even if we don't send the
job to rabbit

To test:
1 - In KTD: sudo service rabbitmq-server stop
2 - sudo koha-worker --stop kohadev; sudo koha-worker --queue long_tasks --stop kohadev
3 - In staff client enqueue a batch item modification - note the link to view the enqueued job has no id in the url and takes you to the main background jobs viewer
4 - Apply patch
5 - Restart all
6 - Enqueue a new batch item modification
7 - Confirm the link works and has the correct id

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Marcel de Rooy 2023-03-03 08:10:58 UTC
Looking here
Comment 4 Marcel de Rooy 2023-03-03 08:21:40 UTC
Created attachment 147682 [details] [review]
Bug 33044: Return job id if there is no connection to rabbit

This patch simply ensures we return the job id even if we couldn't send
the job to rabbit.

To test:
1 - In KTD: sudo service rabbitmq-server stop
2 - sudo koha-worker --stop kohadev; sudo koha-worker --queue long_tasks --stop kohadev
3 - In staff client enqueue a batch item modification - note the link to view the enqueued job has no id in the url and takes you to the main background jobs viewer
4 - Run t/db_dependent/Koha/BackgroundJob.t and notice that it fails.
5 - Apply patch
6 - Restart all
7 - Enqueue a new batch item modification
8 - Confirm the link works and has the correct id
9 - Run t/db_dependent/Koha/BackgroundJob.t; it should pass now.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2023-03-03 08:40:26 UTC
The patch is simple and works.
But I feel that we are on the crossroad here: Should we facilitate working without rabbitmq any further?

Should we put more time in getting it stable in Koha? Is the plugin we use the cause of that? Bad configuration? Many people run it and it should be able to handle millions of messages.. The volume of messages in Koha is not expected to be that large.

The hybrid solution for background jobs feels like 'suboptimal' design to me. The current state allows for strange sync issues: you can submit jobs without rabbitmq; if the worker is running and rabbit is not, they will be picked up. If the worker was not running, and you restart rabbit and worker, these jobs will be ignored since they are not in rabbit.

PQA could have been In discussion too :)
Comment 6 Marcel de Rooy 2023-03-03 08:46:17 UTC
Just for reference about connectivity issues:

https://www.rabbitmq.com/heartbeats.html
Comment 7 David Cook 2023-03-05 23:00:20 UTC
(In reply to Marcel de Rooy from comment #5)
> The patch is simple and works.
> But I feel that we are on the crossroad here: Should we facilitate working
> without rabbitmq any further?

Imho, we should focus on it just working with rabbitmq.
 
> Should we put more time in getting it stable in Koha? Is the plugin we use
> the cause of that? Bad configuration? Many people run it and it should be
> able to handle millions of messages.. The volume of messages in Koha is not
> expected to be that large.

I'm curious about this as well. I use RabbitMQ for a different system and I process hundreds of thousands of messages a day (usually in bursts of several per second) with zero problems. 

I do use the AMQP for that one instead of the STOMP but I'm not sure what people's problem with RabbitMQ is...
Comment 8 Nick Clemens 2023-03-06 12:28:39 UTC
(In reply to David Cook from comment #7)
> (In reply to Marcel de Rooy from comment #5)
> Imho, we should focus on it just working with rabbitmq.
>

I agree, Rabbit should be the focus, but as long as this code is still there, we should make it work :-)
  
> > Should we put more time in getting it stable in Koha? Is the plugin we use
> > the cause of that? Bad configuration? Many people run it and it should be
> > able to handle millions of messages.. The volume of messages in Koha is not
> > expected to be that large. I'm curious about this as well. I use RabbitMQ for a different system and I process hundreds of thousands of messages a day (usually in bursts of several per second) with zero problems. 

I think we are much closer - with the initial implementation we had may problems in production (connection failing, jobs being lost, things getting 'stuck') so we went back to using the DB polling

> I do use the AMQP for that one instead of the STOMP but I'm not sure what
> people's problem with RabbitMQ is...

Our goal is to run rabbit in production, and we are working on moving all the bugs to address our issues forward too. We are waiting for the next stable release, and then will begin switching back and seeing how things go
Comment 9 Tomás Cohen Arazi 2023-03-06 12:49:10 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 10 David Cook 2023-03-06 23:02:54 UTC
(In reply to Nick Clemens from comment #8)
> Our goal is to run rabbit in production, and we are working on moving all
> the bugs to address our issues forward too. We are waiting for the next
> stable release, and then will begin switching back and seeing how things go

Certainly grateful for Bywater kicking the tires/tyres!
Comment 11 Jacob O'Mara 2023-03-07 18:28:12 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 12 Lucas Gass 2023-03-10 16:32:31 UTC
Backported to 22.05.x for upcoming 22.05.11
Comment 13 Arthur Suzuki 2023-03-29 13:13:03 UTC
applied to 21.11 for 21.11.19
Comment 14 wainuiwitikapark 2023-04-27 05:38:47 UTC
Not backported to 21.05.x