Bug 2091129 - Bugzilla::DB pings MySQL before every query (about 200 pings per show_bug.cgi request)

Bugzilla/DB.pm

@@ -26,13 +26,15 @@ use Bugzilla::DB::Schema;
 use Bugzilla::Version;
 
 use List::Util qw(max);
-use Scalar::Util qw(weaken);
+use Scalar::Util qw(refaddr weaken);
+use Time::HiRes ();
 use Storable qw(dclone);
 use English qw(-no_match_vars);
 use Module::Runtime qw(require_module);
 
-has 'connector' => (is => 'lazy', handles => [qw( dbh )]);
+has 'connector' => (is => 'lazy');
 has 'model'     => (is => 'lazy');
+
 has [qw(dsn user pass attrs)] => (is => 'ro', required => 1);
 
 around 'attrs' => sub {
@@ -52,6 +54,35 @@ around 'attrs' => sub {
   return $attrs;
 };
 
+# Longest time (seconds) a checked handle is reused without asking
+# DBIx::Connector again. Only matters for long-running processes; web
+# requests re-check at the start of every request.
+use constant DBH_CHECK_INTERVAL => 5;
+
+# DBIx::Connector->dbh pings the server on every call, and every proxied DBI
+# method below calls dbh(), so a single page used to send hundreds of pings.
+# Reuse the handle the connector last returned, re-checking it once per
+# request (the request cache is cleared before each one), after a fork, when
+# the handle is no longer active, or after DBH_CHECK_INTERVAL seconds.
+sub dbh {
+  my ($self) = @_;
+  my $cache = Bugzilla->request_cache->{bz_db_checked_dbh} //= {};
+  my $entry = $cache->{refaddr $self};
+  my $now   = Time::HiRes::time();
+
+  if ( $entry
+    && $entry->{pid} == $PID
+    && $entry->{dbh}{Active}
+    && $now - $entry->{checked} < DBH_CHECK_INTERVAL)
+  {
+    return $entry->{dbh};
+  }
+
+  my $dbh = $self->connector->dbh;
+  $cache->{refaddr $self} = {dbh => $dbh, pid => $PID, checked => $now};
+  return $dbh;
+}
+
 # Install proxy methods to the DBI object.
 # We can't use handles() as DBIx::Connector->dbh has to be called each
 # time we need a DBI handle to ensure the connection is alive.

t/db-dbh-check.t

@@ -0,0 +1,73 @@
+#!/usr/bin/env perl
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+use strict;
+use warnings;
+use 5.10.1;
+use lib qw( . lib local/lib/perl5 );
+
+BEGIN { $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf' }
+
+# Only SELECT 1 is needed, so skip Bugzilla::Test::MockDB's schema setup.
+use Bugzilla::Test::MockLocalconfig (
+  db_driver => 'sqlite',
+  db_name   => ':memory:'
+);
+use Bugzilla;
+use Class::Method::Modifiers qw(install_modifier);
+use Scalar::Util qw(refaddr);
+use Test2::V0;
+
+# Bugzilla::DB->dbh should only ask DBIx::Connector (which pings) once per
+# request, not once per query.
+
+my $checks = 0;
+install_modifier('DBIx::Connector', 'before', 'dbh', sub { $checks++ });
+
+my $db    = Bugzilla->dbh;
+my $entry = sub { Bugzilla->request_cache->{bz_db_checked_dbh}{refaddr $db} };
+
+Bugzilla->clear_request_cache;
+$checks = 0;
+
+subtest 'one check for many queries in a request' => sub {
+  $db->selectrow_array('SELECT 1') for 1 .. 20;
+  my $sth = $db->prepare('SELECT 1');
+  $sth->execute;
+  is($checks,  1,                   'connector consulted once');
+  is($db->dbh, $db->connector->dbh, 'same handle as the connector');
+};
+
+subtest 'new request re-checks' => sub {
+  Bugzilla->clear_request_cache;
+  $checks = 0;
+  $db->selectrow_array('SELECT 1') for 1 .. 5;
+  is($checks, 1, 'checked again after clear_request_cache');
+};
+
+subtest 'long-running process re-checks after the interval' => sub {
+  $checks = 0;
+  $entry->()->{checked} -= Bugzilla::DB::DBH_CHECK_INTERVAL + 1;
+  $db->selectrow_array('SELECT 1') for 1 .. 5;
+  is($checks, 1, 'checked again once the interval passed');
+};
+
+subtest 'forked child re-checks' => sub {
+  $checks = 0;
+  $entry->()->{pid} = -1;
+  $db->selectrow_array('SELECT 1');
+  is($checks, 1, 'pid change forces a check');
+};
+
+subtest 'dropped handle reconnects' => sub {
+  $db->dbh->disconnect;
+  $checks = 0;
+  is($db->selectrow_array('SELECT 1'), 1, 'query works after disconnect');
+  is($checks,                          1, 'inactive handle forces a check');
+};
+
+done_testing;