Skip to content

Conversation

nadiamoe
Copy link
Member

@nadiamoe nadiamoe commented May 18, 2023

As discussed in #1182 (comment), we spend around seven minutes copying a hundred-megabyte artifact across jobs. While bigger jobs are often not ideal, in this case I think it is preferable than the artifact upload/download overhead.

With a full cache hit, this brings ci/cd time to 3 minutes. This may be due to S3 being extra smart and might be larger on the first time a PR is opened though.

image

Base automatically changed from gha-cache to main May 22, 2023 09:32
Both npm and node_modules cache will be generated by other PRs regardless of this job.
@nadiamoe nadiamoe marked this pull request as ready for review May 22, 2023 09:35
nadiamoe added 2 commits May 22, 2023 11:58
By doing this, we remove the need to upload the build output as a GH artifact as an intermediary step.
@nadiamoe
Copy link
Member Author

nadiamoe commented May 22, 2023

During development of this PR, I noticed it was not able to reuse the build cache from other PRs as I intended with #1182 . This is expected according to the docs:

Workflow runs cannot restore caches created for child branches or sibling branches. For example, a cache created for the child feature-b branch would not be accessible to a workflow run triggered on the parent main branch. Similarly, a cache created for the feature-a branch with the base main would not be accessible to its sibling feature-c branch with the base main

In order for PRs to be able to reuse each other's cache, we need to also cache the gatsby build in the staging job, which in turn would also speed up this job. I'll set this up on #1188, which will hopefully be the latest PR on these series :)

@nadiamoe
Copy link
Member Author

Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍

I did a double-take and couldn't believe the 3m36s run time 👀😀

2023-05-22-125551_1354x234_scrot

That's a radical improvement over the previous 23m! 👏

I'm slightly concerned that the aggressive caching won't bite us in the ass in some way later (inconsistent dependencies, wrong cache shared between PRs, etc.), but I currently can't see any drawbacks to this strategy, and we can address any issues with it later.

@nadiamoe
Copy link
Member Author

@imiric

I'm slightly concerned that the aggressive caching won't bite us in the ass in some way later (inconsistent dependencies, wrong cache shared between PRs, etc.), but I currently can't see any drawbacks to this strategy, and we can address any issues with it later.

I am too, at least a bit 😅

Dependencies should be alright because no cache will be restored if any of 'package.json', 'gatsby-config.js', 'gatsby-node.js' change. But we're trusting Gatsby to be smart with its cache and not leave leftover files in public/. I think this should be alright, because in a normal local develoment flow, those caches are persisted on disk essentially forever.

In any case, I purposefully left the deploy to prod job out of this caching patch series, just to be safe :)

@nadiamoe nadiamoe merged commit 61a7cb6 into main May 22, 2023
@nadiamoe nadiamoe deleted the gha-single-step branch May 22, 2023 11:07
@ppcano
Copy link
Collaborator

ppcano commented May 23, 2023

@roobre, thanks for improving the build process! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants