[Bug 40820] New: STOMP errors even when JobsNotificationMethod='polling'
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Bug ID: 40820 Summary: STOMP errors even when JobsNotificationMethod='polling' Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org We added the JobsNotificationMethod preference so people can disable the use of RabbitMQ explicitly and not get the logs flooded with connection errors. But a patch on bug 34070 reintroduced the warn ♪┏(・o・)┛♪ e2e587273c14f092d71046f21266473445b219c5 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |34070 Status|NEW |ASSIGNED CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34070 [Bug 34070] background_jobs_worker.pl floods logs when it gets error frames -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186506&action=edit Bug 40820: Fix STOMP connection warnings when JobsNotificationMethod is 'polling' The background job workers are showing STOMP/RabbitMQ connection error warnings even when the JobsNotificationMethod system preference was set to 'polling', which doesn't require a message broker connection. This patch adds checks for the JobsNotificationMethod preference before displaying connection warnings, so warnings only appear when STOMP is actually expected to be available. Changes: - misc/workers/background_jobs_worker.pl: Check syspref before warning - misc/workers/es_indexer_daemon.pl: Check syspref before warning Test plan: 1. Set JobsNotificationMethod to 'polling' 2. Run background_jobs_worker.pl - should not show STOMP warnings 3. Run es_indexer_daemon.pl - should not show STOMP warnings 4. Set JobsNotificationMethod to 'STOMP' without RabbitMQ running 5. Run workers - should show appropriate STOMP connection warnings 6. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186507&action=edit Bug 40820: (follow-up) es_indexer_daemon.pl missed on 34070 The es_indexer_daemon.pl was missing handling for the case where Koha::BackgroundJob->connect() returns undef without throwing an exception (which happens when connection fails silently). This adds the missing warning for that case, matching the behavior in background_jobs_worker.pl. Test plan: 1. Set JobsNotificationMethod to 'STOMP' 2. Configure invalid STOMP settings that cause connect() to return undef 3. Run es_indexer_daemon.pl - should show connection warning 4. Verify jobs are still processed via database polling Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, | |kyle@bywatersolutions.com, | |michael.adamyk@bywatersolut | |ions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |35655 CC| |dcook@prosentient.com.au Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35655 [Bug 35655] Make it possible to switch off RabbitMQ without any warns in logs/about page -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #0)
We added the JobsNotificationMethod preference so people can disable the use of RabbitMQ explicitly and not get the logs flooded with connection errors.
But a patch on bug 34070 reintroduced the warn ♪┏(・o・)┛♪
It took me a few minutes to wrap my head around this one but I see it now. Bug 34070 added the following: '$error ||= "Cannot connect to the message broker, the jobs will be processed anyway" unless $conn;' I didn't even know about bug 35655 until this moment.
e2e587273c14f092d71046f21266473445b219c5
I don't get anything for that commit hash? -- At a glance I think these patches should make sense... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to David Cook from comment #3)
(In reply to Tomás Cohen Arazi (tcohen) from comment #0)
We added the JobsNotificationMethod preference so people can disable the use of RabbitMQ explicitly and not get the logs flooded with connection errors.
But a patch on bug 34070 reintroduced the warn ♪┏(・o・)┛♪
It took me a few minutes to wrap my head around this one but I see it now. Bug 34070 added the following: '$error ||= "Cannot connect to the message broker, the jobs will be processed anyway" unless $conn;'
I didn't even know about bug 35655 until this moment.
e2e587273c14f092d71046f21266473445b219c5
I don't get anything for that commit hash?
In a hurry, I just grabbed the hash from the submitted patch on bug 34070 https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182574 but you guessed right. I believe 34070 patches were wrong because they treated the 'skip if polling' behavior as an error by not acknowledging bug 35655. The code has been there for a good while and this caused a regression.
At a glance I think these patches should make sense...
The patches are trivial and just wrap things around a check on JobsNotificationMethod='STOMP' which is also correct. But if we don't call ->connect() unless STOMP, then we don't need the code handling non-STOMP in ->connect. So any of the refactoring efforts should clear this for good I'd say. I might try to resurrect those patches... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- We need to improve the logging here. We actually do not log the $error from the workers because ->connect does not rethrow the exception. Cannot connect to broker (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #5)
We need to improve the logging here.
We actually do not log the $error from the workers because ->connect does not rethrow the exception.
Cannot connect to broker (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.
Sorry, forget that. It's the point of "Bug 34070: Display the 'jobs will be processed anyway' message from worker" I was missing the worker logs in my tail -f -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- I don't understand the discussion.
I might try to resurrect those patches...
The patches look good to me and I am ready to signoff. What's missing? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #8 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #7)
I don't understand the discussion.
I might try to resurrect those patches...
The patches look good to me and I am ready to signoff. What's missing?
On this bug, there's nothing missing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186506|0 |1 is obsolete| | Attachment #186507|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 186620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186620&action=edit Bug 40820: Fix STOMP connection warnings when JobsNotificationMethod is 'polling' The background job workers are showing STOMP/RabbitMQ connection error warnings even when the JobsNotificationMethod system preference was set to 'polling', which doesn't require a message broker connection. This patch adds checks for the JobsNotificationMethod preference before displaying connection warnings, so warnings only appear when STOMP is actually expected to be available. Changes: - misc/workers/background_jobs_worker.pl: Check syspref before warning - misc/workers/es_indexer_daemon.pl: Check syspref before warning Test plan: 1. Set JobsNotificationMethod to 'polling' 2. Run background_jobs_worker.pl - should not show STOMP warnings 3. Run es_indexer_daemon.pl - should not show STOMP warnings 4. Set JobsNotificationMethod to 'STOMP' without RabbitMQ running 5. Run workers - should show appropriate STOMP connection warnings 6. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 186621 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186621&action=edit Bug 40820: (follow-up) es_indexer_daemon.pl missed on 34070 The es_indexer_daemon.pl was missing handling for the case where Koha::BackgroundJob->connect() returns undef without throwing an exception (which happens when connection fails silently). This adds the missing warning for that case, matching the behavior in background_jobs_worker.pl. Test plan: 1. Set JobsNotificationMethod to 'STOMP' 2. Configure invalid STOMP settings that cause connect() to return undef 3. Run es_indexer_daemon.pl - should show connection warning 4. Verify jobs are still processed via database polling Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186620|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 186816 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186816&action=edit Bug 40820: Fix STOMP connection warnings when JobsNotificationMethod is 'polling' The background job workers are showing STOMP/RabbitMQ connection error warnings even when the JobsNotificationMethod system preference was set to 'polling', which doesn't require a message broker connection. This patch adds checks for the JobsNotificationMethod preference before displaying connection warnings, so warnings only appear when STOMP is actually expected to be available. Changes: - misc/workers/background_jobs_worker.pl: Check syspref before warning - misc/workers/es_indexer_daemon.pl: Check syspref before warning Test plan: 1. Set JobsNotificationMethod to 'polling' 2. Run background_jobs_worker.pl - should not show STOMP warnings 3. Run es_indexer_daemon.pl - should not show STOMP warnings 4. Set JobsNotificationMethod to 'STOMP' without RabbitMQ running 5. Run workers - should show appropriate STOMP connection warnings 6. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186816|0 |1 is obsolete| | --- Comment #12 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 186817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186817&action=edit Bug 40820: Fix STOMP connection warnings when JobsNotificationMethod is 'polling' The background job workers are showing STOMP/RabbitMQ connection error warnings even when the JobsNotificationMethod system preference was set to 'polling', which doesn't require a message broker connection. This patch adds checks for the JobsNotificationMethod preference before displaying connection warnings, so warnings only appear when STOMP is actually expected to be available. Changes: - misc/workers/background_jobs_worker.pl: Check syspref before warning - misc/workers/es_indexer_daemon.pl: Check syspref before warning Test plan: 1. Set JobsNotificationMethod to 'polling' 2. Run background_jobs_worker.pl - should not show STOMP warnings 3. Run es_indexer_daemon.pl - should not show STOMP warnings 4. Set JobsNotificationMethod to 'STOMP' without RabbitMQ running 5. Run workers - should show appropriate STOMP connection warnings 6. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186621|0 |1 is obsolete| | --- Comment #13 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 186818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186818&action=edit Bug 40820: (follow-up) es_indexer_daemon.pl missed on 34070 The es_indexer_daemon.pl was missing handling for the case where Koha::BackgroundJob->connect() returns undef without throwing an exception (which happens when connection fails silently). This adds the missing warning for that case, matching the behavior in background_jobs_worker.pl. Test plan: 1. Set JobsNotificationMethod to 'STOMP' 2. Configure invalid STOMP settings that cause connect() to return undef 3. Run es_indexer_daemon.pl - should show connection warning 4. Verify jobs are still processed via database polling Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #14 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Keywords| |rel_24_05_candidate, | |rel_24_11_candidate, | |rel_25_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.11.00 |25.11.00,25.05.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 --- Comment #15 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.wojtkowski@biblibr | |e.com Status|Pushed to stable |Pushed to oldstable Version(s)|25.11.00,25.05.05 |25.11.00,25.05.05,24.11.11 released in| | --- Comment #16 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Pushed into 24.11.x for 24.11.11 nice work everyone -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_25_05_candidate | CC| |paul.derscheid@lmscloud.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Needs documenting CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #17 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #18 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Nothing to add or edit in the Koha manual for this. Please reopen and provide more information if that's not the case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40820 Bug 40820 depends on bug 34070, which changed state. Bug 34070 Summary: background_jobs_worker.pl floods logs when it gets error frames https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34070 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org