Page MenuHomePhabricator

rdbms connection reuse mangles the DB domain of connection handles passed to transaction lifecycle callbacks
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue:

  • Configure your local wiki setup to have another database named centralauth on the same section as your main test wiki (doesn't matter if via a virtual domain or not).
  • Run this maintenance script:
<?php

use Wikimedia\Rdbms\IDatabase;

require_once __DIR__ . '/../../maintenance/Maintenance.php';

class DBReuse extends Maintenance {
	public function execute() {
		$lbFactory = $this->getServiceContainer()->getDBLoadBalancerFactory();
		$lbFactory->beginPrimaryChanges( __METHOD__ );

		$this->test( false );
		$this->test( 'centralauth' );

		$lbFactory->commitPrimaryChanges( __METHOD__ );
	}

	/**
	 * @param string|bool $domain
	 */
	private function test( $domain ) {
		$connectionProvider = $this->getServiceContainer()->getConnectionProvider();
		$dbw = $connectionProvider->getPrimaryDatabase( $domain );
		$dbw->query( 'select 1' );

		if ( $domain === false ) {
			$dbw->onTransactionPreCommitOrIdle(
				static function ( IDatabase $dbw ): void {
					var_dump( $dbw->getDomainID() );
				},
			);
		}
	}
}

$maintClass = DBReuse::class;
require RUN_MAINTENANCE_IF_MAIN;

What happens?:
It outputs string(11) "centralauth".

What should have happened instead?:
It should output string(...) "<your local test DB name>".

This caused a train blocker UBN: T386171.

Event Timeline

We could implement some bookkeeping to keep track of the expected DB domain for each pending callback and call selectDomain before each invocation or something, but given that nobody seems to make use of the passed-in connection handle, it seems easier to just remove the argument.

Change #1119095 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/core@master] rdbms: Remove usages of passed-in DB handle from transaction callbacks

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

Change #1119096 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/core@master] rdbms: Don't forward DB connection handle to transaction callbacks

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

Change #1119095 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Remove usages of passed-in DB handle from transaction callbacks

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

Change #1119096 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Don't forward DB connection handle to transaction callbacks

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

aaron assigned this task to mszabo.