diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index 96532c88ad..33900f2514 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} == $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; +} 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..b9ae562721 --- /dev/null +++ b/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;