Page MenuHomePhabricator

TypeError: Argument 1 passed to Wikimedia\Rdbms\TransactionProfiler::recordConnection() must be of the type string, null given
Closed, ResolvedPublic

Description

This error happens when setting up a new wiki on https://patchdemo.wmflabs.org/, when running the edit.php maintenance script:

TypeError: Argument 1 passed to Wikimedia\Rdbms\TransactionProfiler::recordConnection() must be of the type string, null given, called in /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php on line 1006
Backtrace:
from /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/includes/libs/rdbms/TransactionProfiler.php(197)
#0 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1006): Wikimedia\Rdbms\TransactionProfiler->recordConnection(NULL, string, boolean)
#1 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(960): Wikimedia\Rdbms\LoadBalancer->getServerConnection(integer, string, integer)
#2 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1099): Wikimedia\Rdbms\LoadBalancer->getConnection(integer, array, string, integer)
#3 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/includes/user/User.php(819): Wikimedia\Rdbms\LoadBalancer->getConnectionRef(integer)
#4 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/maintenance/edit.php(64): User::newSystemUser(string, array)
#5 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/maintenance/doMaintenance.php(112): EditCLI->execute()
#6 /srv/patchdemo-wikis/78374bbd8ec1152e7332eda02d0f3ff7/w/maintenance/edit.php(134): require_once(string)
#7 {main}

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I will bisect and revert if no one investigates.

Had this locally, reverting I5d106ac5c95d0ea94c4f6bdee90ca51f8f7ddbad (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/666508) fixed it.

This is borderline UBN. Breaking master. Also some regression testing would be nice.

Krinkle moved this task from Inbox to Doing: Prio Interrupt on the Performance-Team board.
Krinkle moved this task from Untriaged to Rdbms library on the Wikimedia-Rdbms board.

Change 670476 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] rdbms: cleanup getServer() and connection parameter fields in Database

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

Change 671282 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] rdbms: work around Database::getServer() returning null

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

The patches (when both are applied) fix the issue for me, but I'm not comfortable merging a 200-line change in database internals.

The second one alone should be enough for a quick fix.

It's not:

Uncaught TypeError: Argument 1 passed to Wikimedia\Rdbms\TransactionProfiler::transactionWritingOut() must be of the type string, null given, called in /srv/patchdemo-wikis/659ccdb7732485b5980061ba1de79679/w/includes/libs/rdbms/database/Database.php on line 4699 and defined in /srv/patchdemo-wikis/659ccdb7732485b5980061ba1de79679/w/includes/libs/rdbms/TransactionProfiler.php:322
Stack trace:
#0 /srv/patchdemo-wikis/659ccdb7732485b5980061ba1de79679/w/includes/libs/rdbms/database/Database.php(4699): Wikimedia\Rdbms\TransactionProfiler->transactionWritingOut(NULL, 'patchdemo_659cc...', '9b0417', 0, 0)
#1 /srv/patchdemo-wikis/659ccdb7732485b5980061ba1de79679/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1952): Wikimedia\Rdbms\Database->rollback('MWExceptionHand...', 'flush')
#2 /srv/patchdemo-wikis/659ccdb7732485b5980061ba1de79679/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(2255): Wikimedia\Rdbms\LoadBalancer::Wikimedia\Rdbms\{closure}(Object(Wikimedia\Rdbms\DatabaseMysqli))
#3 /srv/patchdemo-wikis/ in /srv/patchdemo-wikis/659ccdb7732485b5980061ba1de79679/w/includes/libs/rdbms/TransactionProfiler.php on line 322

@aaron For testing, you can copy the Gerrit change number into https://patchdemo.wmflabs.org/, and see if it creates a wiki or fails (in which case the exception will be somewhere in the log it prints).

Change 672549 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Revert "rdbms: Group DBPerformance logs by violated measure"

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

Change 672550 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Revert "rdbms: avoid undefined "expectBy" notices in TransactionProfiler"

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

@aaron I'm sorry but I don't think it's acceptable to leave master broken for almost a week now. This bug is interfering with my work. I hope I'm not causing you too much annoyance by reverting these patches.

Agreeing with @matmarex, I do know a little about database internals and would be happy to review but rather smaller patches with proper tests, less duplicated code, hopefully following SOLID a bit, etc.

Change 672549 merged by jenkins-bot:
[mediawiki/core@master] Revert "rdbms: Group DBPerformance logs by violated measure"

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

Unfortunately the change in question contained many unrelated internal changes and refactorings, so I have no idea if it safe to revert or what other commits may have started to depend on this since then. Please keep this task open for @aaron to review after the fact, which I'm considering a train blocker for safety.

Change 672550 merged by jenkins-bot:
[mediawiki/core@master] Revert "rdbms: avoid undefined "expectBy" notices in TransactionProfiler"

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

Change 672555 had a related patch set uploaded (by Krinkle; owner: Aaron Schulz):
[mediawiki/core@master] rdbms: avoid undefined "expectBy" notices in TransactionProfiler (II)

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

Change 672587 had a related patch set uploaded (by Krinkle; owner: Aaron Schulz):
[mediawiki/core@wmf/1.36.0-wmf.35] rdbms: avoid undefined "expectBy" notices in TransactionProfiler (II)

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

Change 672555 merged by jenkins-bot:
[mediawiki/core@master] rdbms: avoid undefined "expectBy" notices in TransactionProfiler (II)

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

It seems it's not a blocker anymore. Can you remove it?

oh sorry I thought the merged one was the backport

Change 672587 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.35] rdbms: avoid undefined "expectBy" notices in TransactionProfiler (II)

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

Change 671282 abandoned by Aaron Schulz:
[mediawiki/core@master] rdbms: work around Database::getServer() returning null

Reason:
obsolete

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