Problem/Motivation

Core has three overall stages in gitlab pipelines: Build, Lint and Test.

The build stage consists of two jobs - composer and yarn, which exist only to build caches/artifiacts for later stages.

However, composer install and yarn install very fast, maybe 10 seconds or so on gitlab, while waiting for a runner, downloading an image and running a job in total is at least 45s-1m.

If we flatten the build and yarn jobs, and have the lint jobs do their own yarn and composer installs, then we only add 5-10 seconds to each lint job, but we save a full minute by running the build and lint jobs in parallel.

Because the lint jobs will now be doing composer install and yarn install, we can then go a step further and use those to build the caches for the test pipeline/stage, removing the composer and yarn jobs altogether.

This means we lose two jobs from the pipeline (couple of minutes of compute time), while doing an extra 2-3 composer/yarn installs (probably 30-60s of compute time). It should save on both wall time and compute time overall.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3463479-nr-bot.txt90 bytesneeds-review-queue-bot

Issue fork drupal-3463479

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

catch created an issue. See original summary.

catch opened merge request !8903

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs work
mstrelan’s picture

mstrelan commented
Title: Merge the build and link stages in core MR pipelines » Merge the build and lint stages in core MR pipelines
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
Parent issue: » #3462762: [meta] Core test suite performance
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Yeah this came from how we test on circle ci where spin up is faster, agree on current gitlab setup it's hurting us in overall resection time

catch’s picture

catch
he/him
Primary language English
commented

Only realised this after working on #3462763: Use artifacts to share the phpstan result and cspell caches from core to MRs and seeing just how much time we were spending on spinning up vs doing things. With the combination of the two issues, the newly combined lint stage finishes in around a minute. Once we add cspell/phpcs/eslint cache support, it should be consistently under a minute hopefully.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3463479-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

catch’s picture

catch
he/him
Primary language English
commented

Just a straightforward rebase.

However I noticed since we added when: manual to the performance job, the job is blocked - needs to allow fail too it looks like.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
catch’s picture

catch
he/him
Primary language English
commented

Latest pipeline on the MR finished in 6 minutes and 52 seconds: https://git.drupalcode.org/project/drupal/-/pipelines/233855

catch’s picture

catch
he/him
Primary language English
commented

This will also help the performance test run finish quicker. At the moment we're building composer and yarn to use the cache in performance tests, but it's much quicker to just run composer install in the actual test job before running the tests. Should save ~90s on those jobs.

catch’s picture

catch
he/him
Primary language English
commented
Priority: Normal » Major

Since this takes a full minute off every core MR pipeline, bumping to major.

Combining this with some other changes, was able to get the minimum time for a full MR pipeline run down to 5m37s https://git.drupalcode.org/project/drupal/-/pipelines/234735

spokje’s picture

spokje
Primary language Dutch
commented

The nightwatch job failed, not sure if fluke or bad, and can't rerun since MR was created by a core committer.

catch’s picture

catch
he/him
Primary language English
commented

Shouldn't affect the nightwatch test at all but re-ran it just in case.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Thanks, tests are now green, as expected.
Code changes make sense.

RTBC for me.

  • nod_ committed 3c4f80ad on 11.0.x 
    Issue #3463479 by catch, Spokje, larowlan: Merge the build and lint...

  • nod_ committed 81d8971a on 11.x 
    Issue #3463479 by catch, Spokje, larowlan: Merge the build and lint...

nod_ closed merge request !8903

nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 3c4f80a and pushed to 11.0.x. Thanks!
Committed 3c4f80a and pushed to 11.0.x. Thanks!

Doesn't cherry pick to 10.4.x, need a new MR for it if we want to backport that

catch’s picture

catch
he/him
Primary language English
commented

OK the commit conflicts are because we haven't done #3428614: Resync .gitlab-ci.yml and .gitignore following Yarn 4 in 11.x yet. Put up an MR there. Once that's in, this should be a clean(er) cherry-pick.

catch’s picture

catch
he/him
Primary language English
commented

Double checked the performance pipeline on 11.x and it's down to 8m30 now with the combination of this and #3463351: Consolidate Umami performance tests, was previously closer to 13 minutes.

https://git.drupalcode.org/project/drupal/-/pipelines/235402

nod_’s picture

nod_
Primary language French
Location Lille
commented

Still an issue with the cherry pick, but not too bad

spokje’s picture

spokje
Primary language Dutch
commented
Version: 10.3.x-dev » 10.4.x-dev
Assigned: Unassigned » spokje

I think 10.4.x-dev is the correct version for this now.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned

And as was said in #21, lets postpone this on #3428614: Resync .gitlab-ci.yml and .gitignore following Yarn 4 in 11.x to make our lives easier.

nod_ opened merge request !8960

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Patch (to be ported) » Needs review
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Backport looks good.

Opened an issue for gitlab_templates since we might see a similar improvement there #3464400: Consider merging the build and validate stages.

nod_’s picture

nod_
Primary language French
Location Lille
commented

adding back the -s for yarn 1 that got squeezed from https://git.drupalcode.org/project/drupal/-/commit/19c608d552a3bbedc554a...

  • nod_ committed d170a1e0 on 10.3.x 
    Issue #3463479 by catch, nod_, Spokje, larowlan: Merge the build and...

  • nod_ committed 963de909 on 10.4.x 
    Issue #3463479 by catch, nod_, Spokje, larowlan: Merge the build and...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 10.4.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 963de90958 to 10.4.x and d170a1e09d to 10.3.x. Thanks!

nod_ closed merge request !8960

Status: Fixed » Closed (fixed)

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

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3464400: Consider merging the build and validate stages

Thanks for #3464400: Consider merging the build and validate stages! 🙏