Bugzilla::DB should gracefully handle disconnection events that happen during transactions

RESOLVED FIXED

Status

()

P1
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: dylan, Assigned: dylan)

Tracking

Production

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
Every friday, the Request Naggers script runs a query that (at least recently) crashes the RDS/mysql server it connects to. This is an annoyance, but has been made into a dire situation due to interaction with the feed daemon that syncs phabricator and bugzilla.

If the feed daemon is connected to the same server, this causes the feed daemon to be disconnected.

In order to fix problems with stale connections (which also caused the feed daemon to break, but in a different way) Bugzilla::DB switched from being a subclass of DBI to having a 'has a' relationship with DBIx::Connector, which is a connection pool factory for DBI.

Bugzilla does not currently use the transaction features offered by DBIx::Connector, and instead opts to use a pairing of Bugzilla::DB->bz_start_transaction() and Bugzilla::DB->bz_commit_transaction() to handle transactions. 

In addition, bugzilla tracks if it is in a transaction using an attribute 'bz_in_transaction'.

For every DBI method called, the Bugzilla::DB object delegates to the DBIx::Connector instance, which returns a (possibly reused) DBI object.

Imagine the following scenario in pseudo-code:

$db->bz_start_transaction();
$db->do("INSERT ...");
$db->do("UPDATE ...");
$db->bz_commit_transaction();

In between the INSERT and UPDATE, a reconnection event may occur.
Should this happen, there is a 'on_connection' callback that will be called.
This callback will note that $db->bz_in_transaction is true,
and as a result will throw an error.

This is what you'd want, as it is not possible to continue.

However, the problem happens in Bugzilla's ->cleanup routine.

The cleanup routine is called at the end of every request, and
one of the things it does is if any transaction is left open, it will roll it back by calling bz_rollback_transaction().

The problem is that bz_rollback_transaction() will call the delegated 'rollback' method, which will cause a new DBI object to be created.
This means the on_connect() callback is called, and this is a fatal error.

The rest of the cleanup process will be skipped, and bz_in_transaction() will never be reset.

The solution to this becomes quite clear after figuring all this out...
(Assignee)

Updated

5 months ago
Summary: Feed daemon crashes / looping → Bugzilla::DB should gracefully handle disconnection events that happen during transactions
(Assignee)

Comment 1

5 months ago
Posted file GitHub Pull Request
Merged to master.
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.