Problem/Motivation

Builds regularly fail because we reach GitHub rate limits. I've tried adding a GitHub PAT but they don't seem to be inherited by issue forks so that looks like a dead end.

Steps to reproduce

Proposed resolution

Add a vendor cache similar to the lint cache.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3631331

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

longwave created an issue. See original summary.

longwave opened merge request !17468

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Priority: Normal » Critical
Status: Active » Needs review

The "Write lint caches" created a vendor artifact and uploaded it: https://git.drupalcode.org/project/drupal/-/jobs/12919390

The PHPCS on the next run downloaded it again and didn't need to touch GitHub: https://git.drupalcode.org/project/drupal/-/jobs/12919474

The PHPCS job took 60-69 seconds here if it has to do composer install; with this additional caching it took 47 seconds.

Bumping to critical as multiple CI pipelines are failing with the GitHub rate limit issue and this should solve them.

godotislate’s picture

godotislate
he/him
commented

Is it possible to cache the vendor directory with a key based on composer.lock? That way all jobs within the same project that use the same composer.lock could reuse the vendor files?

Something like what's documented here: https://docs.gitlab.com/ci/caching/examples/#compute-the-cache-key-from-..., except using composer.lock?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Not with this artifact technique, but I don't think it's much of a problem. Unless something has changed on the DA side, GitLab caching is not really any use to us as it isn't shared between runners - as we have ephemeral k8s runners each cache expires when the runner is terminated.

This technique means that pushes to main build a cache*, then all subsequent MRs will download that cache from main. We can backport it to other branches if we need to and it will also work there the same way. As composer.lock is largely stable in branches unless we are actually testing dependency updates this should be fine for the majority of cases.

(*) the cache was only built here because I used the special MR label to test it

godotislate’s picture

godotislate
he/him
commented

Sounds good. I asked because the artifact URL looks to be built using the ${CI_MERGE_REQUEST_IID} env variable, so my concern was whether other MRs or jobs would be able to access the same artifact.

Edit: Actually that seems to apply only when testing the cache warming itself.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

This gave us a 4m16 pipeline run which is one of the fastest I have ever seen. On discussing with @catch I think we can improve this further by reusing the vendor cache in test jobs...

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

This run was actually slower - 4m25 - due to test runner variance. Given that this also removes the fast-fail when phpcs or phpstan fail, which does probably save the DA some money, let's undo this.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue summary: View changes

Analysing all the jobs run in this issue, the "composer setup" step takes about 8 seconds when it has to download from GitHub and about 3 seconds now when it doesn't.

This saves about 5 seconds wall clock time from each pipeline run - because the runs were all parallel before, it doesn't save any more unfortunately. But, as there were 7 jobs each running composer install in parallel before, this saves about 35 seconds of total runner time per pipeline, which should hopefully add up to some savings for the DA.

godotislate’s picture

godotislate
he/him
commented
Status: Needs review » Reviewed & tested by the community

I see the vendor artifact being uploaded successfully in the Write lint caches job (such as https://git.drupalcode.org/project/drupal/-/jobs/12921005) 

$ if [ ! -d vendor ]; then exit 1; fi
$ tar -czf vendor.tar.gz vendor
Uploading artifacts for successful job
00:03
Uploading artifacts...
core/phpstan-tmp/resultCache.php: found 1 matching artifact files and directories 
core/.cspellcache: found 1 matching artifact files and directories 
core/.eslintcache: found 1 matching artifact files and directories 
core/.stylelintcache: found 1 matching artifact files and directories 
core/.phpcscache: found 1 matching artifact files and directories 
core/.twigcsfixercache: found 1 matching artifact files and directories 
vendor.tar.gz: found 1 matching artifact files and directories 
Uploading artifacts as "archive" to coordinator... 201 Created  correlation_id=978479176 id=12921319 status=201 token=69_6CsAD5

and downloaded successfully in the jobs like PHPCS: https://git.drupalcode.org/project/drupal/-/jobs/12921291

$ if [[ "$DAILY_TEST" != "1" ]]; then # collapsed multi-line command
Downloading https://git.drupalcode.org/api/v4/projects/59858/jobs/artifacts/refs%2Fmerge-requests%2F17468%2Fmerge/raw/vendor.tar.gz?job=Write%20lint%20caches
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 18.1M  100 18.1M    0     0  15.0M      0  0:00:01  0:00:01 --:--:-- 15.0M
$ composer validate
./composer.json is valid
$ composer install --optimize-autoloader
> Drupal\Composer\Composer::ensureComposerVersion
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Nothing to install, update or remove
Generating optimized autoload files
> Drupal\Composer\Plugin\Scaffold\Plugin::preAutoloadDump
Hardening vendor directory with .htaccess file.
89 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
drupal/drupal: This package is meant for core development,
               and not intended to be used for production sites.
               See: https://www.drupal.org/node/3082474
Cleaning installed packages.

lgtm.

  • catch committed 92b0a465 on 12.0.x 
    task: #3631331 [ci] Cache Composer vendor directory

By: longwave
By:...

  • catch committed f0818395 on main 
    task: #3631331 [ci] Cache Composer vendor directory

By: longwave
By:...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 12.0.x-dev
Status: Reviewed & tested by the community » Fixed

This looks great to me, should mean less load during normal operation but also improve things when github itself is flaky.

We might want to open a gitlab templates issue to see if they could use the core vendor directory by branch once this lands?

edit: cross-posted with @godotislate's RTBC, but that means I can go ahead and commit this. Let's get the benefit for other MRs asap.

Committed/pushed to main and cherry-picked to 12.0.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

This broke the "Write lint caches" job in performance test, which negates the whole thing: https://git.drupalcode.org/project/drupal/-/jobs/12923864#L36

Will see if I can hotfix this...

  • longwave committed dbfce851 on main 
    task: #3631331 followup [ci] Cache Composer vendor directory

By:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.