Change 508423 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] localisation: Inject 'directory' option to LCStore classes

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

Change 508422 merged by jenkins-bot:
[mediawiki/core@master] localisation: Improve documentation around wgLocalisationCacheConf

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

Change 508423 merged by jenkins-bot:
[mediawiki/core@master] localisation: Inject 'directory' option to LCStore classes

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

Change 528907 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] DefaultSettings: Document wgTmpDirectory guarantees and expectations

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

I think as the first step we can add it to DevelopmentSettings.php. For one reason is that we put experimental configs there first and then slowly we migrate to to DefaultSettings.php (I admit the reason I care is that our tests are extremely slow because of more than 50k db queries to l10n_cache in every quibble run)

Change 529057 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Set l10n cache to array in DevelopmentSettings.php

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

Change 528907 merged by jenkins-bot:
[mediawiki/core@master] DefaultSettings: Document wgTmpDirectory guarantees and expectations

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

Change 532730 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] MessageCache: Minor wgMsgCacheExpiry doc fix, and cleare constant access

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

Change 532732 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] MessageCache: Increase APC 'messages-big' expiry from 1min to 1h

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

Change 532730 merged by jenkins-bot:
[mediawiki/core@master] MessageCache: Minor wgMsgCacheExpiry doc fix, and clear constant access

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

Change 532732 merged by jenkins-bot:
[mediawiki/core@master] MessageCache: Increase APC 'messages-big' expiry from 1min to 1h

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

I was testing this and noticed one significant issue when running rebuildLocalisationCache.php:

• With files (aka CDB) memory use stays <100M for each thread when running with 4 threads.
• With array memory use grows >500M for each thread when running with 4 threads.

PHP 7.1.31. in case it is relevant.

In T218207#5444552, @Nikerabbit wrote:

I was testing this and noticed one significant issue when running rebuildLocalisationCache.php:

• With files (aka CDB) memory use stays <100M for each thread when running with 4 threads.
• With array memory use grows >500M for each thread when running with 4 threads.

PHP 7.1.31. in case it is relevant.

What are $wgMemoryLimit and PHP memory_limit set to?

Did you see it grow the whole time and 500M is where it ended, or does it get to 500M early on and then remain stable at that level? I'll be sure to test for this as well before making any changes.

Command line scripts do not have any memory limit unless specified separately.

It grew during the duration of the script, reaching 500M at the end, while the files <100M stayed relatively stable.

Change 534527 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] localisation: Release data from memory in LCStoreStaticArray::finishWrite

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

Thanks @Nikerabbit I'm able to reproduce the leak, as well as confirm that CDB wasn't affected. I found the source of the leak and fixed it in the above patch.

Change 534527 merged by jenkins-bot:
[mediawiki/core@master] localisation: Release data from memory in LCStoreStaticArray::finishWrite

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

Hey there, should this be moved to 1.35? The cut is a couple of weeks away. If it needs to go out in 1.34, is there anything I can do to help get it out of the door?

The next step here is deciding on what wgCacheDirectory would be by default. Some options and thoughts from me about that are at T218207#5160934. I'm basically just waiting for others to say whether some or all of those are a bad idea.

In T218207#5157583, @Krinkle wrote:

Interesting. In the specific case of SQLite, "cache in database" and "cache on disk" are effectively both use the disk. Some quick comparisons using Quick MediaWiki to install MediaWiki with SQLite (macOS, on-disk /private/tmp/quickmw, PHP 7.1.26 from Homebrew).

SQLite (default installation)

LocalSettings.php (generated)

$wgLocalisationCacheConf['storeServer'] = [
	'type' => 'sqlite',
	'dbname' => "{$wgDBname}_l10n_cache",
	'tablePrefix' => '',
	'variables' => [ 'synchronous' => 'NORMAL' ],
	'dbDirectory' => $wgSQLiteDataDir,
	'trxMode' => 'IMMEDIATE',
	'flags' => 0
];

Test: Deploy one language

time php maintenance/rebuildLocalisationCache.php --lang de --force
real	0m0.404s, 0m0.416s, 0m0.407s

StaticArray

LocalSettings.php (appendix)

$wgCacheDirectory = $wgSQLiteDataDir;
$wgLocalisationCacheConf['store'] = 'array';

Test: Deploy one language

real	0m0.140s, 0m0.156s, 0m0.148s

Looks like Static Array beats SQLite as well. We've shown in all previous benchmarks that the "All languages" and "Page load time" use cases always align with the "One language" use case, so I won't bother re-running those. Besides, I don't think this would inform our decision here, as I don't think we should optimise the stock MW default for SQLite against MySQL and other RDBMS'es.

In the unlikely event someone finds that sqlite3-based writing or reading outperforms opcache-backed arrays, it will still work by default, and can be optimised by setting wgLocalisationCacheConf directly.

FYI, with https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/535338 and SQLite 3.30 (compiled), I get:

# SQLite (cold table)
aaron@DESKTOP-78G5GQL:~/OSS/core$ sudo -u www-data echo "delete from l10n_cache" | sqlite3 /home/data/my_wiki_l10n_cache.sqlite
aaron@DESKTOP-78G5GQL:~/OSS/core$ sudo -u www-data php maintenance/rebuildLocalisationCache.php --lang de --force
Rebuilding de...done (205ms)

# SQLite (table already set)
aaron@DESKTOP-78G5GQL:~/OSS/core$ sudo -u www-data php maintenance/rebuildLocalisationCache.php --lang de --force
Rebuilding de...done (122ms)
1 languages rebuilt out of 1

# LCStoreStaticArray
aaron@DESKTOP-78G5GQL:~/OSS/core$ sudo -u www-data php maintenance/rebuildLocalisationCache.php --lang de --force
Rebuilding de...done (106ms)
1 languages rebuilt out of 1

It's hard to beat file_put_contents().

We're discussing this in CPT feature request meeting, and we think it should be decided before 1.35 but shouldn't gate the 1.34 release per @Jdforrester-WMF

Change 529057 merged by jenkins-bot:
[mediawiki/core@master] Set l18n cache to array in DevelopmentSettings.php

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

Updating title to reflect priority from CPT (thirty-party wikis will remain on the slower SQL-based LCStore for MW 1.34).

Moving back as we're now half-way the 1.35 cycle. This is blocked on deciding if and what to use as the default cache directory in MediaWiki core. See previous comments for arguments in favour and against various of the options I was able to gather.

CPT needs to provide input on a technical decision. We (@CCicalese_WMF and I) think this would be good for a technical planning discussion, so we're adding to Clinic Duty and tagging for @WDoranWMF .

Speaking as a disinterested person, $IP/cache seems the most sensible option to me.

Change 594218 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] language: Add test coverage for LCStoreStaticArray

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

Change 594234 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] language: Avoid LCStoreStaticArray::decode() recursion for arrays

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

Change 594218 merged by jenkins-bot:
[mediawiki/core@master] language: Add test coverage for LCStoreStaticArray

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

Change 594234 merged by jenkins-bot:
[mediawiki/core@master] language: Avoid LCStoreStaticArray::decode() recursion for arrays

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

Moving on workboard from Backlog to Next, since the Backlog column was hidden some time ago. Was T218207#5866709 ever resolved?

Not yet. This is still pending advice/direction from CPT to decide where we want to store this in MW by default, based on the options and trade-offs provided above.

OK, thanks. Moving it to tech planning for review.

Bumping again, doesn't seem like enough progress has been made yet.

Vanilla installs on shared hosting may not be able to write to disk, as far as I know. That means we can't use the disk based LC store per default, right?

At the risk of oversimplifying, it sounds like the tradeoff is that we can make MW faster by default, at the cost of being more difficult to install for some people. Further, it sounds like the people that would experience difficulties installing are exactly the people who are not well equipped to overcome those difficulties. In other words, the suggested change sounds like a showstopper for certain people.

I'd recommend that:

1. we don't make the change
2. we consider whether there are documentation improvements to make it more obvious how to do performance tuning

In T218207#6783250, @daniel wrote:

Vanilla installs on shared hosting may not be able to write to disk, as far as I know.

I don't think we support not being able to write to disk anywhere. We have uploads, file cache, git info cache, and various tmp file writers. Some of these can be disabled, and are auto-disabled by the installer if they are unwritable, but they can't all be unwritable. One of the options proposed in this task was to use the "tmp" directory instead of $IP/cache. That seems like a reasonable fallback to use. There are pros and cons as laid out in previous comments.

That means we can't use the disk based LC store per default, right?

That depends on how you interpret "default". Most file-related features are enabled in MW by default, but gracefully fail at run-time and are also warned about by the installer, and statically disabled by the installer if one proceeds (for better UX instead of runtime discovery).

However, we do recommend, allow, and at least try by default. I think we should do the same for LCStore - try by default if the cache directory is writable, like with uploads, gitinfo cache, etc. If we really think it's valid to not even have a writable tmp dir under normal circumstances, then I suppose we could fallback to the installer assinging LCStoreDB like today. (Note I'm referring to configuration, not runtime behaviour. At runtime we will of course need to tolerate things like full disk or permission problems causing temporary problems at which point we already fallback to not having a cache and that works fine.)

In T218207#6863006, @BPirkle wrote:

[…] we can make MW faster by default, at the cost of being more difficult to install for some people.

None of this should affect site admin experience. This all happens silently and automatically. It will only make it faster and significantly easier for most, and only the same or easier for others. What would become more difficult and for whom?

In T218207#6919429, @Krinkle wrote:

In T218207#6783250, @daniel wrote:

Vanilla installs on shared hosting may not be able to write to disk, as far as I know.

I don't think we support not being able to write to disk anywhere. We have uploads, file cache, git info cache, and various tmp file writers. Some of these can be disabled, and are auto-disabled by the installer if they are unwritable, but they can't all be unwritable. One of the options proposed in this task was to use the "tmp" directory instead of $IP/cache. That seems like a reasonable fallback to use. There are pros and cons as laid out in previous comments.

It's been a few MW releases since I last tried it, but "not being able to write to the disk anywhere" works fine, since none of the things you mentioned are actually enabled by default. I also think it's something we should continue to support as immutable containers become more popular.

Hey there, should this be moved to 1.37? The cut for 1.36 has happened, and 1.36.0-rc.0 will be cut in a fortnight or so, after which feature changes shouldn't be landed and back-ported.

12:32:27 <Krinkle> legoktm: if I understand correctly, your reply at https://phabricator.wikimedia.org/T218207 is not in opposition or incompatible with my intent there, right? Or did I miss something? My intent being that we detect and use by default where possible.

Indeed, I'm overall supportive of this change as it will likely provide a better/faster MW experience out of the box.

Are there reasons we haven't already?

At the time of the introduction of $wgCacheDirectory, upload was disabled by default and there was no option to enable it in the installer. $wgCacheDirectory was disabled largely by analogy, except that the security problems with $wgCacheDirectory are somewhat more severe than with $wgUploadDirectory. The main concern is that people will set it to $IP/cache or something and thereby leak private data and create XSS vulnerabilities.

/tmp is probably OK -- the historical temp directory was $IP/images/tmp which is almost as problematic as $IP/cache. I say "almost" because at least the documentation was clear about disabling PHP execution in $IP/images.

Anyway, it's 100% an installer problem. If you let the installer set $wgCacheDirectory, the installer has to verify that the proposed location is not accessible via the web. I don't think it is safe to trust the user to do that.