Revision 386521405 belongs to M43864745 instead of expected M79277318
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error

MediaWiki version: 1.35.0-wmf.16

message

Revision 386521405 belongs to M43864745 instead of expected M79277318

Reproduction: https://commons.wikimedia.org/w/api.php?action=query&prop=description&titles=File:L%27Int%C3%A9rieur%20du%20Mus%C3%A9e%20des%20beaux-arts%20de%20Nancy.jpg

Impact

Notes

COVID-19 Deployment Criteria

Can you roll back this change without lasting impact?
1. A recovery plan is required as this will help identify our capacity for recovering from the failure
2. THIS IS A KEY QUESTION, if you can’t answer it, you shouldn’t deploy

Is specialized knowledge required to support this change in production? If so, are there multiple people with this knowledge?

Is there a way to increase confidence about the correctness of this change?
1. Reviews (Design, Code, etc)
2. Testing coverage (unit tests, integration tests)
3. Manual testing (e.g. Beta, vagrant, docker)

Details

Request ID: XiXviQpAAD8AAI1KJCkAAADN
Request URL: https://commons.wikimedia.org/w/api.php

Stack Trace

exception.trace

#0 /srv/mediawiki/php-1.35.0-wmf.16/extensions/WikibaseMediaInfo/src/Services/MediaInfoPrefetchingTermLookup.php(59): Wikibase\Lib\Store\Sql\WikiPageEntityRevisionLookup->getEntityRevision(Wikibase\MediaInfo\DataModel\MediaInfoId)
#1 /srv/mediawiki/php-1.35.0-wmf.16/extensions/WikibaseMediaInfo/src/Services/MediaInfoPrefetchingTermLookup.php(108): Wikibase\MediaInfo\Services\MediaInfoPrefetchingTermLookup->prefetchTerm(Wikibase\MediaInfo\DataModel\MediaInfoId, array, array)
#2 /srv/mediawiki/php-1.35.0-wmf.16/extensions/Wikibase/data-access/src/ByTypeDispatchingPrefetchingTermLookup.php(52): Wikibase\MediaInfo\Services\MediaInfoPrefetchingTermLookup->prefetchTerms(array, array, array)
#3 /srv/mediawiki/php-1.35.0-wmf.16/extensions/Wikibase/data-access/src/ByTypeDispatchingPrefetchingTermLookup.php(52): Wikibase\DataAccess\ByTypeDispatchingPrefetchingTermLookup->prefetchTerms(array, array, array)
#4 /srv/mediawiki/php-1.35.0-wmf.16/extensions/Wikibase/client/includes/Store/DescriptionLookup.php(143): Wikibase\DataAccess\ByTypeDispatchingPrefetchingTermLookup->prefetchTerms(array, array, array)
#5 /srv/mediawiki/php-1.35.0-wmf.16/extensions/Wikibase/client/includes/Store/DescriptionLookup.php(73): Wikibase\Client\Store\DescriptionLookup->getCentralDescriptions(array)
#6 /srv/mediawiki/php-1.35.0-wmf.16/extensions/Wikibase/client/includes/Api/Description.php(72): Wikibase\Client\Store\DescriptionLookup->getDescriptions(array, array, array)
#7 /srv/mediawiki/php-1.35.0-wmf.16/includes/api/ApiQuery.php(255): Wikibase\Client\Api\Description->execute()
#8 /srv/mediawiki/php-1.35.0-wmf.16/includes/api/ApiMain.php(1603): ApiQuery->execute()
#9 /srv/mediawiki/php-1.35.0-wmf.16/includes/api/ApiMain.php(539): ApiMain->executeAction()
#10 /srv/mediawiki/php-1.35.0-wmf.16/includes/api/ApiMain.php(510): ApiMain->executeActionWithErrorHandling()
#11 /srv/mediawiki/php-1.35.0-wmf.16/api.php(78): ApiMain->execute()
#12 /srv/mediawiki/w/api.php(3): require(string)
#13 {main}

Subject	Repo	Branch	Lines +/-
Update entity data on undelete to match new page id	mediawiki/extensions/WikibaseMediaInfo	master	+110 -2
"wikibase/data-model": "9.4.0"	mediawiki/vendor	master	+34 -15
[WIP] Don't allow restoring conflicting structured data	mediawiki/extensions/WikibaseMediaInfo	master	+41 -0

Customize query in gerrit

Related Objects

Mentioned In: T267126: Lowercase Qs in some statement URLs
T244023: GetEntities, DivergingEntityIdException: Revision 1072801264 belongs to Q354111 instead of expected d:Q354111
Mentioned Here: T204541: Drop EntityId from statement GUID
T232087: Statements not editable on a File page whose id has changed
T244654: Wikibase\Lib\Store\DivergingEntityIdException: "Revision 386521405 belongs to M43864745 instead of expected M79277318"
T231276: RevisionBasedEntityLookup.php: Revision 363395998 belongs to M77688146 instead of expected M81625979

Event Timeline

Addshore created this task.Feb 4 2020, 9:21 AM

Restricted Application added a project: Structured-Data-Backlog. · View Herald TranscriptFeb 4 2020, 9:21 AM

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

Addshore mentioned this in T244023: GetEntities, DivergingEntityIdException: Revision 1072801264 belongs to Q354111 instead of expected d:Q354111.Feb 4 2020, 9:22 AM

Cparle edited projects, added Structured-Data-Backlog (Current Work); removed Structured-Data-Backlog.Feb 4 2020, 5:24 PM

This is similar to T231276.

This basically happens when a page's history gets split (https://commons.wikimedia.org/wiki/Commons:History_merging_and_splitting#History_splitting)
In this case: https://commons.wikimedia.org/?curid=79277318 & https://commons.wikimedia.org/?curid=43864745

The original page (and its slots) gets deleted.
It gets restored (along with the relevant slots) & moved somewhere in the middle of the history.
The rest of history (along with the relevant slots) gets restored.
And we now have 2 pages with MediaInfo slots pointing to 1 original source...

Same as T244654 and T232087

Change 573315 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] [WIP] Update entity data on undelete to match new page id

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

Change 573316 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] [WIP] Don't allow restoring conflicting structured data

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

• Ramsey-WMF moved this task from Incoming to Code Review on the Structured-Data-Backlog (Current Work) board.Feb 20 2020, 6:54 PM

Change 573316 abandoned by Matthias Mullie:
[WIP] Don't allow restoring conflicting structured data

Reason:
let's not do this

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

Addshore moved this task from incoming to in progress on the Wikidata board.Feb 27 2020, 11:03 PM

Krinkle moved this task from Untriaged to Feb2020/1.35-wmf.18 on the Wikimedia-production-error board.Feb 29 2020, 7:56 PM

matthiasmullie moved this task from Code Review to Doing on the Structured-Data-Backlog (Current Work) board.Mar 5 2020, 2:50 PM

So @Addshore @thiemowmde @matthiasmullie what's the story with this https://github.com/wmde/WikibaseDataModel/pull/823 ? We don't seem to have a consensus on how to move forward

Linking T204541 also as reference to this task as it may be relevant.

I recently wrote some docs in Wikibase.git covering this element of mediawiki which Wikibase just avoids.
This patch can be found at https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/576393/

I also just wrote a rather lengthy response to some points in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikibaseMediaInfo/+/573315/ that I want to quote here for searchability etc!

I would like to argue that a statement GUID should never change, not even when this means the entity ID part in that ID doesn't match any more. It's called a globally unique identifier for a reason. If we start changing it, it's not a unique identifier any more.

It's a fair argument, but the "norm" is already that these "statement guids" do change when we talk about merges.

For example these edits:
https://test.wikidata.org/w/index.php?title=Q212044&diff=prev&oldid=527336
https://test.wikidata.org/w/index.php?title=Q212045&diff=527337&oldid=527334

Result in this guid change for the statement:
Q212044$c75a619d-4e17-a091-acbf-9496c67c7031
Q212045$ECEBF45B-DD57-4094-BC80-D4CDE946A5F0

I'm aware of a single place in the Wikibase codebase that tries to extract the entity ID from a statement GUID: the "wbsetclaim" as well as "wbsetclaimvalue" APIs allow to pass nothing but a GUID. In theory, it's possible to scan the entire database for this GUID. For performance reasons, this is not what happens, but the entity is found by extracting it's ID from the statement GUID.

The Entity type (that comes from the ID) seems to be used once in DispatchingEntityTypeStatementGrouper.
GetClaims, RemoveClaims, RemoveQualifiers, RemoveReferences, SetClaim, SetClaimValue, SetQualifier, SetReference uses the entity ID from the GUID if no entity ID is specified. Without this either the client must know which entity the GUID belongs to, or we must have an index of the current 1.1 billion statements (and future created ones too).
WikibaseQualityConstraints then also uses the entityid from statement guid (briefly discussed on https://phabricator.wikimedia.org/T204541#4601136)

But this is really just a performance optimization. Technically, there is no guarantee a statement GUID will always be constructed this way.

Right now it is an assumption enforced in code. I would call that somewhat of a guarantee right now.
The StatementGuid class also refers to:

A statement id consists of the entity id serialization

of the entity it belongs to (e.g. "Q1") and a randomly generated global unique identifier (GUID),

separated by a dollar sign.

If you are not using these APIs, there should be no need for you to touch this. The same can be achieved by using the wbeditentity API.

I believe some or most of these APIs are being used by mediainfo. And anyway, as said, the guid is specified as including the entity id that the statement belongs to, so, unless we change that definition, we should go removing the entityid from anywhere.

T204541 raised the idea of removing the entity ID before. If something like that is in peoples heads then perhaps we should focus the discussion there.
Otherwise it might make sense to keep the more fundamental discussion on T244207 for searchability in the future.

Current Wikibase "native" merging code can be found at https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/2d76a79e2be4833e8c32564bbf925fe570f41228/repo/includes/Merge/StatementsMerger.php#L44-L60
Here you can see that that statement guids are totally recreated at this point, (not needing to access the guid part of the old statement guid as it is not used)
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikibaseMediaInfo/+/573315/2/src/WikibaseMediaInfoHooks.php line 734 takes a different approach trying to reuse this guid part of the statement guid which is also why https://github.com/wmde/WikibaseDataModel/pull/823 has been created.

One way of looking at this is that no merge is happening here, in that case why should the statement guids or the smaller guids that make up part of the statement guid change?

The second way of looking at it is, these statements are indeed being moved from one entity to another, in which case the guids should probably change in the same way as when statements move in other places.

I lean toward this second option personally with everything that has so far surfaced.

A few questions to the mediainfo team that may not yet be explicitly stated anywhere:

How often is this history merge thing happening? And why? (Understanding the reason behind such merges might help out here a bit)
Is the code in the patch (keeping the guid part the same) intentional (I guess it is). And Why is that deemed necessary for the mediainfo usecase?

This circles back to the fact that probably the media info IDs should be more stable than they currently are.
Perhaps we should re asses the decision to keep these bound to the pageid of the file?

CBogen updated the task description. (Show Details)Mar 25 2020, 6:12 PM

Change 585754 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/vendor@master] "wikibase/data-model": "9.4.0"

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

Change 585754 merged by jenkins-bot:
[mediawiki/vendor@master] "wikibase/data-model": "9.4.0"

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.27; 2020-04-07).Apr 3 2020, 3:00 PM

matthiasmullie moved this task from Doing to Code Review on the Structured-Data-Backlog (Current Work) board.Apr 10 2020, 1:47 PM

• Ramsey-WMF moved this task from Code Review to Verify on Production on the Structured-Data-Backlog (Current Work) board.Apr 20 2020, 4:39 PM

Cparle moved this task from Verify on Production to Code Review on the Structured-Data-Backlog (Current Work) board.Apr 20 2020, 4:41 PM

Change 573315 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Update entity data on undelete to match new page id

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

Cparle moved this task from Code Review to Needs QA on the Structured-Data-Backlog (Current Work) board.May 1 2020, 12:59 PM

ReleaseTaggerBot edited projects, added MW-1.35-notes (1.35.0-wmf.31; 2020-05-05); removed MW-1.35-notes (1.35.0-wmf.27; 2020-04-07).May 1 2020, 1:00 PM

• Ramsey-WMF moved this task from Needs QA to Verify on Production on the Structured-Data-Backlog (Current Work) board.May 7 2020, 3:42 PM

Tested in prod (https://commons.wikimedia.org/wiki/File:2019-09-18-tenerife-teide-trees.jpg & https://commons.wikimedia.org/wiki/File:2019-09-18-tenerife-teide-trees-mv.jpg) and seems to work.

Tarrow mentioned this in T267126: Lowercase Qs in some statement URLs.Nov 9 2020, 10:22 AM

Revision 386521405 belongs to M43864745 instead of expected M79277318Closed, ResolvedPublicPRODUCTION ERRORActions