Page MenuHomePhabricator

LogicException: Role mediainfo is already defined
Closed, ResolvedPublic

Description

Seen 248 times in the last 2 days on the beta cluster: https://logstash-beta.wmflabs.org/goto/5858d0a14210de44a466e4732abee1ce

Only appears on the deployment-parsoid11 host

Stack trace
#0 /srv/mediawiki/php-master/includes/Revision/SlotRoleRegistry.php(115): MediaWiki\Revision\SlotRoleRegistry->defineRole(string, Closure)
#1 /srv/mediawiki/php-master/extensions/WikibaseMediaInfo/src/WikibaseMediaInfoHooks.php(73): MediaWiki\Revision\SlotRoleRegistry->defineRoleWithModel(string, string)
#2 [internal function]: Wikibase\MediaInfo\WikibaseMediaInfoHooks::Wikibase\MediaInfo\{closure}(MediaWiki\Revision\SlotRoleRegistry, MediaWiki\MediaWikiServices)
#3 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(454): call_user_func_array(Closure, array)
#4 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(416): Wikimedia\Services\ServiceContainer->createService(string)
#5 /srv/mediawiki/php-master/includes/MediaWikiServices.php(1266): Wikimedia\Services\ServiceContainer->getService(string)
#6 /srv/mediawiki/php-master/includes/ServiceWiring.php(1142): MediaWiki\MediaWikiServices->getSlotRoleRegistry()
#7 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(447): Wikimedia\Services\ServiceContainer->{closure}(MediaWiki\MediaWikiServices)
#8 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(416): Wikimedia\Services\ServiceContainer->createService(string)
#9 /srv/mediawiki/php-master/includes/MediaWikiServices.php(1201): Wikimedia\Services\ServiceContainer->getService(string)
#10 /srv/mediawiki/php-master/includes/ServiceWiring.php(1124): MediaWiki\MediaWikiServices->getRevisionStoreFactory()
#11 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(447): Wikimedia\Services\ServiceContainer->{closure}(MediaWiki\MediaWikiServices)
#12 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(416): Wikimedia\Services\ServiceContainer->createService(string)
#13 /srv/mediawiki/php-master/includes/MediaWikiServices.php(1193): Wikimedia\Services\ServiceContainer->getService(string)
#14 /srv/mediawiki/php-master/includes/ServiceWiring.php(1110): MediaWiki\MediaWikiServices->getRevisionStore()
#15 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(447): Wikimedia\Services\ServiceContainer->{closure}(MediaWiki\MediaWikiServices)
#16 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(416): Wikimedia\Services\ServiceContainer->createService(string)
#17 /srv/mediawiki/php-master/includes/MediaWikiServices.php(1177): Wikimedia\Services\ServiceContainer->getService(string)
#18 /srv/mediawiki/php-master/includes/ServiceWiring.php(978): MediaWiki\MediaWikiServices->getRevisionLookup()
#19 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(447): Wikimedia\Services\ServiceContainer->{closure}(MediaWiki\MediaWikiServices)
#20 /srv/mediawiki/php-master/vendor/wikimedia/services/src/ServiceContainer.php(416): Wikimedia\Services\ServiceContainer->createService(string)
#21 /srv/mediawiki/php-master/includes/MediaWikiServices.php(1113): Wikimedia\Services\ServiceContainer->getService(string)
#22 /srv/mediawiki/php-master/includes/Rest/EntryPoint.php(46): MediaWiki\MediaWikiServices->getPermissionManager()
#23 /srv/mediawiki/php-master/includes/Rest/EntryPoint.php(108): MediaWiki\Rest\EntryPoint::createRouter(RequestContext, MediaWiki\Rest\RequestFromGlobals, MediaWiki\Rest\ResponseFactory, MediaWiki\Rest\CorsUtils)
#24 /srv/mediawiki/php-master/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#25 /srv/mediawiki/w/rest.php(3): require(string)
#26 {main}

Event Timeline

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

Looks like they were only happening on deployment-parsoid-11?

[1e5dfe70-1dd8-11eb-b835-abf5d16005e0] /w/rest.php/commons.wikimedia.beta.wmflabs.org/v3/page/pagebundle/File%3ADidus_ineptus_-_foto_van_model_-_1935_-_Print_-_Iconographia_Zoologica_-_Special_Collections_University_of_Amsterdam_-_UBA01_IZ15600009.tif/95794
commons.wikimedia.beta.wmflabs.org

Not sure how to create a reproduction URL for beta?
If it is still happening on beta I think this is highly likely to happen in production too for testcommonswiki and commonswiki.

Looks like they were only happening on deployment-parsoid-11?

[1e5dfe70-1dd8-11eb-b835-abf5d16005e0] /w/rest.php/commons.wikimedia.beta.wmflabs.org/v3/page/pagebundle/File%3ADidus_ineptus_-_foto_van_model_-_1935_-_Print_-_Iconographia_Zoologica_-_Special_Collections_University_of_Amsterdam_-_UBA01_IZ15600009.tif/95794
commons.wikimedia.beta.wmflabs.org

Not sure how to create a reproduction URL for beta?
If it is still happening on beta I think this is highly likely to happen in production too for testcommonswiki and commonswiki.

Yeah, I couldn't figure out how to reproduce it either

Same error message appeared in T255704.

If you dig into it, you'll probably find that you're accessing services before they are fully initialized, and this apparently blows up. IMO this is a bug in MediaWiki (T255704#6233300), but others disagreed with me on that task.

brennen triaged this task as Unbreak Now! priority.Nov 5 2020, 8:54 PM
brennen added a subscriber: brennen.

A large number of these (15000) immediately after deploy of 1.36.0-wmf.16 to group1, all for commonswiki / parsoid.

Change 639626 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/extensions/WikibaseMediaInfo@master] Do not try to define mediainfo role twice

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

This happened inside the MediaWikiServices hook consumed by WikibaseMediaInfo that exists exactly for this purpose. I think it somehow got called twice (should it?). Uploading a naive (?) attempt to fix it.

Same error message appeared in T255704.

If you dig into it, you'll probably find that you're accessing services before they are fully initialized, and this apparently blows up. IMO this is a bug in MediaWiki (T255704#6233300), but others disagreed with me on that task.

This was useful. https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/635096 is probably the patch in question that triggered the same core "bug" / "behavior". @Pchelolo is fixing.

Change 639628 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/services/parsoid@master] Don't access MWServices in onRegistration hook

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

I still think we should merge https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseMediaInfo/+/639626/, it seems to be a standard to do sanity checks (unless that hook somewhere guarantees it runs only once?)

I still think we should merge https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseMediaInfo/+/639626/, it seems to be a standard to do sanity checks (unless that hook somewhere guarantees it runs only once?)

But, we don't know if after we add this check here, some other extension might break in other ways. So, better to avoid the pattern for now.

Sure, I think we should merge both.

I +2ed @Pchelolo's patch. I leave wikibase folks to review the change you made there ( @Addshore ). Thanks for jumping on this quick!

Change 639628 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Don't access MWServices in onRegistration hook

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

Change 639631 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a16

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

If someone +2s that vendor patch, we can get the show started again. :)

Change 639504 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/vendor@wmf/1.36.0-wmf.16] Bump wikimedia/parsoid to 0.13.0-a16

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

Change 639504 merged by jenkins-bot:
[mediawiki/vendor@wmf/1.36.0-wmf.16] Bump wikimedia/parsoid to 0.13.0-a16

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

Mentioned in SAL (#wikimedia-operations) [2020-11-05T23:09:58Z] <brennen@deploy1001> Synchronized php-1.36.0-wmf.16/vendor: Backport: [[gerrit:639504|Bump wikimedia/parsoid to 0.13.0-a16 (T267146)]] (duration: 01m 14s)

Change 639631 merged by jenkins-bot:
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a16

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

brennen lowered the priority of this task from Unbreak Now! to High.Nov 5 2020, 11:51 PM

Removing as train blocker and decreasing priority, since we've got a patch ready to roll forward with the train on Monday.

ssastry assigned this task to Pchelolo.

I am going to be bold and call this resolved. The log events on beta cluster stopped as of 22:44 utc and I separately verified commons access via parsoid on scandium (which had the latest code for rt testing purposes anyway).

Change 639626 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Do not try to define mediainfo role twice

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

Looks like everything is good here?
I saw a mention of a patch in Wikibase to review?
But I'm going to assume that was the WikibaseMediaInfo patch? and is done? :)

Yes, looks good to me. Thanks Addshore.