Page MenuHomePhabricator

Graphs are constantly being regenerated, no matter what
Open, Needs TriagePublic

Description

Setup

  • MediaWiki 1.32.0 (6573069) 17:52, 17. Jan. 2019
  • PHP 7.0.33-0+deb9u1 (apache2handler)
  • MariaDB 10.1.37-MariaDB-0+deb9u1
  • Semantic MediaWiki 3.1.0-alpha (ed47685) 09:07, 4. Feb. 2019
  • GraphViz 3.0.0 (d130786) 22:13, 13. Jan. 2019 // Note that this is current master. Release 3.1.0 registers as 3.0.0

Issue
The graphs regenerate in minimum every time I rebuild the semantic data using Semantic MediaWiki's "rebuildData.php" script. See this example. However the issue is sticking around much longer as this example shows. There are even graphs being regenerated without a page actively holding code for generating graphs as this example shows.

For this test wiki it is no problem but this issue is more than critical for production.

Details

Related Gerrit Patches:
mediawiki/extensions/GraphViz : masterDon't upload graphs as wiki files

Event Timeline

Kghbln created this task.Feb 6 2019, 9:59 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 6 2019, 9:59 AM

@Samwilson I have the feeling that this may be connected to the special autogeneration feature for anonymous users that was implemented with release 3.1.0 which by the way is still not working.

I must admit, every time I look at this extension I wonder again why it's uploading the generated graph files at all. :-)

But anyway I'll have a look at this and see what's wrong. I guess it's not correctly checking that there's been no modification to the graph source. And dot doesn't produce deterministic output, so the duplicates aren't caught by the upload mechanism.

Kghbln added a comment.Feb 6 2019, 2:46 PM

No worries. I am just happy that you are sticking around and that you care about this extension. :)

I am not absolutely sure but I just had a look at the error log and saw this:

Issue

Re: Graph Process Example.

2019-02-11 15:20:41 phalaris 0210020150926-02100_: [0fb1483df5eb357d80b57236] /wiki/Graph_Process_Example   MWException from line 4238 of /../w/includes/user/User.php: CAS update failed on user_touched. The version of the user to be saved is older than the current version.

Stacktrace

#0 /../w/includes/libs/rdbms/database/Database.php(3815): User->{closure}(Wikimedia\Rdbms\DatabaseMysqli, string)
#1 /../w/includes/user/User.php(4251): Wikimedia\Rdbms\Database->doAtomicSection(string, Closure)
#2 /../w/includes/user/User.php(877): User->saveSettings()
#3 /../w/extensions/GraphViz/includes/GraphViz.php(465): User::newSystemUser(string, array)
#4 /../w/extensions/GraphViz/includes/GraphViz.php(641): MediaWiki\Extension\GraphViz\GraphViz::getUser()
#5 /../w/extensions/GraphViz/includes/GraphViz.php(444): MediaWiki\Extension\GraphViz\GraphViz::render(string, array, Parser, PPFrame_DOM)
#6 /../w/includes/parser/Parser.php(3968): MediaWiki\Extension\GraphViz\GraphViz::graphvizParserHook(string, array, Parser, PPFrame_DOM)
#7 /../w/includes/parser/Preprocessor_DOM.php(1364): Parser->extensionSubstitution(array, PPFrame_DOM)
#8 /../w/includes/parser/Parser.php(3014): PPFrame_DOM->expand(DOMElement, integer)
#9 /../w/includes/parser/Parser.php(1350): Parser->replaceVariables(string)
#10 /../w/includes/parser/Parser.php(684): Parser->internalParse(string, boolean, boolean)
#11 /../w/extensions/SemanticResultFormats/formats/graphviz/SRF_Graph.php(136): Parser->recursiveTagParse(string)
#12 /../w/extensions/SemanticMediaWiki/src/Query/ResultPrinters/ResultPrinter.php(342): SRFGraph->getResultText(SMWQueryResult, integer)
#13 /../w/extensions/SemanticMediaWiki/src/Query/ResultPrinters/ResultPrinter.php(307): SMW\Query\ResultPrinters\ResultPrinter->buildResult(SMWQueryResult)
#14 /../w/extensions/SemanticMediaWiki/includes/query/SMW_QueryProcessor.php(388): SMW\Query\ResultPrinters\ResultPrinter->getResult(SMWQueryResult, array, integer)
#15 /../w/extensions/SemanticMediaWiki/src/ParserFunctions/AskParserFunction.php(344): SMWQueryProcessor::getResultFromQuery(SMWQuery, array, integer, integer)
#16 /../w/extensions/SemanticMediaWiki/src/ParserFunctions/AskParserFunction.php(183): SMW\ParserFunctions\AskParserFunction->doFetchResultsFromFunctionParameters(array, array)
#17 /../w/extensions/SemanticMediaWiki/src/ParserFunctionFactory.php(388): SMW\ParserFunctions\AskParserFunction->parse(array)
#18 /../w/includes/parser/Parser.php(3493): SMW\ParserFunctionFactory->SMW\{closure}(Parser, string, string, string, string, string, string, string, string, string, string, string, string, string)
#19 /../w/includes/parser/Parser.php(3200): Parser->callParserFunction(PPFrame_DOM, string, array)
#20 /../w/includes/parser/Preprocessor_DOM.php(1279): Parser->braceSubstitution(array, PPFrame_DOM)
#21 /../w/includes/parser/Parser.php(3014): PPFrame_DOM->expand(DOMElement, integer)
#22 /../w/includes/parser/Parser.php(1350): Parser->replaceVariables(string)
#23 /../w/includes/parser/Parser.php(476): Parser->internalParse(string)
#24 /../w/includes/content/WikitextContent.php(341): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#25 /../w/includes/content/AbstractContent.php(517): WikitextContent->fillParserOutput(Title, integer, ParserOptions, boolean, ParserOutput)
#26 /../w/includes/Revision/RenderedRevision.php(242): AbstractContent->getParserOutput(Title, integer, ParserOptions, boolean)
#27 /../w/includes/Revision/RenderedRevision.php(211): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(WikitextContent, boolean)
#28 /../w/includes/Revision/RevisionRenderer.php(175): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string)
#29 /../w/includes/Revision/RevisionRenderer.php(128): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#30 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#31 /../w/includes/Revision/RenderedRevision.php(175): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#32 /../w/includes/poolcounter/PoolWorkArticleView.php(194): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#33 /../w/includes/poolcounter/PoolCounterWork.php(123): PoolWorkArticleView->doWork()
#34 /../w/includes/page/Article.php(774): PoolCounterWork->execute()
#35 /../w/includes/actions/ViewAction.php(68): Article->view()
#36 /../w/includes/MediaWiki.php(501): ViewAction->show()
#37 /../w/includes/MediaWiki.php(294): MediaWiki->performAction(Article, Title)
#38 /../w/includes/MediaWiki.php(860): MediaWiki->performRequest()
#39 /../w/includes/MediaWiki.php(517): MediaWiki->main()
#40 /../w/index.php(42): MediaWiki->run()
#41 {main}

Change 505446 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/GraphViz@master] [WIP] Don't upload graphs as wiki files

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

Does anyone think it might be a good idea to get rid of the uploading of GraphViz images as wiki files?

As far as I understand, it's done because the ImageMap extension requires it. What if we remove that restriction, and just generate the images independently and link to them directly? I'm experimenting with this approach in the above patch.

Does anyone think it might be a good idea to get rid of the uploading of GraphViz images as wiki files?

Depends I guess: If it will still be possible to link from areas of the generated graphs to some page like in e.g. Mise en place d'un wiki when I have nothing against it. Otherwise we would loose a pretty important feature.

Yes, definitely we'll be able to keep the imagemap functionality of linking nodes to pages. The only user-facing breaking change would be that it'd no longer be possible to generate a graph on one page and then refer to that graph on another page via normal wiki syntax e.g. [[File:GraphViz digraph example1 dot.png]] but I suspect that's not a very common thing to do.

I'll keep working on the above patch.

There seems to be a few existing bugs around image-handling, such as when you provide an image name that is just numbers, but I think we can clean those up as we go.

The above patch is ready for review, if anyone's able to have a look.

I ran into an issue where graphs were being regenerated because of a bug in LocalFile. I have since pushed a fix for it, and now our graphs are not being regenerated unless their source changes.

In terms of this fix, as I understand it, it would no longer be possible to use a different File Backend to store graph images? Please let me know if I'm wrong. My issue would be that we do not have the images in the local images folder, but on a external cloud blob storage. Having several wiki server instances with limited storage would present issues with this solution.

That's a good point, thanks @osorio-juan-microsoft.

How are previews meant to be handled in the non-local file repo scenario? I guess the non-hacky way that would follow the way things are currently done would be to upload them as files themselves (with the user-specific filename suffix). At the moment we are manually modifying the image table, which feels wrong.

Or is there some way to expose a generated image to the client, without it being registered in the file repository? Although, I can't see how that'd work and also allow the ImageMap extension access to it.

Prod added a subscriber: Prod.Jul 18 2019, 4:25 AM