From e3db8dcef37f99ab12d64bef53b559cfc09c8096 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 9 Nov 2026 17:02:38 -0500 Subject: [PATCH 1/2] Bug 2091129 - Ping the database once per request instead of on every query --- Bugzilla/DB.pm | 34 ++++++++++++++++++++-- t/db-dbh-check.t | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 t/db-dbh-check.t diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index 96532c88ad..d1bdb155e7 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -26,13 +26,43 @@ 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'); + +# 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} == $$ + && $entry->{dbh}{Active} + && $now - $entry->{checked} < DBH_CHECK_INTERVAL) + { + return $entry->{dbh}; + } + + my $dbh = $self->connector->dbh; + $cache->{refaddr $self} = {dbh => $dbh, pid => $$, checked => $now}; + return $dbh; +} has [qw(dsn user pass attrs)] => (is => 'ro', required => 1); around 'attrs' => sub { diff --git a/t/db-dbh-check.t b/t/db-dbh-check.t new file mode 100644 index 0000000000..a1d1ee35fb --- /dev/null +++ b/t/db-dbh-check.t @@ -0,0 +1,76 @@ +#!/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 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; +{ + no warnings 'redefine'; + my $orig = \&DBIx::Connector::dbh; + *DBIx::Connector::dbh = sub { $checks++; goto &$orig }; +} + +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; From fc46691008682578f551aee9859730e57ef85381 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 9 Nov 2026 18:38:43 -0500 Subject: [PATCH 2/2] Fix tests --- Bugzilla/DB.pm | 4 ++-- t/db-dbh-check.t | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index d1bdb155e7..33900f2514 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -52,7 +52,7 @@ sub dbh { my $now = Time::HiRes::time(); if ( $entry - && $entry->{pid} == $$ + && $entry->{pid} == $PID && $entry->{dbh}{Active} && $now - $entry->{checked} < DBH_CHECK_INTERVAL) { @@ -60,7 +60,7 @@ sub dbh { } my $dbh = $self->connector->dbh; - $cache->{refaddr $self} = {dbh => $dbh, pid => $$, checked => $now}; + $cache->{refaddr $self} = {dbh => $dbh, pid => $PID, checked => $now}; return $dbh; } has [qw(dsn user pass attrs)] => (is => 'ro', required => 1); diff --git a/t/db-dbh-check.t b/t/db-dbh-check.t index a1d1ee35fb..b9ae562721 100644 --- a/t/db-dbh-check.t +++ b/t/db-dbh-check.t @@ -18,6 +18,7 @@ use Bugzilla::Test::MockLocalconfig ( db_name => ':memory:' ); use Bugzilla; +use Class::Method::Modifiers qw(install_modifier); use Scalar::Util qw(refaddr); use Test2::V0; @@ -25,11 +26,7 @@ use Test2::V0; # request, not once per query. my $checks = 0; -{ - no warnings 'redefine'; - my $orig = \&DBIx::Connector::dbh; - *DBIx::Connector::dbh = sub { $checks++; goto &$orig }; -} +install_modifier('DBIx::Connector', 'before', 'dbh', sub { $checks++ }); my $db = Bugzilla->dbh; my $entry = sub { Bugzilla->request_cache->{bz_db_checked_dbh}{refaddr $db} };