Maniphest T203424

Replace the WikiExporter backup dump streaming mode with batched queries
Closed, ResolvedPublic
Actions

Description

dumpBackup.php uses WikiExporter's streaming mode, which is to say, it sets unbuffered mode on a database connection and attempts to get all the data it needs in a single enormous SQL query. I propose breaking up this enormous query into batches and using the buffered mode.

Using the unbuffered mode is fragile and overly complicated from a development perspective. It is presumably not portable across all DBMSs. It makes the code difficult to test due to the way it interacts with PHPUnit database setup. Recently, MediaWikiTestCase::prepareConnectionForTesting() was added, solely to support testing of the backup code. I think that method is a barrier to refactoring of MediaWikiTestCase's database handling.

Using unbuffered mode may improve performance slightly. After the script consumes a row, the DB server is presumably free to do I/O work to produce the next row while PHP is busy processing the row. In production, this advantage only applies to stub dumps. Most of the time to generate dumps is spent in dumpTextPass.php which does not use unbuffered mode. So at best, unbuffered mode provides only a small improvement, while other proposed changes to the dump infrastructure, such as parallel or incremental dump generation, could improve the speed by orders of magnitude.

Unbuffered mode is an outdated style. In early versions of MediaWiki, we used it often, but the implementation complexities led to it being removed everywhere except WikiExporter and a MediaWiki 1.5 upgrade script called convertLinks.php. With this change, we will be very close to being able to remove it completely.

My proposal in detail:

Refactor dumpFrom(), split log dumps out to a separate function
In the page dump section, be consistent about rev_page/rev_id ordering
The start conditions for each batch can be: rev_page>$page or (rev_page=$page AND rev_id>$id)
Keep track of the number of revisions seen. Respect the $this->history['limit'] parameter by adjusting the batch size downwards if necessary.
The ModifyExportQuery hook will apparently work fine with batched queries

Details

Subject	Repo	Branch	Lines +/-
Minor cleanup to match changes to WikiExporter	mediawiki/extensions/Flow	wmf/1.32.0-wmf.24	+3 -3
Minor cleanup to match changes to WikiExporter	mediawiki/extensions/Flow	master	+3 -3
Replace WikiExporter streaming (unbuffered) mode with batched queries	mediawiki/core	master	+202 -241
[1.32.0-wmf.23] Logging related to WikiExport cleanup and changes in T203424	mediawiki/core	wmf/1.32.0-wmf.23	+10 -0
Logging related to WikiExport cleanup and changes in T203424	mediawiki/core	master	+10 -0
Add export logging channel	operations/mediawiki-config	master	+1 -0

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Declined		dchen	T118706 Conduct heuristic evaluation of image upload and insert flow in VisualEditor
Open		None	T115858 Design improvements for mw.ForeignStructuredUpload.BookletLayout
Open		None	T115865 Insert image in content immediately after it's uploaded, skipping the "General settings" step
Duplicate		None	T115864 Figure out if the description of the image can be used as the caption on-wiki
Open	Feature	None	T53032 When inserting an image, set its caption by default to be the Commons image description
Open	BUG REPORT	None	T12863 Links on commons upload summaries do not link to commons
Open		None	T5498 Image history is confusing
Stalled		None	T96384 Integrate file revisions with description page history
Open	Feature	None	T39534 Wikimedia Commons should support searching by color
Duplicate		None	T39535 Wikimedia Commons should support filtering by color
Resolved		None	T19503 Provide metadata support on Wikimedia Commons
Open		None	T122038 Moving/Deleting a video file does not move/delete subtitles files, nor does it inform the file mover of their existence
Open		None	T135221 Make TimedText content an integral part of the File page
Open	Feature	None	T2167 Use a dedicated interface for adding meta-data like interwiki links, rather than wikitext
Open		None	T132072 Integrate page meta-data as a new content model revision slot for consistency and atomicity
Resolved		None	T51662 VisualEditor: Use Multimedia/Wikidata's proposed rich structured meta-data in the image insertion dialog
Resolved		None	T68108 [Epic] Store media information for files on Wikimedia Commons as structured data
Open	Feature	None	T14963 allow per-page exceptions to spam blacklist
Open		None	T203157 Make the spam whitelist its own slot
Open	Feature	None	T56140 Move TemplateData to its own JSON-content namespace and associate with Template-namespace, or to its own TemplateData content model and revision slot
Stalled		None	T174043 Deploy Multi-Content Revisions
Open		None	T174022 Implement multi-content revisions
Invalid		None	T198492 Create a maintenance script to drop rev_text_id and ar_text_id from the database.
Stalled		None	T174045 DB schema migration for MCR
Resolved		Anomie	T174030 Implement DB schema migration tools for MCR
Resolved		daniel	T174028 Finalize database schema for MCR content meta-data
Resolved		Anomie	T182678 [MCR] Script for populating empty ar_rev_id fields
Resolved		Anomie	T182682 [MCR] Script(s) for populating new tables (slots, content, content_models, slot_roles)
Open		None	T183490 MCR schema migration stage 4: Migrate External Store URLs (wmf production)
Resolved		• Bstorm	T184446 Configure Toolforge replica views and dumps for the new MCR tables
Resolved		None	T231668 MCR schema migration
Resolved		Zabe	T231674 [Epic] Remove support for writing to the pre-MCR schema
Resolved		daniel	T198557 Remove the ability to write pre-MCR fields, limit the ability to read pre-MCR fields to migration scripts
Resolved		daniel	T231673 Make SCHEMA_COMPAT_NEW the default MCR migration schema
Resolved		• Clarakosi	T214308 Force usage of MCR aware database schema
Resolved		None	T231672 Stop writing to pre-MCR fields in WMF production
Resolved		None	T183487 MCR schema migration stage 3: stop using legacy fields
Resolved		Anomie	T198312 Set the WMF cluster to use the new MCR-only schema
Resolved		None	T231671 [EPIC] Ensure all direct or indirect access to pre-MCR fields is gated with the MCR migration stage and emits a warning if any pre-MCR schema fields are accessed
Resolved		CCicalese_WMF	T231689 Can get bulk access from code that has been using the 'text' for getQueryInfo()
Resolved		• Pchelolo	T230834 Create batch access interface for BlobStore
Resolved		• Pchelolo	T228988 Create batch access interface for page content
Resolved		• Pchelolo	T233173 Make RevisionStore::newRevisionsFromBatch use BlobStore::getBlobBatch
Resolved		CCicalese_WMF	T231693 No direct or indirect access to pre-MCR fields is ungated
Resolved		daniel	T198341 Remove all references to the rev_text_id and ar_text_id fields
Resolved		• Pchelolo	T198342 Remove all usages of the 'text' flag in calls to Revision::getQueryInfo() and RevisionStore::getQueryInfo().
Resolved		None	T231699 A warning is emitted if any pre-MCR fields are accessed
Resolved		None	T200918 Make sure code that accesses legacy pre-MCR fields triggers warnings before switching off WRITE_OLD (compat) mode.
Resolved		daniel	T174031 MCR: Include all slots in XML dumps
Resolved		daniel	T198706 Make BackupDumper, WikiExporter and XmlDumpWriter compliant with the MCR revision retrival mechanism (main slot only)
Resolved		BPirkle	T203424 Replace the WikiExporter backup dump streaming mode with batched queries

Event Timeline

tstarling created this task.Sep 4 2018, 4:43 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 4 2018, 4:43 AM

CCicalese_WMF moved this task from Inbox to CPT TEC1 Backlog on the Core-Platform-Team-Old board.Sep 5 2018, 1:17 AM

tstarling assigned this task to BPirkle.Sep 5 2018, 2:12 AM

I notice that WikiExporter::STREAM is also used in SpecialExport.php, for the Special:Export page, which provides similar functionality via the UI as dumpBackup.php does via the command line. Removing unbuffered mode from WikiExporter.php will necessarily (and happily) remove this as well, with only a trivial change to SpecialExport.php.

daniel added a parent task: T198706: Make BackupDumper, WikiExporter and XmlDumpWriter compliant with the MCR revision retrival mechanism (main slot only).Sep 7 2018, 12:19 PM

The dumpFrom() function (which will be split up per the task description) potentially sorts the db results by rev_timestamp in DESC order. Using this option would require callers to pass an array instead of a constant as the $history parameter to the WikiExporter constructor, which is inconsistent.

This sorting option complicates the batch conditions. While I could make it work, I don't see anywhere that functionality is used, either in core code or by any code in the extensions repository. In all cases I could find, $history is passed as a constant rather than as an array. If this functionality is indeed unused, can I take this opportunity to simplify WikiExporter by removing that entirely and recognizing only constants as valid values for $history?

FWIW, the code relevant to this sorting (search for "rev_timestamp DESC" in WikiExporter.php) has been around since the initial commit in 2015. So it wasn't recently added in support of new incoming functionality or anything of that nature.

Nevermind, I spoke too soon. While I didn't see a way to invoke the sorting code via the UI, I found https://www.mediawiki.org/wiki/Manual:Parameters_to_Special:Export, which describes using curl to invoke it manually or via script. So that seems to be an officially supported usage.

I guess I do need to work out the batching in combination with the sorting.

Change 459885 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/core@master] Replace WikiExporter streaming (unbuffered) mode with batched queries

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

gerritbot added a project: Patch-For-Review.Sep 12 2018, 1:43 AM

Change 460349 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[operations/mediawiki-config@master] Add export logging channel

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

Change 460351 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/extensions/Flow@master] Minor cleanup to match changes to WikiExporter

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

Change 461647 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/core@master] Logging related to WikiExport cleanup and changes in T203424

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

Change 460349 merged by jenkins-bot:
[operations/mediawiki-config@master] Add export logging channel

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

Change 461647 merged by jenkins-bot:
[mediawiki/core@master] Logging related to WikiExport cleanup and changes in T203424

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

Change 462844 had a related patch set uploaded (by Tim Starling; owner: BPirkle):
[mediawiki/core@wmf/1.32.0-wmf.23] [1.32.0-wmf.23] Logging related to WikiExport cleanup and changes in T203424

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-10-02 (1.32.0-wmf.24)).Sep 26 2018, 4:00 AM

Change 462844 merged by jenkins-bot:
[mediawiki/core@wmf/1.32.0-wmf.23] [1.32.0-wmf.23] Logging related to WikiExport cleanup and changes in T203424

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

ReleaseTaggerBot edited projects, added MW-1.32-notes (WMF-deploy-2018-09-25 (1.32.0-wmf.23)); removed MW-1.32-notes (WMF-deploy-2018-10-02 (1.32.0-wmf.24)).Sep 26 2018, 5:00 AM

ArielGlenn subscribed.Sep 27 2018, 11:02 AM

Regarding possible cleanup of the $history parameter, logging shows that in the (roughly) 18 hours preceding the posting of this comment, Special:Export has received 57,986 POST requests across all logged wikis (including enwiki). Of those, 17 have included the sorting option (specifically, a 'desc' value for the "dir" value of $history).

CCicalese_WMF added a project: Platform Team Legacy (Next).Oct 1 2018, 11:21 PM

CCicalese_WMF edited projects, added Core Platform Team Goals (Security, stability, performance and scalability (TEC1)); removed Core-Platform-Team-Old.Oct 2 2018, 12:04 AM

CCicalese_WMF edited projects, added Platform Engineering (Needs Cleaning - Security, stability, performance, and scalability (TEC1)); removed Core Platform Team Goals (Security, stability, performance and scalability (TEC1)).Oct 2 2018, 2:23 AM

Change 459885 merged by jenkins-bot:
[mediawiki/core@master] Replace WikiExporter streaming (unbuffered) mode with batched queries

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

ReleaseTaggerBot edited projects, added MW-1.32-notes (WMF-deploy-2018-10-02 (1.32.0-wmf.24)); removed MW-1.32-notes (WMF-deploy-2018-09-25 (1.32.0-wmf.23)).Oct 2 2018, 6:00 AM

Change 460351 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Minor cleanup to match changes to WikiExporter

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

ReleaseTaggerBot edited projects, added MW-1.32-notes (WMF-deploy-2018-10-16 (1.32.0-wmf.26)); removed MW-1.32-notes (WMF-deploy-2018-10-02 (1.32.0-wmf.24)).Oct 3 2018, 2:00 AM

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/459885/ seems to have broken Flow dumps, which use streams:

2018-10-03 11:23:53 failed history content of flow pages in xml format
[48f79972c1cea21e104aa6a5] [no req] Error from line 82 of /srv/mediawiki/php-1.32.0-wmf.24/extensions/Flow/maintenance/dumpBackup.php: Undefined class constant 'STREAM' 
Backtrace: 
#0 /srv/mediawiki/php-1.32.0-wmf.24/extensions/Flow/maintenance/dumpBackup.php(62): FlowDumpBackup->dump(integer) 
#1 /srv/mediawiki/php-1.32.0-wmf.24/maintenance/doMaintenance.php(94): FlowDumpBackup->execute() 
#2 /srv/mediawiki/php-1.32.0-wmf.24/extensions/Flow/maintenance/dumpBackup.php(144): require_once(string) 
#3 /srv/mediawiki/multiversion/MWScript.php(100): require_once(string) 
#4 {main}

I guess it needs https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Flow/+/460351/ to go out to wmf/1.32.0-wmf.24 as well.

Change 464167 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/extensions/Flow@wmf/1.32.0-wmf.24] Minor cleanup to match changes to WikiExporter

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

Change 464167 merged by jenkins-bot:
[mediawiki/extensions/Flow@wmf/1.32.0-wmf.24] Minor cleanup to match changes to WikiExporter

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

Mentioned in SAL (#wikimedia-operations) [2018-10-03T16:24:16Z] <reedy@deploy1001> Synchronized php-1.32.0-wmf.24/extensions/Flow/: fixup flow exporting T203424 (duration: 01m 03s)