Bug 35655 - Make it possible to switch off RabbitMQ without any warns in logs/about page
Summary: Make it possible to switch off RabbitMQ without any warns in logs/about page
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 35092
  Show dependency treegraph
 
Reported: 2023-12-27 14:30 UTC by Katrin Fischer
Modified: 2024-11-18 22:17 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 35655: Add 'JobsNotificationMethod' system preference (3.72 KB, patch)
2024-09-12 15:05 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (2.10 KB, patch)
2024-09-12 15:05 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (3.85 KB, patch)
2024-09-14 09:28 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (4.55 KB, patch)
2024-09-14 11:50 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (4.55 KB, patch)
2024-09-14 12:00 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (4.73 KB, patch)
2024-09-14 12:03 UTC, Mark Hofstetter
Details | Diff | Splinter Review
full patch (6.26 KB, patch)
2024-09-14 12:10 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (2.15 KB, patch)
2024-09-14 12:59 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (3.90 KB, patch)
2024-09-14 12:59 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (4.78 KB, patch)
2024-09-14 12:59 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (5.34 KB, patch)
2024-09-14 13:03 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35655: Add 'JobsNotificationMethod' system preference (3.79 KB, patch)
2024-09-26 19:17 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (5.40 KB, patch)
2024-09-26 19:17 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 35655: Add unit tests (2.05 KB, patch)
2024-09-26 19:17 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 35655: Add 'JobsNotificationMethod' system preference (3.83 KB, patch)
2024-09-27 19:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so (5.45 KB, patch)
2024-09-27 19:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35655: Add unit tests (2.10 KB, patch)
2024-09-27 19:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35655: (follow-up) Fix perlcritic error (882 bytes, patch)
2024-09-30 16:53 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2023-12-27 14:30:51 UTC
A recent survey on IRC revealed that a lot of people are still running Koha without RabbitMQ. It's not a small dependency and for smaller libraries it might not be needed ever. Also we still see issues with jobs not being processed correctly that has people turning it off.

We should not create any warns or logs, but have a proper switch to turn RabbitMQ off without Koha complaining until we can fully rely on RabbitMQ at least.
Comment 1 Magnus Enger 2023-12-29 07:43:03 UTC
+1 We have turned RabbitMQ off on a lot of sites, because things work better without it.
Comment 2 Marcel de Rooy 2023-12-29 08:28:42 UTC
See the discussion on 35092. And the several options suggested. Your feedback is appreciated.
Comment 3 Katrin Fischer 2023-12-29 08:36:22 UTC
(In reply to Marcel de Rooy from comment #2)
> See the discussion on 35092. And the several options suggested. Your
> feedback is appreciated.

Thanks Marcel, for pointing this one out. I am all for fixing the remaining issues, but I feel we need a solution for stable installations until and I also see a use case for very small installations as RabbitMQ is taking additional resources.
Comment 4 davewood 2024-05-02 10:05:52 UTC
I just ran into problems with background jobs again (koha-common/stable,now 23.11.04-4 all [installed])

There has been a discussion regarding RabbitMQ at the workshop in Marseille.

I still think we would be better off without rabbitMQ.

+ no parallel solution (DBpoll+rabbitmq)
+ no extra service that has to be maintained (sysop)

and i dont see the benefits rabbitmq brings.

if i knew for sure the DB poll mechanism is here to stay i would love to have a config param to disable rabbitmq. and not have debian pull in the rabbitmq dependency.
Comment 5 Marcel de Rooy 2024-05-03 06:38:05 UTC
(In reply to davewood from comment #4)
Thx for your feedback. See also bug 35092.

> I just ran into problems with background jobs again (koha-common/stable,now
> 23.11.04-4 all [installed])

Could you provide some more details here? What are those problems? Are they related to insufficient resources? Or ... ?
Comment 6 Tomás Cohen Arazi (tcohen) 2024-09-12 15:05:15 UTC
Created attachment 171412 [details] [review]
Bug 35655: Add 'JobsNotificationMethod' system preference
Comment 7 Tomás Cohen Arazi (tcohen) 2024-09-12 15:05:18 UTC
Created attachment 171413 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) enqueue step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8. Repeat 2-4
=> FAIL: You get the same errors, because it defaults to STOMP and it is
not available
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all
10. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12. Repeat 2-4
=> SUCCESS: No warnings about STOMP
13. Sign off :-D

Dedicated to Liz
Comment 8 Tomás Cohen Arazi (tcohen) 2024-09-12 15:07:37 UTC
Ok, I'm putting this patches here to start the conversation. This shouldn't affect an implementation of the 'Centralize code from workers' bug, but aligns with it.

Let's get the ball rolling on this one, as it is hurting us right now.
Comment 9 Tomás Cohen Arazi (tcohen) 2024-09-12 15:09:20 UTC
Note: I haven't touched the workers code, so they would still show a warning on startup. If there's a minimum consensus on going this route, I will provide a patch for that (super trivial actually) or happily sign off on other's follow-ups.
Comment 10 Mark Hofstetter 2024-09-14 07:12:09 UTC
first of all thx for getting this started - and it seems to work as intended (ie no stomp messages)

but minor notices/questions

11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all

+ checking "about" page (cgi-bin/koha/about.pl) BEFORE stopping rabbitmq
still says "Using RabbitMQ" which is unexpected from my POV, because if I switch it of I want to stop using it regardless whether it's running or not

+ if you do a stop rabbitmq => "Using SQL polling"

+ a change of a "system preference" which requires a restart?

+ related: is there an easy way to find out how a background job was started?
Comment 11 Mark Hofstetter 2024-09-14 09:28:50 UTC
Created attachment 171503 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8. Repeat 2-4
=> FAIL: You get the same errors, because it defaults to STOMP and it is
not available
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all
10. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
11a. check about page (<hostname>/cgi-bin/koha/about.pl) says "Message
broker: 	Using SQL polling"
12. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D
Comment 12 Mark Hofstetter 2024-09-14 11:36:33 UTC
I did a reflow of the code so that the "JobsNotificationMethod" is checked only once at connect time, so that a warning is only emitted when you actually WANT to connect to rabbitmq but it is not working.

I also fixed the about page which now has three states:
+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ
Comment 13 Mark Hofstetter 2024-09-14 11:50:16 UTC
Created attachment 171504 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D
Comment 14 Mark Hofstetter 2024-09-14 12:00:27 UTC
Created attachment 171505 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D
Comment 15 Mark Hofstetter 2024-09-14 12:03:36 UTC
Created attachment 171506 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D
Comment 16 Mark Hofstetter 2024-09-14 12:10:37 UTC
Created attachment 171507 [details] [review]
full patch

I hate bugzilla patches guts 

git diff main > patch
Comment 17 Katrin Fischer 2024-09-14 12:35:47 UTC
Hi Mark, the last patch won't apply with our tools (git bz).
If you are using koha-testing-docker, you want to do:

1) sign off on the patches: git so 2
2) git bz attach -e HEAD~2..

If you are not using ktd or git bz is not set up correctly, something like git format-patch will do the trick to create patch files that have the right headers the diff file is missing.
Comment 18 Katrin Fischer 2024-09-14 12:36:19 UTC
... was missing the bug number: git bz attach -e 35655 HEAD~2..
Comment 19 Katrin Fischer 2024-09-14 12:36:50 UTC
Any changes to the original patch set should be done in a follow-up commit/separate patch file.
Comment 20 Mark Hofstetter 2024-09-14 12:59:10 UTC
Created attachment 171509 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D
Comment 21 Mark Hofstetter 2024-09-14 12:59:13 UTC
Created attachment 171510 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8. Repeat 2-4
=> FAIL: You get the same errors, because it defaults to STOMP and it is
not available
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all
10. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
11a. check about page (<hostname>/cgi-bin/koha/about.pl) says "Message
broker: 	Using SQL polling"
12. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D

Signed-off-by: Mark Hofstetter <mark@hofstetter.at>
Comment 22 Mark Hofstetter 2024-09-14 12:59:16 UTC
Created attachment 171511 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D

Signed-off-by: Mark Hofstetter <mark@hofstetter.at>
Comment 23 Mark Hofstetter 2024-09-14 13:03:03 UTC
Created attachment 171512 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D
Comment 24 Kyle M Hall (khall) 2024-09-26 19:17:34 UTC
Created attachment 172027 [details] [review]
Bug 35655: Add 'JobsNotificationMethod' system preference

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall (khall) 2024-09-26 19:17:42 UTC
Created attachment 172028 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall (khall) 2024-09-26 19:17:45 UTC
Created attachment 172029 [details] [review]
Bug 35655: Add unit tests

Add the ability to specify STOMP connection info via environment
variables and utilize those in unit tests.
Comment 27 Martin Renvoize (ashimema) 2024-09-27 19:13:17 UTC
Created attachment 172157 [details] [review]
Bug 35655: Add 'JobsNotificationMethod' system preference

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize (ashimema) 2024-09-27 19:13:20 UTC
Created attachment 172158 [details] [review]
Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize (ashimema) 2024-09-27 19:13:23 UTC
Created attachment 172159 [details] [review]
Bug 35655: Add unit tests

Add the ability to specify STOMP connection info via environment
variables and utilize those in unit tests.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Tomás Cohen Arazi (tcohen) 2024-09-30 11:20:18 UTC
(In reply to Mark Hofstetter from comment #12)
> I did a reflow of the code so that the "JobsNotificationMethod" is checked
> only once at connect time, so that a warning is only emitted when you
> actually WANT to connect to rabbitmq but it is not working.


Excelsior!
Comment 31 Martin Renvoize (ashimema) 2024-09-30 14:33:53 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 32 Tomás Cohen Arazi (tcohen) 2024-09-30 16:53:28 UTC
Created attachment 172232 [details] [review]
Bug 35655: (follow-up) Fix perlcritic error

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Lucas Gass (lukeg) 2024-11-18 22:17:12 UTC
Enhancement won't be backported to 24.05.x