Page MenuHomePhabricator

InvalidArgumentException: The revision does not belong to the given page.
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error
normalized_message
[{reqId}] {exception_url}   InvalidArgumentException: The revision does not belong to the given page.
exception.trace
from /srv/mediawiki/php-1.38.0-wmf.12/includes/page/ParserOutputAccess.php(339)
#0 /srv/mediawiki/php-1.38.0-wmf.12/includes/page/ParserOutputAccess.php(249): MediaWiki\Page\ParserOutputAccess->checkPreconditions(MediaWiki\Page\PageStoreRecord, MediaWiki\Revision\RevisionStoreRecord, integer)
#1 /srv/mediawiki/php-1.38.0-wmf.12/extensions/DiscussionTools/includes/Notifications/EventDispatcher.php(57): MediaWiki\Page\ParserOutputAccess->getParserOutput(MediaWiki\Page\PageStoreRecord, ParserOptions, MediaWiki\Revision\RevisionStoreRecord)
#2 /srv/mediawiki/php-1.38.0-wmf.12/extensions/DiscussionTools/includes/Notifications/EventDispatcher.php(111): MediaWiki\Extension\DiscussionTools\Notifications\EventDispatcher::getParsedRevision(MediaWiki\Revision\RevisionStoreRecord)
#3 /srv/mediawiki/php-1.38.0-wmf.12/extensions/DiscussionTools/includes/Hooks/EchoHooks.php(97): MediaWiki\Extension\DiscussionTools\Notifications\EventDispatcher::generateEventsForRevision(array, MediaWiki\Revision\RevisionStoreRecord)
#4 /srv/mediawiki/php-1.38.0-wmf.12/includes/HookContainer/HookContainer.php(338): MediaWiki\Extension\DiscussionTools\Hooks\EchoHooks::onEchoGetEventsForRevision(array, MediaWiki\Revision\RevisionStoreRecord, boolean)
#5 /srv/mediawiki/php-1.38.0-wmf.12/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#6 /srv/mediawiki/php-1.38.0-wmf.12/includes/Hooks.php(137): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#7 /srv/mediawiki/php-1.38.0-wmf.12/extensions/Echo/includes/DiscussionParser.php(165): Hooks::run(string, array)
#8 /srv/mediawiki/php-1.38.0-wmf.12/extensions/Echo/includes/EchoHooks.php(507): EchoDiscussionParser::generateEventsForRevision(MediaWiki\Revision\RevisionStoreRecord, boolean)
#9 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/MWCallableUpdate.php(38): EchoHooks::{closure}()
#10 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/DeferredUpdates.php(507): MWCallableUpdate->doUpdate()
#11 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/DeferredUpdates.php(392): DeferredUpdates::attemptUpdate(MWCallableUpdate, Wikimedia\Rdbms\LBFactoryMulti)
#12 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/DeferredUpdates.php(221): DeferredUpdates::run(MWCallableUpdate, Wikimedia\Rdbms\LBFactoryMulti, Monolog\Logger, BufferingStatsdDataFactory, string)
#13 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/DeferredUpdatesScope.php(267): DeferredUpdates::{closure}(MWCallableUpdate, integer)
#14 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/DeferredUpdatesScope.php(196): DeferredUpdatesScope->processStageQueue(integer, integer, Closure)
#15 /srv/mediawiki/php-1.38.0-wmf.12/includes/deferred/DeferredUpdates.php(242): DeferredUpdatesScope->processUpdates(integer, Closure)
#16 /srv/mediawiki/php-1.38.0-wmf.12/includes/MediaWiki.php(1106): DeferredUpdates::doUpdates()
#17 /srv/mediawiki/php-1.38.0-wmf.12/includes/MediaWiki.php(830): MediaWiki->restInPeace()
#18 /srv/mediawiki/php-1.38.0-wmf.12/includes/MediaWiki.php(579): MediaWiki->doPostOutputShutdown()
#19 /srv/mediawiki/php-1.38.0-wmf.12/index.php(53): MediaWiki->run()
#20 /srv/mediawiki/php-1.38.0-wmf.12/index.php(46): wfIndexMain()
#21 /srv/mediawiki/w/index.php(3): require(string)
#22 {main}
Notes

11 of these in the last hour. Unclear impact.

Details

Event Timeline

Now 30 of these since deploy of 1.38.0-wmf.12 (T293953) - logspam-watch output:

30      _▂▃▁▂▃▃ 2004    2053 ◦  InvalidArgument.....    .12 i/p/ParserOutputAccess:339  The revision does not belong to the given page.

Not sure if that's ticking upwards or not.

We haven't changed this code recently. My first suspicion is another bug in MediaWiki core similar to T289717.

I've been trying to understand that code in the meantime, I feel like I understand less about it the more I read, but I found and fixed an unrelated bug: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/745804

The code I added in rEDTO4bbbdc970330: EventDispatcher: Try really, really hard to read from master relied on the null being returned with the READ_NORMAL flag.

The problem in that code is that it uses getPageByReference, which will do a lookup by namespace and title (PageReference doesn't have an ID). It only worked by accident, due to me trying too hard to optimize (which led to T296063).

The code in EventDispatcher should use getPageById() instead. I'll make a patch.

Thanks.

Can you explain why the lookup by namespace and title shouldn't work?

Thanks.

Can you explain why the lookup by namespace and title shouldn't work?

I don't have a complete mental model of how this error happens, but during page moves, the association between page ID and title gets out of whack. RevisionStore tries to re-use Title objects provided by the caller, so you may end up with a Title instance that has the correct ID bot the wrong title (or vice versa) still in memory.

To avoid this, we can force a read from master, which is what you are doing. But the canonical association between revision and title is by page ID, and a mismatch will trigger "The revision does not belong to the given page". So doing the lookup by page ID should fix it.

Change 745829 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] Fix PageRecord lookup

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

Change 745829 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Fix PageRecord lookup

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

Change 745652 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@wmf/1.38.0-wmf.12] Fix PageRecord lookup

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

Mentioned in SAL (#wikimedia-operations) [2021-12-10T16:43:37Z] <dancy@deploy1002> Synchronized php-1.38.0-wmf.12/extensions/DiscussionTools/includes/Notifications/EventDispatcher.php: Backport: [[gerrit:745652|Fix PageRecord lookup (T297431)]] (duration: 00m 58s)

Change 745652 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@wmf/1.38.0-wmf.12] Fix PageRecord lookup

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

Mentioned in SAL (#wikimedia-operations) [2021-12-10T16:56:54Z] <dancy@deploy1002> Synchronized php-1.38.0-wmf.12/extensions/DiscussionTools/includes/Notifications/EventDispatcher.php: Backport: [[gerrit:745652|Fix PageRecord lookup (T297431)]] (duration: 00m 58s)

Should be resolved in production now.

Relevant Logstash search: https://logstash.wikimedia.org/goto/c97da891d7a21ae358728627c0c92ea5

image.png (523×2 px, 51 KB)

There were no errors in the last ~30 minutes since the fix was deployed.