Page MenuHomePhabricator

Add Scap support for static-array format of LCStore
Closed, ResolvedPublic

Description

Scap should not know or care about which LCStore subclass is selected as the $wgLocalisationCacheConf['storeClass']. At the moment scap assumes LCStoreCDB is in use. This assumption is manifest in hard-coded checks for l10n_cache-en.cdb, etc. This prevents us from migrating to LCStoreStaticArray.

Event Timeline

ori raised the priority of this task from to High.
ori updated the task description. (Show Details)
ori added subscribers: ori, bd808.

The specific check that exists for l10n_cache-en.cdb is to determine if the --force flag needs to be passed to rebuildLocalisationCache.php or not. To see why this is needed you have to jump in the wayback machine and travel back to the bash implementation of mw-update-l10n and the fix for T53174: Scap broken for deploying new versions of MediaWiki due to ExtensionMessage file not being created (https://gerrit.wikimedia.org/r/#/c/113260/). Removing the check entirely doesn't seem like a good idea, but maybe we can find a way to generalize it?

It seems that there are other changes that will be needed in scap however if we are going to replace LCStoreCDB with LCStoreStaticArray. See scap.tasks._call_rebuildLocalisationCache() and scap.tasks.merge_cdb_updates() as a couple of things that are obviously related to the CDB cache layer.

In practicality I think it actually is scap's job to understand the build artifacts in use for a MediaWiki deployment and how to properly manage them. Scap is not a general purpose tool that is agnostic of usage. It is a very purpose built tool to manage MediaWiki deployments for the WMF production cluster. It might be better to reword this feature request as "Add support for LCStoreStaticArray l10n cache" and determine if we need to continue to support LCStoreCDB or can unilaterally convert the beta cluster and production usage to the new storage backend.

Krinkle subscribed.

I recall seeing at some point changeset(s) for Scap that did some work toward this end, but can't find them now.

I recall seeing at some point changeset(s) for Scap that did some work toward this end, but can't find them now.

It was (sort of) implemented in https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/scap/+/224520/ (and several follow up patches). Then it was removed in https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/scap/+/240440/.

Below are ideas from the parent task about implementing transition stage in Scap.

Deployment plan, […]

In T99740#5743027, @ori wrote:

[…] As for the migration itself, something like this could work:

  1. Add a way to override the LC store class for rebuildLocalisationCache invocations via an env var or command-line argument. E.g., in CommonSettings.php, add:
if ( PHP_SAPI === 'cli' ) {
  if ( getenv( 'MW_FORCE_LC_CLASS' ) ) {
       $wgLocalisationCacheConf['storeClass'] = getenv( 'MW_FORCE_LC_CLASS' );
  }
}
  1. Make scap's _call_rebuildLocalisationCache run rebuildLocalisationCache.php twice: once with MW_FORCE_LC_CLASS=cdb, and once with MW_FORCE_LC_CLASS=array.
  1. At this point, scap will be generating a set of .php l10n cache files in addition to the .cdb/.json files. The .php files will get synced with the .json files.

[…]

I like Ori's version better here as it allows for generating the array format ahead of any wiki using it (not even testwiki). And then doing the switch gradually separate from that. Ori's version also allows us to be 100% on the new array format and still (for a short time) generate the cdb files. Thus allowing an instance switch-back using nothing more but a config patch and SWAT (no full Scap l10n-rebuild required).

Krinkle renamed this task from scap should be LCStore-agnostic to Add Scap support for static-array format of LCStore.Dec 16 2019, 6:05 PM

Change 558239 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[operations/mediawiki-config@master] Add a bit for forcing LC caching backend in cli mode

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

Change 558243 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/tools/scap@master] Add the part for rebuilding array l10n cache

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

Change 558239 merged by jenkins-bot:
[operations/mediawiki-config@master] Add a bit for forcing LC caching backend in cli mode

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

Mentioned in SAL (#wikimedia-operations) [2019-12-18T12:21:08Z] <ladsgroup@deploy1001> Synchronized wmf-config/CommonSettings.php: SWAT: [[gerrit:558239|Add a bit for forcing LC caching backend in cli mode (T105683)]] (duration: 01m 03s)

I deployed it but I'm not sure if it works with mwscript:

ladsgroup@mwmaint1002:~$ MW_FORCE_LC_CLASS="really" mwscript eval.php --wiki=wikidatawiki
> getenv( 'MW_FORCE_LC_CLASS' );

> ^D
ladsgroup@mwmaint1002:~$ export MW_FORCE_LC_CLASS="really"
ladsgroup@mwmaint1002:~$ mwscript eval.php --wiki=wikidatawiki
> getenv( 'MW_FORCE_LC_CLASS' );

Ah right. We run mediawiki code as a separate user and a clean shell environment with its env variables puppetized (eg. MW STAGING DIR etc.). This is to ensure consistency between users and avoid e.g. poisoning umask and causing files to be made personal or without the correct group.

I assumed both ENV and CLI option would work equally easily, but given not, perhaps go for the rebuildLocalisationCache.php CLI option instead?

Change 560342 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Add option to override storeClass in rebuildLocalisationCache

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

Change 560342 merged by jenkins-bot:
[mediawiki/core@master] Add option to override storeClass in rebuildLocalisationCache

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

Change 560596 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@wmf/1.35.0-wmf.11] Add option to override storeClass in rebuildLocalisationCache

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

Change 560596 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.11] Add option to override storeClass in rebuildLocalisationCache

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

Mentioned in SAL (#wikimedia-operations) [2020-01-06T12:28:05Z] <ladsgroup@deploy1001> Synchronized php-1.35.0-wmf.11/maintenance/rebuildLocalisationCache.php: SWAT: [[gerrit:560596|Add option to override storeClass in rebuildLocalisationCache (T105683 T99740)]] (duration: 00m 55s)

Change 558243 merged by jenkins-bot:
[mediawiki/tools/scap@master] Add the part for rebuilding array l10n cache

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

Change 558243 merged by jenkins-bot:
[mediawiki/tools/scap@master] Add the part for rebuilding array l10n cache

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

Please let me know once a new version of scap gets deployed.

It's not blocked on me? Just double checking if I need to do anything.

Change 558243 merged by jenkins-bot:
[mediawiki/tools/scap@master] Add the part for rebuilding array l10n cache

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

Please let me know once a new version of scap gets deployed.

It's not blocked on me? Just double checking if I need to do anything.

Not blocked on you, nothing for you to do. This is currently blocked on releng.

This change is currently live in the beta/deployment-prep version of scap, FYI.

Next step (once we're sure we're OK with this) is to remove the CDB generation/sync and MD5ing and so on, right?