Page MenuHomePhabricator

Add separate database tables for Mathoid
Closed, ResolvedPublic

Description

Author: physik

Description:
As well as LaTeXML, the Mathoid-server needs separate database tables as well.


Version: unspecified
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=65761
https://bugzilla.wikimedia.org/show_bug.cgi?id=66492
https://bugzilla.wikimedia.org/show_bug.cgi?id=66587

Details

Reference
bz65793

Related Objects

StatusAssignedTask
ResolvedNone
ResolvedNone

Event Timeline

bzimport raised the priority of this task from to High.
bzimport set Reference to bz65793.
bzimport added a subscriber: Unknown Object (MLST).
bzimport created this task.May 27 2014, 5:51 AM

Change 135521 had a related patch set uploaded by Physikerwelt:
Add seperate database tables for Mathoid

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

Change 135522 had a related patch set uploaded by Physikerwelt:
Add table for new math renderer Mathoid

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

Change 135521 merged by jenkins-bot:
Add separate database tables for Mathoid

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

physik wrote:

needs review

Why is this ticket suddenly highest priority and a major issue? Because the first patch already got merged and creates problems now?

physik wrote:

No.
There are two ways to update the database.
One is the update hook that is used on normal mediawiki instances.
The other way is to manually add the the tables.
That's the way its done for the wikimedia istallations. Thats the second unmerged change.
https://gerrit.wikimedia.org/r/#/c/135522/

Without this change we have errors in betalabs http://deployment.wikimedia.beta.wmflabs.org/
which will propagate to prodcution if no action is done.

Thanks for explaining.

(In reply to physikerwelt from comment #6)

Without this change we have errors in betalabs
http://deployment.wikimedia.beta.wmflabs.org/
which will propagate to prodcution if no action is done.

First patch was merged on 2014-05-27 so it should have made it into 1.24wmf7 already, according to https://www.mediawiki.org/wiki/MediaWiki_1.24/Roadmap ?

As you used future tense ("errors *will* propagate to production if no action is done") I wonder what I'm missing, or how I misinterpret the deployment schedule in this case (as I would expect the errors to already be in production).

(In reply to physikerwelt from comment #6)

Without this change we have errors in betalabs
http://deployment.wikimedia.beta.wmflabs.org/
which will propagate to prodcution if no action is done.

Uhm, is the database table *required* by the code right now? The schema change needs to be done before that (see https://wikitech.wikimedia.org/wiki/Schema_changes).

physik wrote:

The first changes changes the Math extension code. This is sufficient for normal mediawiki instances. The second change sets up the new additional table for Wikimedia instances. This change is required for the new experimental MathML rendering option for Wikimedia instances that do not update via the update hook.

Let me rephrase, if the table does not exist on WMF wikis, will anything break in the next deployment?

physik wrote:

Only useres that want to test the MathML rendering will get a database error. That might cause many bug reports.

I'm not seeing any errors at http://deployment.wikimedia.beta.wmflabs.org/wiki/User:Anomie/Sandbox after enabling the "<mw_math_mathml>" option. Nor do I see any changes here that are both merged and not already deployed to all WMF production wikis. Nor would Gerrit change 135522 fix anything besides the process of creating a new wiki; in particular, it wouldn't fix all existing wikis were they somehow broken.

physikerwelt, please review https://wikitech.wikimedia.org/wiki/Schema_changes for the correct way to do database schema changes. In short, schema changes on the WMF production databases need to be done manually by the DBA. If something actually is broken, the solution is to revert it until the schema change is complete rather than claiming your schema change is a major highest-priority issue.

physik wrote:

Nor do I see any changes here that are both merged and not already deployed to all WMF production wikis.

Ok the error I had seems to be gone. On the other hand this means that the new table must have been created.
On the page you linked I see a proper rendered SVG Image with Chrome. Which is the expected behaiviour.
I read through the schema changes page and I'll try to file a bug.
However, for new Wiki's the new table should be added.

greg added a comment.Jun 12 2014, 7:27 PM

Tables are autocreated on the Beta Cluster, not on production, just fyi.

ori added a comment.Jun 13 2014, 12:51 AM

I reverted to 1bb3bfa3b5656 because the exception log was flooded with errors related to the math extension. I also revoked +2 rights on extension/Math from Physikerwelt and Frédéric Wang until Greg has had a chance to review exactly what happened.

Exception rate has gone down since Ori's revert, but users who have MathJax and source Tex mode enabled in their preferences will still see errors:

in math source mode no database caching should happen
#0 /usr/local/apache/common-local/php-1.24wmf8/extensions/Math/MathRenderer.php(297): MathSource->getMathTableName()
#1 /usr/local/apache/common-local/php-1.24wmf8/extensions/Math/MathRenderer.php(359): MathRenderer->writeToDatabase()
#2 /usr/local/apache/common-local/php-1.24wmf8/extensions/Math/Math.hooks.php(130): MathRenderer->writeCache()
#3 [internal function]: MathHooks::mathTagHook('\omega_0', Array, Object(Parser), Object(PPFrame_DOM))
#4 /usr/local/apache/common-local/php-1.24wmf8/includes/parser/Parser.php(4102): call_user_func_array(Array, Array)
#5 /usr/local/apache/common-local/php-1.24wmf8/includes/parser/Preprocessor_DOM.php(1242): Parser->extensionSubstitution(Array, Object(PPFrame_DOM))
#6 /usr/local/apache/common-local/php-1.24wmf8/includes/parser/Parser.php(3227): PPFrame_DOM->expand(Object(PPNode_DOM), 0)
#7 /usr/local/apache/common-local/php-1.24wmf8/includes/parser/Parser.php(1239): Parser->replaceVariables('{{Other uses}}?...')
#8 /usr/local/apache/common-local/php-1.24wmf8/includes/parser/Parser.php(405): Parser->internalParse('{{Other uses}}?...')
#9 [internal function]: Parser->parse('{{Other uses}}?...', Object(Title), Object(ParserOptions), true, true, 612692326)

(easy article to reproduce on is https://en.wikipedia.org/wiki/Omega)

physik wrote:

I can see the problem on Wikipedia. But I can not reproduce it locally, even if I delete the matoid table.
The code in the MathSource.php is
protected function getMathTableName() {

		throw new MWException ( 'in math source mode no database caching should happen');

}
Which does not have any function beside preventing unexpected behavior.

physik wrote:

@ori:
Is there a chance to see the reverted change. I do not find anything here
https://git.wikimedia.org/summary/mediawiki%2Fextensions%2FMath

physik wrote:

OK. The branch used in production is wmf8 not wmf9.
I don't see any problems with wmf9.
With wmf8 I can reproduce the problems locally.
How to make progress on that problem?

physik wrote:

In particular this problem in wmf8 is that
MathSource.php does not do the unchanged assumption.
See
https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FMath/8fe1de94d67e845186192afd168dc11c1b6bb0e1/MathSource.php#L59

		// assume unchanged to avoid unnecessary database access
		$this->changed = false;

If there is an increased number of database write operations for the PNG mode as well?

physik wrote:

This commit is missing:
https://gerrit.wikimedia.org/r/#/c/138023/
I could image that the wmf8 code produces database write operation that are not required.

(In reply to Ori Livneh from comment #15)

I also revoked +2 rights on
extension/Math from Physikerwelt and Frédéric Wang

Per the +2 policy, Brion Vibber, Tim Starling, Mark Bergsma, and Erik Möller (or his delegate) can sign off on +2 revocations. Did Erik give you permission to do this?

greg added a comment.Jun 16 2014, 7:24 PM

(In reply to Ori Livneh from comment #15)

I also revoked +2 rights on
extension/Math from Physikerwelt and Frédéric Wang until Greg has had a
chance to review exactly what happened.

I'm comfortable with granting back +2 to Physikerwelt after having a (very fruitful) conversation with him on Friday. I assume he'll inform Frédéric of the pertinent salient points.

ori added a comment.Jun 16 2014, 7:26 PM

(In reply to Greg Grossmeier from comment #23)

(In reply to Ori Livneh from comment #15)
> I also revoked +2 rights on
> extension/Math from Physikerwelt and Frédéric Wang until Greg has had a
> chance to review exactly what happened.

I'm comfortable with granting back +2 to Physikerwelt after having a (very
fruitful) conversation with him on Friday. I assume he'll inform Frédéric of
the pertinent salient points.

Done!

Sooooo.... Who wants to answer my question?

physik wrote:

I think it would be great if a user would be informed about a change of the rights. I would not even have realized that my rights were changed.
Due to urgent activities related to my job, I did not have a chance to talk to Frédéric. I could image that he has not realized anything because he is not in the CC list.
I have not used +2 in the mean time but at least the menu did not change.
Is there a systematic procedure for getting +2 rights?
I assigned the +2 right to Frédéric Wang and pointed him to the guide I wrote to the best of my knowledge that I wrote to attract new reviewers.
http://www.formulasearchengine.com/review (see the PDF file)
Maybe it was a bug that was able to assign +2 rights to that.
As I explain on https://en.wikipedia.org/wiki/Wikipedia_talk:WikiProject_Mathematics#Comments_on_draft
the main problem what I see for the math extension development, is that there is no fixed contact point at WMF for the math extension development.
Even though all! individual developers I meat at WMF were very helpful and really nice guys nobody took the main responsibility for the Math extension development.
For me it is not suitable to mange the Math extension development on an island without a fixed interface to the other developments happening.

physik wrote:

There is still an open change
https://gerrit.wikimedia.org/r/#/c/135522/
Feel free to abandon it, if this is not required.

Change 135522 merged by jenkins-bot:
Add table for new math renderer Mathoid

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