Attachment 171512 Details for Bug 35655 – Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

[patch] Bug 35655: Only use STOMP if JobsNotificationMethod set to do so

Bug-35655-Only-use-STOMP-if-JobsNotificationMethod.patch (text/plain), 5.34 KB, created by Mark Hofstetter on 2024-09-14 13:03:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Hofstetter

Created: 2024-09-14 13:03:03 UTC

Size: 5.34 KB

patch

obsolete

>From 40f4b0f99c4ca7b6af465fe57be6dbbeee504380 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Thu, 12 Sep 2024 11:14:22 -0300
>Subject: [PATCH] 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
>---
> Koha/BackgroundJob.pm                         | 24 ++++++++++++-------
> about.pl                                      | 13 ++++++----
> .../intranet-tmpl/prog/en/modules/about.tt    |  4 +++-
> 3 files changed, 28 insertions(+), 13 deletions(-)
>
>diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm
>index 33a6872ec0..6e066e51c7 100644
>--- a/Koha/BackgroundJob.pm
>+++ b/Koha/BackgroundJob.pm
>@@ -63,6 +63,10 @@ Connect to the message broker using default guest/guest credential
> 
> sub connect {
>     my ( $self );
>+    my $notification_method = C4::Context->preference('JobsNotificationMethod') // 'STOMP';
>+    return undef
>+        unless $notification_method eq 'STOMP';
>+
>     my $hostname = 'localhost';
>     my $port = '61613';
>     my $config = C4::Context->config('message_broker');
>@@ -77,8 +81,17 @@ sub connect {
>         $credentials->{passcode} = $config->{password} if $config->{password};
>         $credentials->{host} = $config->{vhost} if $config->{vhost};
>     }
>-    my $stomp = Net::Stomp->new( { hostname => $hostname, port => $port } );
>-    $stomp->connect( $credentials );
>+    
>+    my $stomp;
>+
>+    try {
>+        $stomp = Net::Stomp->new( { hostname => $hostname, port => $port } );
>+        $stomp->connect( $credentials );
>+    } catch {
>+        warn "Cannot connect to broker " . $_;
>+        $stomp = undef;
>+    };
>+
>     return $stomp;
> }
> 
>@@ -123,12 +136,7 @@ sub enqueue {
> 
>     $job_args->{job_id} = $self->id;
> 
>-    my $conn;
>-    try {
>-        $conn = $self->connect;
>-    } catch {
>-        warn "Cannot connect to broker " . $_;
>-    };
>+    my $conn = $self->connect;
>     return $self->id unless $conn;
> 
>     $json_args = $json->encode($job_args);
>diff --git a/about.pl b/about.pl
>index 6233d8be23..c8ff0658b2 100755
>--- a/about.pl
>+++ b/about.pl
>@@ -906,10 +906,15 @@ sub message_broker_check {
>     my $template = shift;
>     {
>         # BackgroundJob - test connection to message broker
>-        eval { Koha::BackgroundJob->connect; };
>-        if ($@) {
>-            warn $@;
>-            $template->param( warnConnectBroker => $@ );
>+        my $conn = Koha::BackgroundJob->connect; 
>+        if (! $conn) {
>+            if (C4::Context->preference('JobsNotificationMethod') eq 'STOMP' ) {
>+                $template->param( warnConnectBroker => 'Error connecting' );
>+            } else {
>+                $template->param( 
>+                    warnConnectBroker => C4::Context->preference('JobsNotificationMethod')
>+                );
>+            }          
>         }
>     }
>   }
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
>index 0713215662..13e7c652c3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
>@@ -202,7 +202,9 @@
>             </tr>
>             <tr>
>                 <th scope="row">Message broker: </th>
>-                [% IF warnConnectBroker %]
>+                [% IF warnConnectBroker == 'Error connecting' %]
>+                    <td class="text-bg-warning"><strong>Using SQL polling (Fallback, Error connecting to RabbitMQ)</strong></td>
>+                [% ELSIF warnConnectBroker %]
>                     <td class="text-bg-warning"><strong>Using SQL polling</strong></td>
>                 [% ELSE %]
>                     <td class="text-bg-info">Using RabbitMQ</td>
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 35655: 171412 | 171413 | 171503 | 171504 | 171505 | 171506 | 171507 | 171509 | 171510 | 171511 | 171512 | 172027 | 172028 | 172029 | 172157 | 172158 | 172159 | 172232