Page MenuHomePhabricator

Refactor LoadBalancer connection pooling to be more efficient
Closed, ResolvedPublic

Description

Currently, foreign and local domain connections always use different handles. Also, the foreign connection logic does not support reuse of handles while one is already in use, e.g.:

$cMain = $lb->getConnection( DB_MASTER );
// some queries
$conn1 = $lb->getConnection( DB_MASTER, [], 'x' );
// some queries
$conn2 = $lb->getConnection( DB_MASTER, [], 'y' );
// some queries
$lb->reuseConnection( $conn1 );
// some queries
$lb->reuseConnection( $conn2 );

...will acquire three connections. If everything used connection references, and those DBConnRef instance checked and corrected the domain in each proxy method (implementing IDatabase), then the above scenario could just use a single connection and one atomic transaction. E.g.:

$cMain = $lb->getConnectionRef( DB_MASTER );
// some queries
$conn1 = $lb->getConnectionRef( DB_MASTER, [], 'x' );
// some queries
$conn2 = $lb->getConnectionRef( DB_MASTER, [], 'y' );
// some queries
// $conn2 falls out of scope
// some queries
// $conn1 falls out of scope
// $cMain falls out of scope

Connections acquired by raw getConnection() calls would still use a separate pool for backwards compatibility (no sudden domain changes from DBConnRef instances proxying them).

Event Timeline

Change 519305 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] [WIP] rdbms: improve LoadBalancer connection pool reuse

https://gerrit.wikimedia.org/r/519305

Krinkle triaged this task as Medium priority.Jul 23 2019, 5:30 PM
Krinkle lowered the priority of this task from Medium to Low.Apr 21 2020, 5:53 PM

Change 643563 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] Migrate LoadBalancer::getConnection() calls to LoadBalancer::getConnectionRef()

https://gerrit.wikimedia.org/r/643563

Change 643563 abandoned by Krinkle:

[mediawiki/core@master] Migrate LoadBalancer::getConnection() calls to LoadBalancer::getConnectionRef()

Reason:

Superseded by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/756898/

https://gerrit.wikimedia.org/r/643563

Krinkle raised the priority of this task from Low to High.Jun 15 2022, 4:06 PM
Krinkle subscribed.

Raising priority as per prod error at T193565.

Change 519305 merged by jenkins-bot:

[mediawiki/core@master] rdbms: improve LoadBalancer connection pool reuse

https://gerrit.wikimedia.org/r/519305

Removing task assignee due to inactivity as this open task has been assigned for more than two years. See the email sent to the task assignee on August 22nd, 2022.
Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome!
If this task has been resolved in the meantime, or should not be worked on ("declined"), please update its task status via "Add Action… 🡒 Change Status".
Also see https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator. Thanks!

hashar subscribed.

Reopening since we have reverted:

Change 519305 merged by jenkins-bot:

[mediawiki/core@master] rdbms: improve LoadBalancer connection pool reuse

https://gerrit.wikimedia.org/r/519305

Which caused T318904

Error 1049: Unknown database 'metawiki'
Function: Wikimedia\Rdbms\DatabaseMysqlBase::doSelectDomain
Query: USE `metawiki`

UserGroupManager uses ReadOnlyMode instance for foreign LoadBalancer instances. It doesn't pass the domain down, so the default context domain is used (the current wiki). This causes failures in getReadOnlyReason().

We use per-server read-only mode (not per database) at WMF. Ideally getReadOnlyReason() could use a "don't care" domain, avoiding the selectDomain() in reuseOrOpenConnectionForNewRef().

Change 838235 had a related patch set uploaded (by Aaron Schulz; author: Aaron Schulz):

[mediawiki/core@master] rdbms: improve LoadBalancer connection pool reuse (ii)

https://gerrit.wikimedia.org/r/838235

Change 838235 merged by jenkins-bot:

[mediawiki/core@master] rdbms: improve LoadBalancer connection pool reuse (ii)

https://gerrit.wikimedia.org/r/838235