Castor: mediawiki-core-qunit-jessie node_modules cache ineffective
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Krinkle
	Mar 4 2017, 1:53 AM

Description

"Castor" caches the node_modules directory after each gate-and-submit pipeline job. And it loads it at the beginning of all pipeline's jobs.

However, the job always runs:

00:00:33.100 + rm -rf node_modules
00:00:33.101 + npm install

(From https://github.com/wikimedia/integration-config/blob/561a760837/jjb/macro.yaml#L51 and https://github.com/wikimedia/integration-config/blob/561a760837/jjb/macro.yaml#L151)

As such, the cache is ignored. The original reason for this probably doesn't apply anymore (it's mostly because the directory was retained between runs because the workspace wasn't cleaned up, afaik we do this everywhere now and/or run on Nodepool where each instances is destroyed afterward anyway).

However, even if we make sure that 4e9a7314d2b5c21899f2c329f7e5748c8b55abef is no longer a problem (node_modules preserved between npm-setup-oid jobs that point node_modules to a deploy.git-style repo), there may be other issues we need to think about. For example:

We don't clear the cache periodically (it'd grow indefinitely as unused dependencies will linger).
- May cause unexpected side-effects in case of npm modules that use autodiscovery to change their behaviour - it would find a module that is no longer there on fresh install.
- May cause code to pass even if a package is wrongly removed from package.json.
We don't clear the cache after upgrading npm in CI. In theory the cache consumer in npm is meant to be backward-compatible, but I don't know for how many releases back that support goes or how well tested this is by upstream.
Zuul will abort Jenkins jobs when their result is no longer relevant (e.g. patch-set is updated, or dependant commit failed). This may abort an npm-install process prematurely without it being able to leave behind a clean state.
- Shouldn't be an issue since we only run castor-save after a completed and successful build.

Next steps:

Verify 4e9a7314d2b5c is no longer a problem.
Draft a change that changes rm -rf node_modules; npm install to npm prune && npm install (should address point 1).

Details

	Subject	Repo	Branch	Lines +/-
	(WIP) Cache node_modules between runs	integration/config	master	+40 -6
	castor: cache binaries on a per job basis	integration/config	master	+42 -17

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Krinkle	T159591 Castor: mediawiki-core-qunit-jessie node_modules cache ineffective
		Resolved		Krinkle	T152386 CI tests on wikimedia/portals repo: cache node_modules to save time

Event Timeline

Krinkle created this task.Mar 4 2017, 1:53 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 4 2017, 1:53 AM

On IRC, @hashar reminded me that we aren't using castor to cache the $WORKSPACE/node_modules directory. Instead, we use castor to save the NPM cache directory at $HOME/.npm. This means none of the potential corruption risks apply. It also means that contrary to my assumption, we are in fact benefitting from the cache.

At runtime, npm just validates the cache and copies the appropiate packages and versions to the build directory. Which works fine and is very safe.

That came up when looking at oojs/ui npm job ( T155483 ). It takes roughly 12 minutes, 5:30 minutes being the npm install step. The slowness reason is that although npm benefit from ~/.npm/ cache, it still has to compile native modules every single time.

For ruby / bundler, we cache the vendor directory which is the equivalent of node_modules. Ie ruby jobs have the compiled modules and thus the install step ends up lightning fast when dependencies have not changed.

A note: npm/node-gyp do not support caching the compiled module:

npm: cache build directories https://github.com/npm/npm/issues/3388#issuecomment-215585680
Native add-on compile build & cache service https://github.com/nodejs/build/issues/151

Interesting one is node-pre-gyp which list a few more.

So most probably we could namespace the cache by using the package version and node ABI version among others. Example:

$ node -p 'process.versions.modules;'
51
$

I am willing to try caching node_modules, as a trial on the few repositories that have long install time (eg: oojs/ui). Ideally with a way for developers to clear the cache on demand.

• Prtksxna subscribed.Mar 29 2017, 9:10 AM

In T159591#3139591, @hashar wrote:

I am willing to try caching node_modules, as a trial on the few repositories that have long install time (eg: oojs/ui). Ideally with a way for developers to clear the cache on demand.

Maybe whenever a recheck is issued from gerrit we should rm -rf node_modules and get rid of our cache? Is that sensible and possible?

From the description:

• Draft a change that changes rm -rf node_modules; npm install to npm prune && npm install (should address point 1).

Would it be better to run npm prune && npm update, that would install missing dependencies and update the existing packages to the extent the semver range is satisfied.

In T159591#3141254, @Prtksxna wrote:

From the description:

• Draft a change that changes rm -rf node_modules; npm install to npm prune && npm install (should address point 1).

Would it be better to run npm prune && npm update, that would install missing dependencies and update the existing packages to the extent the semver range is satisfied.

Yes, we should. When caching at the server level, this isn't needed as NPM will do that no matter what. But when (effectively) preserving part of the workspace, npm install will do minimal effort since developers often run this every time before npm test or even whenever changing one of the source files on disk when working. It shouldn't pull in the latest expansion of sem ver every time (which requires a brief network roundtrip, too). However in CI we should definitely switch from npm install to npm update if we cache node_modules. Otherwise we might consider a commit as passing when in fact a fresh install would fail.

@hashar I am curious to know if such a change could be implemented on per-repository basis.

On my usual working directory of OOUI, on a MacBook Air with a 4Mbps connection:

Command	Time
`rm -rf node_modules`	20.529
`npm install`	2:43.07
`npm update`	12.305

The first time I ran npm update yesterday it took quite some time. I forgot to time it but it was surely upwards of a minute. I suppose if you run it regularly, as we will, the amount of time it takes will reduce. npm prune took only a second because I had just run all these commands.

I found a good candidate: wikimedia/portals @Jdrewniak filled T152386 to get node_modules cached. So I guess we can try to switch it to npm prune && npm update.

hashar mentioned this in T152386: CI tests on wikimedia/portals repo: cache node_modules to save time.Apr 3 2017, 12:49 PM

Change 346152 had a related patch set uploaded (by Hashar):
[integration/config@master] Cache node_modules

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

gerritbot added a project: Patch-For-Review.Apr 3 2017, 2:01 PM

Change 395610 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] (WIP) Cache node_modules between runs

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

Change 346152 abandoned by Hashar:
castor: cache binaries on a per job basis

Reason:
Castor was badly conceived requiring to mention each of the paths we want to save.

On Docker containers, the caches are all pointing at /cache thanks to XDG_CACHE_CONFIG or at least can be easily made to point there.

So the idea is to:

npm install
cp node_modules /cache/node_modules
npm test

Then if tests pass, castor would save the node_modules.

On the next build, the node_modules is restored and "npm prune && npm install" bring it up to date with package.json changes.

A basic implementation is https://gerrit.wikimedia.org/r/#/c/395610/

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

I am migrating the npm jobs to Docker containers. https://gerrit.wikimedia.org/r/#/c/395610/ would potentially save and restore node_modules and bring it up to date via a 'npm prune && npm install'.

hashar added projects: Continuous-Integration-Infrastructure (shipyard), Release-Engineering-Team (Kanban).Dec 7 2017, 9:30 PM

hashar moved this task from Backlog to Ready on the Continuous-Integration-Infrastructure (shipyard) board.Dec 7 2017, 9:50 PM

debt changed the status of subtask T152386: CI tests on wikimedia/portals repo: cache node_modules to save time from Open to Stalled.Dec 12 2017, 4:20 PM

hashar mentioned this in T211705: Quibble should support restoring npm cache from Castor.Dec 11 2018, 4:51 PM

hashar removed a project: Continuous-Integration-Infrastructure (shipyard).Dec 31 2018, 3:09 PM

Castor strategy for npm was changed from caching node_modules in workspace, to instead caching the .npm cache directory (normally in HOME).

This working as expected. Aside from being more stable, it is also good for another reason, which is that soon we'll use npm ci in more repos (based on the lock file) which always clears node_modules as part of the install process, so that wouldn't have worked anyway.

Krinkle closed subtask T152386: CI tests on wikimedia/portals repo: cache node_modules to save time as Resolved.Jan 22 2019, 12:56 AM

Change 395610 abandoned by Hashar:
(WIP) Cache node_modules between runs

Reason:
I have abandoned the idea indeed and the more recent npm is way faster at least.

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

hashar added a project: Castor.Aug 5 2020, 5:30 PM