Page MenuHomePhabricator
Create Task
Maniphest T321419

Flaky test RevisionStoreDbTest::testNewRevisionFromRow_getQueryInfo
Closed, ResolvedPublic
Actions

Description

Flaky test failing under php8.0 and php8.1 (right now, not seen under php7)

There was 1 error:

1) MediaWiki\Tests\Revision\RevisionStoreDbTest::testNewRevisionFromRow_getQueryInfo
TypeError: MediaWiki\Revision\RevisionStore::newRevisionFromRowAndSlots(): Argument #1 ($row) must be of type stdClass, bool given, called in /workspace/src/includes/Revision/RevisionStore.php on line 1609

/workspace/src/includes/Revision/RevisionStore.php:1706
/workspace/src/includes/Revision/RevisionStore.php:1609
/workspace/src/tests/phpunit/includes/Revision/RevisionStoreDbTest.php:1253
/workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:517

The bool coming from IDatabase::selectRow in the test, which selects the added revision id from revision table, sounds not possible

On https://gerrit.wikimedia.org/r/c/mediawiki/core/+/845239 under php8.0 - https://integration.wikimedia.org/ci/job/mediawiki-quibble-vendor-mysql-php80-docker/403/console
On https://gerrit.wikimedia.org/r/c/mediawiki/core/+/843942 under php8.1 - https://integration.wikimedia.org/ci/job/mediawiki-quibble-vendor-mysql-php81-docker/295/console

Test passed on both patch after resubmit.

Details

SubjectRepoBranchLines +/-
rdbms: make LoadBalancer::getServerConnection() avoid domain change for DOMAIN_ANYmediawiki/coremaster+2 -9
Customize query in gerrit

Related Objects

Event Timeline

Umherirrender created this task.Oct 21 2022, 9:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 21 2022, 9:58 PM
Krinkle added a project: Platform Engineering.Oct 21 2022, 10:28 PM
Krinkle added a subscriber: daniel.
• tstarling subscribed.Oct 25 2022, 1:52 AM

In the debug log from https://integration.wikimedia.org/ci/job/mediawiki-quibble-vendor-mysql-php74-docker/13035/ :

There's an initial edit from getTestPage() creating rev_id=2

* Line 353804: [PHPUnit] Start test MediaWiki\Tests\Revision\RevisionStoreDbTest::testNewRevisionFromRow_getQueryInfo
* Line 353845: [DBQuery] CommentStore::createComment [0s] localhost:...: INSERT INTO `unittest_comment` (comment_hash,comment_text,comment_data) VALUES (976592354,'UTPageSummary-MediaWiki\\Tests\\Revision\\RevisionStoreDbTest',NULL)
* Line 353846: [DBQuery] MediaWiki\User\ActorStore::findActorIdInternal [0s] localhost:...: SELECT  actor_user,actor_name,actor_id  FROM `unittest_actor`    WHERE actor_name = 'UTSysop'  LIMIT 1  
* Line 353847: [DBQuery] MediaWiki\Revision\RevisionStore::insertRevisionRowOn [0s] localhost:...: INSERT INTO `unittest_revision` (rev_page,rev_parent_id,rev_minor_edit,rev_timestamp,rev_deleted,rev_len,rev_sha1,rev_actor) VALUES (2,0,0,'20221024220939',0,54,'k92ygrjex3t003ullx0faan5zvj79uu',1)
* Line 353850: [DBQuery] CommentStore::insertInternal [0s] localhost:...: INSERT INTO `unittest_revision_comment_temp` (revcomment_rev,revcomment_comment_id) VALUES (2,2)

Then there's an edit creating rev_id=3:

* Line 353971: [DBQuery] MediaWiki\Storage\PageUpdater::doModify [0s] localhost:...: BEGIN
* Line 353977: [DBQuery] CommentStore::createComment [0s] localhost:...: INSERT INTO `unittest_comment` (comment_hash,comment_text,comment_data) VALUES (-2120085738,'MediaWiki\\Tests\\Revision\\RevisionStoreDbTest::testNewRevisionFromRow_getQueryInfoa',NULL)
* Line 353978: [DBQuery] MediaWiki\Revision\RevisionStore::insertRevisionRowOn [0s] localhost:...: INSERT INTO `unittest_revision` (rev_page,rev_parent_id,rev_minor_edit,rev_timestamp,rev_deleted,rev_len,rev_sha1,rev_actor) VALUES (2,2,0,'20221024220939',0,85,'9aa9fmy0ty4zuq5g2mhx1a7e4bmd58v',1)
* Line 353981: [DBQuery] CommentStore::insertInternal [0s] localhost:...: INSERT INTO `unittest_revision_comment_temp` (revcomment_rev,revcomment_comment_id) VALUES (3,3)
* Line 353987: [DBQuery] WikiPage::updateRevisionOn [0s] localhost:...: UPDATE  `unittest_page` SET page_latest = 3,page_touched = '20221024220939',page_is_new = 0,page_is_redirect = 0,page_len = 85,page_content_model = 'wikitext' WHERE page_id = 2
* Line 353991: [DBQuery] MediaWiki\Storage\PageUpdater::doModify [0s] localhost:...: COMMIT

A number of deferred updates are executed. PageUpdater->saveRevision has some log entries indicating that LoadBalancer::isPrimaryRunningReadOnly() did a cache refresh:

* Line 354065: [DeferredUpdates] DeferredUpdates::run: started MWCallableUpdate_MediaWiki\Storage\PageUpdater->saveRevision #122481
* Line 354066: [DBConnection] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/
* Line 354067:  [DBQuery] Wikimedia\Rdbms\DatabaseMysqlBase::serverIsReadOnly [0s] localhost:/workspace/db/quibble-mysql-oputqj5k/socket: SELECT @@GLOBAL.read_only AS Value

Then the test tries to load rev_id=3 and fails:

* Line 354071: [DBQuery] MediaWiki\Tests\Revision\RevisionStoreDbTest::testNewRevisionFromRow_getQueryInfo [0s] localhost:...: SELECT  rev_id,rev_page,rev_timestamp,rev_minor_edit,rev_deleted,rev_len,rev_parent_id,rev_sha1,comment_rev_comment.comment_text AS `rev_comment_text`,comment_rev_comment.comment_data AS `rev_comment_data`,comment_rev_comment.comment_id AS `rev_comment_cid`,actor_rev_user.actor_user AS `rev_user`,actor_rev_user.actor_name AS `rev_user_text`,rev_actor  FROM `revision` JOIN `revision_comment_temp` `temp_rev_comment` ON ((temp_rev_comment.revcomment_rev = rev_id)) JOIN `comment` `comment_rev_comment` ON ((comment_rev_comment.comment_id = temp_rev_comment.revcomment_comment_id)) JOIN `actor` `actor_rev_user` ON ((actor_rev_user.actor_id = rev_actor))   WHERE rev_id = 3  LIMIT 1  
* Line 354072: [PHPUnit] ERROR in test MediaWiki\Tests\Revision\RevisionStoreDbTest::testNewRevisionFromRow_getQueryInfo: Argument 1 passed to MediaWiki\Revision\RevisionStore::newRevisionFromRowAndSlots() must be an instance of stdClass, bool given, called in /workspace/src/includes/Revision/RevisionStore.php on line 1609

When I ran the test locally, LoadBalancer::isPrimaryRunningReadOnly() got a cache hit. I'll see what happens if I force a miss.

• tstarling added a subscriber: aaron.Oct 25 2022, 2:19 AM

Forcing a miss reproduces the test error.

LoadBalancer.php line 1109

						// Stay on the current database, but update the schema/prefix
						$conn->dbSchema( $domain->getSchema() );
						$conn->tablePrefix( $domain->getTablePrefix() );

$domain is DatabaseDomain::newFromId( LoadBalancer::DOMAIN_ANY ), which has database=null, schema=null and prefix=''. So it erases the table prefix from the connection.

RevisionStoreDbTest.php:

		$row = $this->db->selectRow(
			$info['tables'],
			$info['fields'],
			[ 'rev_id' => $revRecord->getId() ],
			__METHOD__,
			[],
			$info['joins']
		);

$this->db is a DatabaseMysqli not a DBConnRef, so ensureConnection() is not called.

aaron added a comment.Oct 25 2022, 3:22 AM
In T321419#8340472, @tstarling wrote:

$this->db is a DatabaseMysqli not a DBConnRef, so ensureConnection() is not called.

It should ideally be migrated to not use $this->db per T316841 .

I was thinking about removing the dbSchema()/tablePrefix() calls purely as optimization a few days back.

gerritbot added a comment.Oct 25 2022, 5:53 PM

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

[mediawiki/core@master] rdbms: make LoadBalancer::getServerConnection() avoid domain change for DOMAIN_ANY

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

gerritbot added a project: Patch-For-Review.Oct 25 2022, 5:53 PM
gerritbot added a comment.Oct 25 2022, 10:32 PM

Change 849134 merged by jenkins-bot:

[mediawiki/core@master] rdbms: make LoadBalancer::getServerConnection() avoid domain change for DOMAIN_ANY

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.8; 2022-10-31).Oct 25 2022, 11:00 PM
Maintenance_bot removed a project: Patch-For-Review.Oct 25 2022, 11:30 PM
• tstarling closed this task as Resolved.Oct 26 2022, 1:47 AM
• tstarling claimed this task.

Should be fixed.

Krinkle edited projects, added Performance-Team; removed Platform Engineering.Oct 31 2022, 6:29 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL