Page MenuHomePhabricator

Zeroconf VisualEditor/Parsoid doesn't work on SQLite
Closed, ResolvedPublic

Description

I was testing the MediaWiki-Debian package (though I believe it's applicable to vanilla MediaWiki) and discovered that VE/Parsoid isn't working on SQLite. Read requests seem to work, in that I'm able to open the editor, change stuff, but when I try to save, I get Error contacting the Parsoid/RESTBase server: (curl error: 28) Timeout was reached.

My hunch is that the loopback is causing a problem in that the first API action=visualeditoredit request holds some lock on the database, and then the loopback request times out because it can't get the lock.

Everything works fine on MySQL.

Event Timeline

Confirmed that it's an issue with not being able to open the database while another request has it open. My debug log hangs at:

[DBConnection] Wikimedia\Rdbms\LoadBalancer::getLocalConnection: opened new connection for 0

and then after a while I'll see:

[http] Error fetching URL "http://localhost:4881/rest.php/localhost/v3/transform/html/to/wikitext/Main_Page/6": (curl error: 28) Timeout was reached

My understanding is that SQLite by default has no write concurrency, but I'm not sure about reads...

Both @cscott and I were wondering why we need to support SQLite. Is this a real use case out there? I say we don't bother with this.

Both @cscott and I were wondering why we need to support SQLite. Is this a real use case out there? I say we don't bother with this.

SQLite is one of the 3 supported database engines by MediaWiki and it's a requirement that it (along with MySQL/Postgres) be supported for inclusion in the tarball. We currently don't have any mechanism in the installer to indicate that an extension doesn't support a particular database engine.

@tstarling and I also discussed this on IRC, he pointed out that SQLite doesn't handle concurrency well and that in his experience, upstream isn't interested in fixing it and his position is that people using SQLite should just move to MySQL. I think that's fine as a long-term goal but doesn't fix our current problem.

I'll keep taking stabs at this in any case since I actually run a SQLite MediaWiki instance in "production".

We could potentially just tweak the message associated with sqlite. Eg config-dbsupport-sqlite in en.json says:

* [{{int:version-db-sqlite-url}} SQLite] is a lightweight database system that is very well supported. ([https://www.php.net/manual/en/pdo.installation.php How to compile PHP with SQLite support], uses PDO)

You could add something like, "however, it does not support VisualEditor", to that message.

There is also a config-outdated-sqlite message warnings about outdated SQLite; you could imagine something similar that just warned that VisualEditor isn't supported if sqlite is selected.

Perhaps $wgJobRunRate is the culprit. If it is > 0, then jobs are going to be run on both the VE action API request and the Parsoid REST API request (as I understand it), and those jobs could fight with Parsoid for a db lock.

Change 622258 had a related patch set uploaded (by Lens0021; owner: Lens0021):
[mediawiki/extensions/Flow@master] Do not throw exception for zero-conf Parsoid

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

Lens0021 added a subscriber: Lens0021.

Sorry, I mistakenly uploaded the patch

@Legoktm any progress on this? It seems like any fix is going to miss the 1.35 train if we don't figure it out soon.

Might be related to T118629. If you can reproduce, try setting $wgMessageCacheType = CACHE_NONE;

@Legoktm any progress on this? It seems like any fix is going to miss the 1.35 train if we don't figure it out soon.

My rough idea was to have VRS (or some other layer) call the rest API internally (similar to the action API), but that's more difficult than it looks and I just haven't gotten to it yet. I don't think we should treat this as a blocker for 1.35 anymore, but if/when a solution comes we should backport it.

Might be related to T118629. If you can reproduce, try setting $wgMessageCacheType = CACHE_NONE;

That didn't help. My understanding is that SQLite grabs a lock on all POST requests, so the first POST to api.php?action=visualeditoredit gets the lock and then the next internal POST to rest.php/... can't get the lock.

Ah. That's T93097 then, includes/db/MWLBFactory.php. Wouldn't be too hard to hack Parsoid requests to be 'isHttpRead' because although the /transform/ end points are POSTs we don't have side effects on the DB.

Change 622663 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Allow REST API POST handlers to opt out of mandatory sqlite locking

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

@Legoktm could you take a look at the above and see if it fixes the issue? It might still run into problems if the POST to action=visualeditoredit prevents even read access from Parsoid's REST API, but it might work, or at least suggest an avenue to a solution. I've no pride in authorship there, feel free to takeover the patch and hack it until it actually works properly...

EDIT: oh, and you'll need https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/622665 as well on the Parsoid side to make the above patch "do something".

daniel triaged this task as Medium priority.Sep 18 2020, 12:58 PM
Reedy renamed this task from Zeroconf VisualEditor/Parsoid doesn't seem to work on SQLite to Zeroconf VisualEditor/Parsoid doesn't work on SQLite .Sep 23 2020, 10:13 PM

Change 629401 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] RELEASE-NOTES-1.35: Note that Zeroconf VisualEditor/Parsoid doesn't work on SQLite

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

Change 629404 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_35] RELEASE-NOTES-1.35: Note that Zeroconf VisualEditor/Parsoid doesn't work on SQLite

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

Change 629404 merged by jenkins-bot:
[mediawiki/core@REL1_35] RELEASE-NOTES-1.35: Note that Zeroconf VisualEditor/Parsoid doesn't work on SQLite

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

Change 629401 merged by jenkins-bot:
[mediawiki/core@master] RELEASE-NOTES-1.35: Note that Zeroconf VisualEditor/Parsoid doesn't work on SQLite

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

SQLite might not be your typical production setup, but it's very handy for getting a MediaWiki instance up and running in a docker container, I'm using it to process Wiktionary data. After switching to Parsoid the performance suffered quite noticeably, related to the aggressive write locking mentioned above.

Change 661213 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/VisualEditor@master] Fix sqlite compatibility by opting out of obligatory write lock

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

Ok, the pair of patches (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/622663 and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/661213) actually fix the problem, according to @DLynch's testing. Sorry it took a while to get that right.

Change 622663 merged by jenkins-bot:
[mediawiki/core@master] Allow REST API POST handlers to opt out of mandatory sqlite locking

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

Change 661213 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Fix sqlite compatibility by opting out of obligatory write lock

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

Change 661763 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/VisualEditor@master] Rename magic header to be consistent with WMF CDN infrastructure

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

Change 661760 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] MWLBFactory: rename magic HTTP header for opting out of sqlite write lock

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

Change 661760 merged by jenkins-bot:
[mediawiki/core@master] MWLBFactory: rename magic HTTP header for opting out of sqlite write lock

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

Change 661763 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Rename magic header to be consistent with WMF CDN infrastructure

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

matmarex assigned this task to cscott.

Sounds like it's resolved!

Change 661933 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@REL1_35] Allow REST API POST handlers to opt out of mandatory sqlite locking

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

Change 661934 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@REL1_35] MWLBFactory: rename magic HTTP header for opting out of sqlite write lock

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

Change 661935 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/VisualEditor@REL1_35] Fix sqlite compatibility by opting out of obligatory write lock

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

Change 661936 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/VisualEditor@REL1_35] Rename magic header to be consistent with WMF CDN infrastructure

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

Change 661933 merged by jenkins-bot:
[mediawiki/core@REL1_35] Allow REST API POST handlers to opt out of mandatory sqlite locking

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

Change 661934 merged by jenkins-bot:
[mediawiki/core@REL1_35] MWLBFactory: rename magic HTTP header for opting out of sqlite write lock

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

Change 661935 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_35] Fix sqlite compatibility by opting out of obligatory write lock

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

Change 661936 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_35] Rename magic header to be consistent with WMF CDN infrastructure

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

Change 678030 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@REL1_35] SQLite compatibility with ZeroConf VisualEditor was fixed in 1.35.2

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

Change 678030 merged by jenkins-bot:

[mediawiki/core@REL1_35] SQLite compatibility with ZeroConf VisualEditor was fixed in 1.35.2

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