Page MenuHomePhabricator
Create Task
Maniphest T286877

newRevisionSlots argument check breaks stub dumps for svwiki
Closed, ResolvedPublic
Actions

Description

Incremental dumps are broken for svwiki, and the regular stubs will be broken too when they run tomorrow:

[07052587822907ecbf7c3687] [no req]   TypeError: Argument 5 passed to MediaWiki\Revision\RevisionStore::newRevisionSlots() must implement interface MediaWiki\Page\PageIdentity, null given, called in /srv/mediawiki/php-1.37.0-wmf.14/includes/Revision/RevisionStore.php on line 1713
Backtrace:
from /srv/mediawiki/php-1.37.0-wmf.14/includes/Revision/RevisionStore.php(1487)
#0 /srv/mediawiki/php-1.37.0-wmf.14/includes/Revision/RevisionStore.php(1713): MediaWiki\Revision\RevisionStore->newRevisionSlots(string, stdClass, array, integer, NULL)
#1 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/XmlDumpWriter.php(346): MediaWiki\Revision\RevisionStore->newRevisionFromRowAndSlots(stdClass, array, integer, NULL)
#2 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(542): XmlDumpWriter->writeRevision(stdClass, array)
#3 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(485): WikiExporter->outputPageStreamBatch(Wikimedia\Rdbms\ResultWrapper, stdClass)
#4 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(306): WikiExporter->dumpPages(string, boolean)
#5 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(204): WikiExporter->dumpFrom(string)
#6 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/includes/BackupDumper.php(318): WikiExporter->revsByRange(integer, integer)
#7 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/dumpBackup.php(90): BackupDumper->dump(integer, integer)
#8 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/doMaintenance.php(108): DumpBackup->execute()
#9 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/dumpBackup.php(144): require_once(string)
#10 /srv/mediawiki/multiversion/MWScript.php(116): require_once(string)
#11 {main}

Details

SubjectRepoBranchLines +/-
prevent PageIdentity checks in RevisionStore from breaking xml dumpsmediawiki/coremaster+8 -2
prevent PageIdentity checks in RevisionStore from breaking xml dumpsmediawiki/corewmf/1.37.0-wmf.14+8 -2
Add sanity check to newRevisionFromRowAndSlots.mediawiki/corewmf/1.37.0-wmf.14+9 -0
Add sanity check to newRevisionFromRowAndSlots.mediawiki/coremaster+9 -0
Customize query in gerrit

Related Objects

Event Timeline

ArielGlenn triaged this task as High priority.Jul 19 2021, 6:44 AM
ArielGlenn created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 19 2021, 6:44 AM
ArielGlenn added projects: Dumps-Generation, Platform Team Workboards (Clinic Duty Team).Jul 19 2021, 6:45 AM
ArielGlenn added a comment.Jul 19 2021, 6:51 AM

I guess there's code in stubs generation that needs to add TypeError to everything else being caught.

gerritbot added a comment.Jul 19 2021, 7:52 AM

Change 705346 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/core@master] prevent PageIdentity checks in RevisionStore from breaking xml dumps

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

gerritbot added a project: Patch-For-Review.Jul 19 2021, 7:52 AM
ArielGlenn added a comment.Jul 19 2021, 7:55 AM

The above has been tested and I used it to manually complete the broken adds-changes run from last night/this morning.

ArielGlenn added a comment.Jul 19 2021, 8:08 AM

This case is interesting and should be investigated further, even after the dumps are back to running.

The page in question, "Stockholm Business School", was restored on July 18, see https://sv.wikipedia.org/w/index.php?title=Stockholm_Business_School&action=history&uselang=en
The entry in the page table for this title:

wikiadmin@10.64.0.99(svwiki)> select * from page where page_namespace=0 and page_title = "Stockholm_Business_School";
+---------+----------------+---------------------------+-------------------+------------------+-------------+----------------+----------------+--------------------+-------------+----------+--------------------+-----------+
| page_id | page_namespace | page_title                | page_restrictions | page_is_redirect | page_is_new | page_random    | page_touched   | page_links_updated | page_latest | page_len | page_content_model | page_lang |
+---------+----------------+---------------------------+-------------------+------------------+-------------+----------------+----------------+--------------------+-------------+----------+--------------------+-----------+
| 3872662 |              0 | Stockholm_Business_School |                   |                0 |           0 | 0.207856671786 | 20210718185812 | 20210718185812     |    49457568 |     7354 | wikitext           | NULL      |
+---------+----------------+---------------------------+-------------------+------------------+-------------+----------------+----------------+--------------------+-------------+----------+--------------------+-----------+
1 row in set (0.001 sec)

However there are entries in the archive table for this same title:

wikiadmin@10.64.0.99(svwiki)> select * from archive where ar_namespace=0 and  ar_title = "Stockholm_Business_School";
+---------+--------------+---------------------------+---------------+----------+----------------+---------------+-----------+------------+--------+------------+--------------+---------------------------------+
| ar_id   | ar_namespace | ar_title                  | ar_comment_id | ar_actor | ar_timestamp   | ar_minor_edit | ar_rev_id | ar_deleted | ar_len | ar_page_id | ar_parent_id | ar_sha1                         |
+---------+--------------+---------------------------+---------------+----------+----------------+---------------+-----------+------------+--------+------------+--------------+---------------------------------+
| 5215947 |            0 | Stockholm_Business_School |      23822663 |     1136 | 20210717092556 |             0 |  49453482 |          0 |     78 |    8579229 |            0 | i5aq79phzojjd2mn019iqikye1cns89 |
| 5217451 |            0 | Stockholm_Business_School |      23827934 |     1136 | 20210718100730 |             0 |  49456280 |          0 |     75 |    8579418 |            0 | ilxrsk5zyft2zwhht95kw234yzgxced |
+---------+--------------+---------------------------+---------------+----------+----------------+---------------+-----------+------------+--------+------------+--------------+---------------------------------+
2 rows in set (0.001 sec)

When it comes time to try to iterate over the revisions, the wrong page id must have been chosen, as I see it has been written out by XmlDumpWriter:

--<snip--
...
  <page>
    <title>Dubai Millennium</title>
    <ns>0</ns>
    <id>8579417</id>
    <revision>
      <id>49456257</id>
      <timestamp>2021-07-18T09:59:18Z</timestamp>
      <contributor>
        <username>Kuriosatempel</username>
        <id>111748</id>
      </contributor>
      <comment>[[Wikipedia:Automatisk sammanfattning|←]]Skapade sidan med '{{Häst | namn = Dubai Millennium | bild =  | bildtext =  | kön = [[Hingst]] | född = {{Sportdatum|1996|3|2}} | födelseland = Storbritannien | död = {{Hästdöd datum och ålder|2001|04|29|1996|03|02}} | död_land = Storbritannien | färg = [[Hästfärg|Brun]] | tecken =  | ras = [[Engelskt fullblod]] | sport = Galoppsport | aktiv = 1998–2000 | efter = [[Seeking The Gold]] | under = [[Colorado Dancer]] | underefter = Sha...'</comment>
      <model>wikitext</model>
      <format>text/x-wiki</format>
      <text bytes="10912" id="49550759" />
      <sha1>p0stf8dyt26b3ipsp171jtwt7smlzkh</sha1>
    </revision>
  </page>
  <
    <title>Stockholm Business School</title>
    <ns>0</ns>
    <id>8579418</id>

That page id isn't in the page table of course:

wikiadmin@10.64.0.99(svwiki)> select * from page where page_id=8579418;
Empty set (0.001 sec)

Naturally nothing good is going to come from that. So my question is, why are those rows still in the archive table after page restoration, and how does the page id from those rows wind up being the unlucky id chosen for use?

daniel subscribed.Jul 19 2021, 11:51 AM

A case like this should trigger a RevisionAccessException, not a TypeError. Looking at newRevisionFromRowAndSlots(), I don't see how $page can end up being null in the call to newRevisionSlots(). But I can just add a check and throw the correct exception anyway.

gerritbot added a comment.Jul 19 2021, 11:57 AM

Change 705368 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Add sanity check to newRevisionFromRowAndSlots.

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

gerritbot added a comment.Jul 19 2021, 2:47 PM

Change 705368 merged by jenkins-bot:

[mediawiki/core@master] Add sanity check to newRevisionFromRowAndSlots.

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

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.15; 2021-07-19).Jul 19 2021, 3:00 PM
ArielGlenn added a comment.Jul 19 2021, 3:07 PM
In T286877#7220894, @daniel wrote:

A case like this should trigger a RevisionAccessException, not a TypeError. Looking at newRevisionFromRowAndSlots(), I don't see how $page can end up being null in the call to newRevisionSlots(). But I can just add a check and throw the correct exception anyway.

This is not enough. Run with the new code:

MediaWiki\Revision\RevisionAccessException from line 1695 of /srv/mediawiki/php-1.37.0-wmf.14/includes/Revision/RevisionStore.php: Failed to determine page associated with revision 49456280
#0 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/XmlDumpWriter.php(346): MediaWiki\Revision\RevisionStore->newRevisionFromRowAndSlots(Object(stdClass), Array, 0, NULL)
#1 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(542): XmlDumpWriter->writeRevision(Object(stdClass), Array)
#2 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(485): WikiExporter->outputPageStreamBatch(Object(Wikimedia\Rdbms\ResultWrapper), Object(stdClass))
#3 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(306): WikiExporter->dumpPages('rev_id >= 49453...', false)
#4 /srv/mediawiki/php-1.37.0-wmf.14/includes/export/WikiExporter.php(204): WikiExporter->dumpFrom('rev_id >= 49453...')
#5 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/includes/BackupDumper.php(318): WikiExporter->revsByRange(49453805, 49456452)
#6 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/dumpBackup.php(90): BackupDumper->dump(16, 1)
#7 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/doMaintenance.php(108): DumpBackup->execute()
#8 /srv/mediawiki/php-1.37.0-wmf.14/maintenance/dumpBackup.php(144): require_once('/srv/mediawiki/...')
#9 /srv/mediawiki/multiversion/MWScript.php(116): require_once('/srv/mediawiki/...')
#10 {main}

Shall I catch this in XmlDumpWriter (instead of TypeError)?

daniel added a comment.EditedJul 19 2021, 3:37 PM

Yes, XmlDumpWriter (or WikiExporter) should catch RevisionAccessException. I was assuming it was already doing that

gerritbot added a comment.Jul 19 2021, 6:07 PM

Change 705448 had a related patch set uploaded (by Ahmon Dancy; author: Daniel Kinzler):

[mediawiki/core@wmf/1.37.0-wmf.14] Add sanity check to newRevisionFromRowAndSlots.

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

gerritbot added a comment.Jul 19 2021, 7:12 PM

Change 705448 merged by jenkins-bot:

[mediawiki/core@wmf/1.37.0-wmf.14] Add sanity check to newRevisionFromRowAndSlots.

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

Stashbot added a comment.Jul 19 2021, 7:14 PM

Mentioned in SAL (#wikimedia-operations) [2021-07-19T19:14:03Z] <dancy@deploy1002> Synchronized php-1.37.0-wmf.14/includes/Revision/RevisionStore.php: Backport: [[gerrit:705448|Add sanity check to newRevisionFromRowAndSlots. (T286877)]] (duration: 00m 57s)

gerritbot added a comment.Jul 19 2021, 7:41 PM

Change 705346 merged by jenkins-bot:

[mediawiki/core@master] prevent PageIdentity checks in RevisionStore from breaking xml dumps

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

gerritbot added a comment.Jul 19 2021, 7:49 PM

Change 705467 had a related patch set uploaded (by Ahmon Dancy; author: ArielGlenn):

[mediawiki/core@wmf/1.37.0-wmf.14] prevent PageIdentity checks in RevisionStore from breaking xml dumps

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

ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.14; 2021-07-12); removed MW-1.37-notes (1.37.0-wmf.15; 2021-07-19).Jul 19 2021, 8:00 PM
gerritbot added a comment.Jul 19 2021, 8:06 PM

Change 705467 merged by jenkins-bot:

[mediawiki/core@wmf/1.37.0-wmf.14] prevent PageIdentity checks in RevisionStore from breaking xml dumps

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

Stashbot added a comment.Jul 19 2021, 8:08 PM

Mentioned in SAL (#wikimedia-operations) [2021-07-19T20:08:31Z] <dancy@deploy1002> Synchronized php-1.37.0-wmf.14/includes/export/WikiExporter.php: Backport: [[gerrit:705467|prevent PageIdentity checks in RevisionStore from breaking xml dumps (T286877)]] (duration: 00m 58s)

ArielGlenn moved this task from Backlog to Active on the Dumps-Generation board.Jul 20 2021, 4:54 AM
ArielGlenn mentioned this in T291164: undelete sometimes leaves the association between pages and their revisions in a strange state.Sep 16 2021, 9:57 AM
daniel closed this task as Resolved.Sep 16 2021, 10:00 AM
daniel claimed this task.

The root cause is still unclear, filed as T291164

ArielGlenn moved this task from Active to Done on the Dumps-Generation board.Feb 19 2022, 6:46 AM
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