Page MenuHomePhabricator

Comparing revisions can fatal (timeout from wikidiff2 via TextSlotDiffRenderer)
Open, MediumPublic5 Estimated Story PointsPRODUCTION ERROR

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 11 2018, 1:10 AM

You can find the logs by going to, selecting "Dev Tools" and entering the query

GET _search
  "query": {
    "query_string": {
      "fields": ["_type", "channel", "@timestamp", "fatal_exception.trace"],
      "query": "_type:mediawiki AND channel:fatal AND fatal_exception.trace:TextSlotDiffRenderer"

I don't use the dashboard interface of logstash anymore because it freezes my browser for tens of seconds (T189333)

tstarling updated the task description. (Show Details)Sep 11 2018, 3:08 AM
mmodell added a subscriber: mmodell.EditedApr 10 2019, 7:33 PM

Seeing quite a bit of this error in logstash:

Fatal error: entire web request took longer than 60 seconds and timed out in /srv/mediawiki/php-1.33.0-wmf.24/includes/diff/TextSlotDiffRenderer.php on line 219

Specifically, 40 occurrences in the past 15 minutes.

Krinkle renamed this task from Timeouts in wikidiff2 to Comparing revisions can fatal (timeout from wikidiff2 via TextSlotDiffRenderer).Apr 11 2019, 1:04 AM
Krinkle added a subscriber: Krinkle.EditedApr 11 2019, 1:13 AM

Would is be possible (in a relatively easy not to crazy investment kind of way) to artificially limit time spent in wikidiff2's main logic?

This would effectively be similar to what we do with ElasticSearch timeouts (which may be easier to cap, given it's over the network). The main point being that it avoids exposing uncached urls producing HTTP 500 responses.

And, it could still render the meta data of both revisions (timestamp, author, edit summary, etc.), the complete wikitext of both revisions skin-by-side (without difference). Perhaps with a notice indicating that the diff could not be generated.

I believe the Visual Diff beta feature also has a timeout where a more basic version is used as fallback. This would mean that the comparison API and HTML table format would still be rendered, just in a more dull manner.

WMDE-Fisch added a subscriber: WMDE-Fisch.

Possibly unrelated, but curious: I also get a timeout for the the page in the first test case (the page itself, not the diff):

PHP fatal error:
entire web request took longer than 60 seconds and timed out

Change 506148 had a related patch set uploaded (by Jkroll; owner: Jkroll):
[mediawiki/php/wikidiff2@master] [WiP] Correct AllowPrintMovedLineDiff to count lines, not edits

WMDE-Fisch set the point value for this task to 5.
thiemowmde triaged this task as Medium priority.Apr 30 2019, 1:32 PM
thiemowmde added a subscriber: thiemowmde.
awight added a subscriber: awight.May 10 2019, 8:30 AM

My extremely unscientific sampling of diffs, using this patch: P8510, found that in benchmarks/revs.json.gz, there were 244 total chunks, with:

php benchmarks/bench.php | grep chunk | awk -e '{adds += $3} {dels += $6} END {print adds; print dels}'

505 added lines and 493 deleted lines, or (505 + 493) / 244 lines per chunk = approximately 4 lines per chunk. This is an estimate of the initial factor we should use to increase the chunk threshold to convert it to a line count threshold.

Based on the factor above, I would suggest this patch:

--- a/DiffEngine.h
+++ b/DiffEngine.h
@@ -33,7 +33,7 @@
 // Default value for INI setting wikidiff2.moved_line_threshold
 // Default value for INI setting wikidiff2.moved_paragraph_detection_cutoff

 inline void debugLog(const char *fmt, ...) {

Change 506148 merged by jenkins-bot:
[mediawiki/php/wikidiff2@master] Correct AllowPrintMovedLineDiff to count lines, not edits

MaxSem added a subscriber: MaxSem.Jul 26 2019, 2:48 AM

You might want to investigate the algorithm in DiffEngine.php, formerly known as Wikidiff3. While wikidiff2 was based on the previous MediaWiki diff implementation, wikidiff3 has a drastically improved handing of worst case scenarios at the expense of slightly decreased diff quality (in theory; in practice in my limited user testing people liked Wikidiff3 more).

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:09 PM
thiemowmde removed jkroll as the assignee of this task.Nov 12 2019, 1:15 PM