Page MenuHomePhabricator

Track down unusual callers to SimpleCacheWithBagOfStuff
Closed, DeclinedPublic

Description

It seems that despite adding more careful filtering of languages before using them as cache keys we are still trying to make some keys that contain unusual content.

While we hope to make these likely to result in production errors with T245396 we should also investigate the cause of them which the probably represent places the cache key is being wrong build and thus wrongly split.

See https://phabricator.wikimedia.org/T245396#5912675 for an example of calls still happening.

Event Timeline

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

If there are functional problems in Wikibase with these values, then by all means file bugs for them and validate them where it makes sense.

But I would strongly recommend not investing effort into "fixing" unusual callers of the cache. I believe that would create more technical debt and spread out the problematic pattern further (and will be hard to understand from the call site way that is needed for some and not others).

The problem is the way BagOStuff is used in Wikibase violates its contract. More details at T245396#5962687.

Got a one-off(?) burst of this when switching group1 to 1.35.0-wmf.28:

Cache key contains characters that are not allowed: `commonswiki:wikibase.repo.formatter.:Q166_1132694752_%E2%A7%BClang%E2%A7%BD_label`

(from AXF_OSyE3bOlOASG0bng)

… all on category page renders on Commons, if that helps.

Got a one-off(?) burst of this when switching group1 to 1.35.0-wmf.28:

Cache key contains characters that are not allowed: `commonswiki:wikibase.repo.formatter.:Q166_1132694752_%E2%A7%BClang%E2%A7%BD_label`

(from AXF_OSyE3bOlOASG0bng)

… all on category page renders on Commons, if that helps.

I would have thought that this would no longer happen because of https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/583199/
This should have gone out with .26, and we are not in .28....
However I still see logs with this message right now.

[XqC9YApAIDAAAAOmQ2sAAABD] /wiki/Category:Association_football_players_from_Austria?uselang=%E2%A7%BCLang%E2%A7%BD Wikibase\Lib\CacheInvalidArgumentException from line 269 of /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/lib/includes/SimpleCacheWithBagOStuff.php: Cache key contains characters that are not allowed: commonswiki:wikibase.repo.formatter.:Q6998382_1137583700_%E2%A7%BClang%E2%A7%BD_label

#0 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/lib/includes/SimpleCacheWithBagOStuff.php(70): Wikibase\Lib\SimpleCacheWithBagOStuff->assertKeyIsValid(string)
#1 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/lib/includes/StatsdRecordingSimpleCache.php(59): Wikibase\Lib\SimpleCacheWithBagOStuff->get(string, string)
#2 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/lib/includes/Store/CachingFallbackLabelDescriptionLookup.php(120): Wikibase\Lib\StatsdRecordingSimpleCache->get(string, string)
#3 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/lib/includes/Store/CachingFallbackLabelDescriptionLookup.php(106): Wikibase\Lib\Store\CachingFallbackLabelDescriptionLookup->getTerm(Wikibase\DataModel\Entity\ItemId, string, string)
#4 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/client/includes/Usage/UsageTrackingLanguageFallbackLabelDescriptionLookup.php(71): Wikibase\Lib\Store\CachingFallbackLabelDescriptionLookup->getLabel(Wikibase\DataModel\Entity\ItemId)
#5 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/client/includes/DataAccess/Scribunto/WikibaseLanguageDependentLuaBindings.php(60): Wikibase\Client\Usage\UsageTrackingLanguageFallbackLabelDescriptionLookup->getLabel(Wikibase\DataModel\Entity\ItemId)
#6 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Wikibase/client/includes/DataAccess/Scribunto/Scribunto_LuaWikibaseLibrary.php(606): Wikibase\Client\DataAccess\Scribunto\WikibaseLanguageDependentLuaBindings->getLabel(string)
#7 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Scribunto/includes/engines/LuaSandbox/LuaSandboxCallback.php(26): Wikibase\Client\DataAccess\Scribunto\Scribunto_LuaWikibaseLibrary->getLabel(string)
#8 [internal function]: Scribunto_LuaSandboxCallback->__call(string, array)
#9 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Scribunto/includes/engines/LuaSandbox/LuaSandboxInterpreter.php(113): LuaSandboxFunction->call(LuaSandboxFunction)
#10 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Scribunto/includes/engines/LuaCommon/LuaEngine.php(291): Scribunto_LuaSandboxInterpreter->callFunction(LuaSandboxFunction, LuaSandboxFunction)
#11 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Scribunto/includes/engines/LuaCommon/LuaModule.php(69): Scribunto_LuaEngine->executeFunctionChunk(LuaSandboxFunction, PPTemplateFrame_Hash)
#12 /srv/mediawiki/php-1.35.0-wmf.28/extensions/Scribunto/includes/common/Hooks.php(128): Scribunto_LuaModule->invoke(string, PPTemplateFrame_Hash)
#13 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3613): ScribuntoHooks::invokeHook(Parser, PPTemplateFrame_Hash, array)
#14 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3317): Parser->callParserFunction(PPTemplateFrame_Hash, string, array)
#15 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#16 /srv/mediawiki/php-1.35.0-wmf.28/extensions/ParserFunctions/includes/ParserFunctions.php(143): PPFrame_Hash->expand(PPNode_Hash_Tree)
#17 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3613): MediaWiki\Extensions\ParserFunctions\ParserFunctions::ifeq(Parser, PPTemplateFrame_Hash, array)
#18 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3317): Parser->callParserFunction(PPTemplateFrame_Hash, string, array)
#19 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#20 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3495): PPFrame_Hash->expand(PPNode_Hash_Tree)
#21 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#22 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/PPTemplateFrame_Hash.php(89): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#23 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3492): PPTemplateFrame_Hash->cachedExpand(string, PPNode_Hash_Tree)
#24 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPFrame_Hash)
#25 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(3158): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#26 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(1504): Parser->replaceVariables(string)
#27 /srv/mediawiki/php-1.35.0-wmf.28/includes/parser/Parser.php(618): Parser->internalParse(string)
#28 /srv/mediawiki/php-1.35.0-wmf.28/includes/content/WikitextContent.php(368): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#29 /srv/mediawiki/php-1.35.0-wmf.28/includes/content/AbstractContent.php(565): WikitextContent->fillParserOutput(Title, integer, ParserOptions, boolean, ParserOutput)
#30 /srv/mediawiki/php-1.35.0-wmf.28/includes/Revision/RenderedRevision.php(267): AbstractContent->getParserOutput(Title, integer, ParserOptions, boolean)
#31 /srv/mediawiki/php-1.35.0-wmf.28/includes/Revision/RenderedRevision.php(236): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(WikitextContent, boolean)
#32 /srv/mediawiki/php-1.35.0-wmf.28/includes/Revision/RevisionRenderer.php(215): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string)
#33 /srv/mediawiki/php-1.35.0-wmf.28/includes/Revision/RevisionRenderer.php(152): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#34 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#35 /srv/mediawiki/php-1.35.0-wmf.28/includes/Revision/RenderedRevision.php(198): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#36 /srv/mediawiki/php-1.35.0-wmf.28/includes/poolcounter/PoolWorkArticleView.php(196): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#37 /srv/mediawiki/php-1.35.0-wmf.28/includes/poolcounter/PoolCounterWork.php(125): PoolWorkArticleView->doWork()
#38 /srv/mediawiki/php-1.35.0-wmf.28/includes/page/Article.php(787): PoolCounterWork->execute()
#39 /srv/mediawiki/php-1.35.0-wmf.28/includes/page/CategoryPage.php(66): Article->view()
#40 /srv/mediawiki/php-1.35.0-wmf.28/includes/actions/ViewAction.php(66): CategoryPage->view()
#41 /srv/mediawiki/php-1.35.0-wmf.28/includes/MediaWiki.php(519): ViewAction->show()
#42 /srv/mediawiki/php-1.35.0-wmf.28/includes/MediaWiki.php(305): MediaWiki->performAction(CategoryTreeCategoryPage, Title)
#43 /srv/mediawiki/php-1.35.0-wmf.28/includes/MediaWiki.php(973): MediaWiki->performRequest()
#44 /srv/mediawiki/php-1.35.0-wmf.28/includes/MediaWiki.php(535): MediaWiki->main()
#45 /srv/mediawiki/php-1.35.0-wmf.28/index.php(47): MediaWiki->run()
#46 /srv/mediawiki/w/index.php(3): require(string)

The comment in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/583199/2/lib/includes/SimpleCacheWithBagOStuff.php is relevant here, as this is the extra assert that is now in Wikibase code that is throwing.
As said in that comment Wikibase code shouldn't care about the validity of the cache key for this class, as the contract of the cache class that we are using says that if we use makeKey which we do we will be good.

The URL decoded cache key is commonswiki:wikibase.repo.formatter.:Q6998382_1137583700_⧼lang⧽_label and again we see this ⧼lang⧽ that keeps cropping up.
Anyway, the bag o stuff is getting a valid cache key for storing in the cache medium, and our assert is then throwing the exception when it doesnt need to, so the assert should go.

Now, ⧼lang⧽ still shouldn't be making it all the way down the stack here...
I dont know what triggers calls to pages such as https://commons.wikimedia.org/wiki/Category:Plates?uselang=%E2%A7%BCLang%E2%A7%BD but I will file a seperate task for that.

I'm going to submit the patch for removing the assert as part of T245396

I'm not sure if I have a good approach for the topic of this ticket as a whole "Track down unusual callers to SimpleCacheWithBagOfStuff"

@Tarrow any thoughts? or is T250930 enough for now for the one case specificaly identified?