Granular control over cache restore and save

[kotewar]

Update

The beta version is released and available for everyone to test.
Please use the #1028 for instructions on how to try the new actions and give your feedback. 🙇🏽‍♂️

---

🚨🚨 ANNOUNCEMENT 🚨🚨

Hey everyone, 👋🏽

Over a period of time, many users have shared feedback about the need for better control of when to save/restore a cache. Currently cache action attempts a restore of a cache, and if there is no exact match, then the cache is saved during the post phase of a job. We looked at these feedback and have come up with a solution which aims to address a majority of these feedback.

Proposal

We will be releasing two new actions in the coming weeks. These will be in addition to the existing actions/cache action which will continue to exist.

• actions/cache/restore
• actions/cache/save

As their names suggest, these new actions will have the ability to just restore and save the cache respectively. This allows a workflow to have individual restore and save steps instead of relying on a single cache action which internally does both. This also opens up an opportunity to control conditions for specific steps so as to skip restore or save on need.

Inspired from: #46

Examples

Use case 1:

In case of multi-module projects, where the built artifact of one project needs to be reused in subsequent child modules, the need of rebuilding the parent module again and again with every build can be eliminated. The actions/cache or actions/cache/save action can be used to build and save the parent module artifact once, and restored multiple times while building the child modules.

Inspired from: #350, #334

Workflow for building the parent module and save it

steps:
  - uses: actions/checkout@v3
  - name: Build
    run: /build-parent-module.sh
  - uses: actions/cache/save@v3
    id: cache
    with:
      path: path/to/dependencies
      key: ${{ runner.os }}-${{ hashFiles('**/lockfiles') }}

Workflow for restoring the built artifact from cache using the same key and path

steps:
  - uses: actions/checkout@v3
  - uses: actions/cache/restore@v3
    id: cache
    with:
      path: path/to/dependencies
      key: ${{ runner.os }}-${{ hashFiles('**/lockfiles') }}
  - name: Install Dependencies
    if: steps.cache.outputs.cache-hit != 'true'
    run: /install.sh
  - name: Build
    run: /build-child-module.sh
  - name: Publish package to public
    run: /publish.sh

Use case 2:

There are instances where some flaky test cases would fail the entire workflow and users would get frustrated because the builds would run for hours and the cache couldn't get saved as the workflow failed in between. For such use-cases, users would now have the ability to use actions/cache/save action to save the cache by using if: always() condition. This way the cache will always be saved if generated, or a warning will be thrown that nothing is found on the cache path. Users can also use the if condition to only execute the actions/cache/save action depending on the output of the previous steps. This way they get more control on when to save the cache.

Inspired from: #92, #272, #849

steps:
  - uses: actions/checkout@v3
  .
  . // restore if need be
  .
  - name: Build
    run: /build.sh
  - uses: actions/cache/save@v3
    if: always() // or any other condition to invoke the save action
    with:
      path: path/to/dependencies
      key: ${{ runner.os }}-${{ hashFiles('**/lockfiles') }}

Use case 3:

Some technologies like dot-net generate the lockfiles during the build time, due to which the already evaluated ${{ hashFiles('**/lockfiles') }} hash doesn't match the actual hash. Using save action with the same key will not re-evaluate the key as hash would be calculated after the build step hence allowing the hash to be latest.

Inspired from: #989

We will also be making the restore inputted key available as output of restore action to be reused in the input of the save action. This way the user has to control to reuse the same key or get it re-evaluated based on their choice.

Let's say we have a restore step that computes key at runtime.

uses: actions/cache/restore@v3
id: restore-cache
with:
    key: cache-${{ hashFiles('**/lockfiles') }}

Case 1: Where an user would want to reuse the key as it is

uses: actions/cache/save@v3
with:
    key: steps.restore-cache.output.key

Case 2: Where the user would want to re-evaluate the key

uses: actions/cache/save@v3
with:
    key: npm-cache-${{hashfiles(package-lock.json)}}

Conclusion

There are many other use cases that can be tackled by gaining this granular control of cache restore and cache save actions.

These two new actions will be present in the same actions/cache repository and will share most of the code ensuring consistency in behavior Please note that actions/cache action will continue to have the same functionality as earlier.

These will be soon made available for testing as part of our beta release.

Please drop any feedback or suggestions that you might have regarding this change in the comments section. We are happy to listen. 😊

[DaSchTour]

Well to be able to solve the requirements from #901 there needs to be an actions/cache/check or something like that to be able to verify if a cache is available without restoring. This would make sense together with the action/cache/save action. If cache is empty do something and save the cache, if the cache is already filled skip the action.

[teohhanhui]

The problem is only when you have a separate "check" action, and then "restore" later. You'd run into the TOCTOU problem.

https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use

[Kurt-von-Laven]

That's already a theoretical issue when using a separate job for cache creation.

I'm not sure if I understand what you have in mind here, but I'm not claiming there are no ways to create systems with race conditions involving the original cache action. It is quite straightforward though to create systems with the original cache action that do not have race conditions.

From my experience though, this isn't a problem in practice.

Whether the TOCTOU problem affects a system in practice depends on the details of what one is doing, especially, for example, the scale. I agree that many people will not experience significant consequences if they ignore this particular consideration, but they ought to decide that based on their own system's characteristics and understand that it may change with growth.

[eyal0]

I struggle to understand why you might need a check in a cache system.

[Kurt-von-Laven]

The original reason that was given by @DaSchTour in #901 was:

I have some jobs that do actions if cache there is a cache hit or not or add stuff to the cache, but they actually do not care about the content of the cache. I think it would be convenient to have an option not download the content of the cache.

Additional reasons are mentioned in that thread.

[DaSchTour]

I struggle to understand why you might need a check in a cache system.

Isn't 304 Not Modified the best example of a check in a cache system. It's not something new.

[ben-spiller]

This proposal seems great. A key advantage is that it allows you to save the cache earlier in the workflow e.g. could save it after doing a big download or build but before running (possibly flaky) tests

[AdamTyler]

Awesome proposal... I look forward to testing it out.
Will end up saving use a ton a compute cycles for building flaky tests

[eyal0]

@kotewar Will the new save and restore actions continue to support key and restore-keys?

We still need them both so that we can deal sort-of workaround the situation that the server will not allow us to overwrite an existing cache entry. (This is an issue in the implementation of the cache storage server, not in the action.)

Even better would be if the save action were able to clobber and rewrite an old cache entry but this is impossible in the current server, right?

[kotewar]

Hi @eyal0, yes the new actions will continue to support key and restore-keys input. However restore-keys will only be supported by restore action as the save action only needs the key as input for saving the cache and restore-keys play no role in it.

Coming to rewriting the old cache entry, as of now, we are keeping the caches immutable. So it is not possible to rewrite the cache. We do plan to explore this possibility and understand the use cases that it will help and how it might affect the existing users/functionalities.

[eyal0]

Thanks for your quick response. So the workaround continues to be that the key is whatever key that I want to use followed by a unique number, such as ${{ steps.sanitize-key.outputs.key }}-${{ github.run_id }}-${{ github.run_attempt }} and the restore-keys is ${{ steps.sanitize-key.outputs.key }}- This is the workaround.

Being able to overwrite a cache entry would help with the LRU function of the cache.

Thanks!

[kotewar]

Makes sense, the workaround will do its job, but eat up storage space as every run/attempt would create a new cache. 🤔

[cdce8p]

Makes sense, the workaround will do its job, but eat up storage space as every run/attempt would create a new cache. 🤔

Pretty much. One project I work on regularly has 50-100GB of saved caches until those are auto-removed.

Coming to rewriting the old cache entry, as of now, we are keeping the caches immutable. So it is not possible to rewrite the cache.

I think that's completely fine. It would probably be enough if the cache action could delete all existing caches in the current scope (i.e. ref) which only matched the restore-keys. That way we would only keep the latest (just saved) cache entry which is the one which will be used anyway.

This can somewhat already be archived with the current rest endpoints, however it's not strait forward and requires a lot of additional API calls. We actually hit the API rate limit which is why we needed to removed it for now. (There are also other components which use API calls, the cache remove action wasn't solely responsible for it.)

Example workflow implementation

A workflow run on pull_request doesn't have the permission to delete caches (with the REST api). Thus create a workflow artifact first

      - name: Prepare cleanup-cache artifact
        run: |
          ref="${{ github.ref }}"
          label_keep="${{ contains(github.event.pull_request.labels.*.name, 'ci-keep-cache') }}"

          echo "ref: $ref"
          echo "label_keep: $label_keep"
          echo "$ref $label_keep" >> cleanup_cache
      - name: Upload cleanup-cache artifact
        uses: actions/upload-artifact@v3.1.1
        with:
          name: cleanup_cache
          path: cleanup_cache

The workflow to delete the cache entries needs to be triggered by a completed workflow_run

name: Delete Caches (CI)

on:
  workflow_run:
    workflows:
      - CI
    types:
      - completed

jobs:
  delete:
    name: Delete caches
    runs-on: ubuntu-22.04
    permissions:
      actions: write
    env:
      GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      ARTIFACT_NAME: cleanup_cache
      ALL_VENV_PREFIXES: venv, pip
      ALL_PYTHON_VERSIONS: 3.9, 3.10
      KEEP_VENV_DEV_CACHES: 3
    steps:
      - name: Download workflow artifact
        id: download-artifact
        run: |
          all_artifacts=$(gh api \
            -H "Accept: application/vnd.github+json" \
            /repos/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }}/artifacts)

          ids=$(echo "$all_artifacts" | jq \
            --arg key "$ARTIFACT_NAME" \
            '.artifacts[] | select(.name == $key) | [.id]')

          count=$(echo "$ids" | jq 'length')
          if [[ "$count" -eq 0 ]]; then
            echo "result=skip" >> $GITHUB_OUTPUT
            exit
          fi

          id=$(echo "$ids" | jq '.[0]')
          echo "Download artifact with id: $id ..."
          gh api \
            -H "Accept: application/vnd.github+json" \
            /repos/${{ github.repository }}/actions/artifacts/$id/zip >> "$ARTIFACT_NAME.zip"

          unzip "$ARTIFACT_NAME.zip"
      - name: Identify caches to delete
        id: identify-caches
        if: steps.download-artifact.outputs.result != 'skip'
        run: |
          sort="last_accessed_at"
          cache_count=0

          all_versions=$(echo $ALL_PYTHON_VERSIONS | tr -d ",")
          venv_prefixes=$(echo $ALL_VENV_PREFIXES | tr -d ",")

          inputs=($(cat "$ARTIFACT_NAME"))
          ref="${inputs[0]}"
          label_keep="${inputs[1]}"
          echo "ref: $ref"
          echo "label_keep: $label_keep"

          res=$(gh api --paginate \
            -H "Accept: application/vnd.github+json" \
            /repos/${{ github.repository }}/actions/caches?sort=$sort)

          res_count=$(echo "$res" | jq '.actions_caches | length')
          if [[ "$res_count" -eq 0 ]]; then exit; fi

          for prefix in $venv_prefixes; do
            index=0
            if [[ "$prefix" == "venv" || "$ref" == "refs/heads/dev" || "$label_keep" == "true" ]]; then
              index=1
            fi
            if [[ "$prefix" == "venv" && "$ref" == "refs/heads/dev" ]]; then
              index=$KEEP_VENV_DEV_CACHES
            fi

            for version in $all_versions; do
              key=".+-$version.+-$prefix-\d"
              echo "Check key regex: $key ..."
              if [[ "$index" -eq 1 ]]; then
                echo "..Keep last cache entry"
              elif [[ "$index" -eq "$KEEP_VENV_DEV_CACHES" ]]; then
                echo "..Keep last $KEEP_VENV_DEV_CACHES cache entries"
              fi

              # Select all cache keys which match the ref and key regex
              targets=$(echo "$res" | jq \
                --arg ref "$ref" --arg key "$key" --arg index $index \
                '[.actions_caches[]
                    | select(.ref == $ref)
                    | select(.key | test($key))][$index | fromjson:][]
                  | del(.created_at, .size_in_bytes, .version)')

              num_ids=$(echo "$targets" | jq -s 'length')
              if [[ "$num_ids" -eq 0 ]]; then continue; fi
              echo "$targets"

              echo "$targets" | jq '.id' >> cache_ids
              cache_count=$(( cache_count + num_ids ))
            done
          done

          echo
          echo "Found $cache_count caches to delete"
          echo "cache_count=$cache_count" >> $GITHUB_OUTPUT
      - name: Delete caches
        if: steps.identify-caches.outputs.cache_count != '0'
        run: |
          del_count=0

          num_ids=$(cat cache_ids | jq -s 'length')
          echo "Found $num_ids caches to delete"
          if [[ "$num_ids" -eq 0 ]]; then exit; fi

          for id in $(cat cache_ids); do
            echo "Delete cache with id: $id ..."
            gh api \
              --method DELETE \
              -H "Accept: application/vnd.github+json" \
              /repos/${{ github.repository }}/actions/caches/$id
            (( del_count +=  1 ))
          done

          echo "Deleted $del_count caches"

[MartijnHols]

Glad to see this finally being adopted in the main project! This functionality is what I have maintained in forks for the past two years, and why I made and shared https://github.com/MartijnHols/actions-cache. This has had some iterations to mature, and it would be great to get parity so I can deprecate that. Here are a few (in my opinion essential) suggestions:

A "check" action to check if the a key already exists in the cache. This allows things such as skipping building the app entirely if it already exists in the cache. An old build rarely speeds up building a new build and can be large so you don't want to spend precious time downloading it. This is especially useful in monorepos, where only a few apps need to be build each run.

From my Cache build output and skipping build recipe:

  build:
    needs: [install]
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2

    # Using martijnhols/actions-cache/check we check if a cache entry exists without downloading it
    - name: Check if "build" is already cached
      uses: martijnhols/actions-cache/check@v3
      id: cache
      with:
        path: build
        key: ${{ runner.os }}-node_modules-${{ hashFiles('yarn.lock', 'patches', 'src', '.babelrc') }}

    - name: Restore "node_modules" from cache
      # Only execute if the build isn't already in cache
      if: steps.cache.outputs.cache-hit != 'true'
      uses: martijnhols/actions-cache/restore@v3
      with:
        path: node_modules
        key: ${{ runner.os }}-node_modules-${{ hashFiles('yarn.lock', 'patches') }}
        required: true

    - name: Build app
      # Only execute if the build isn't already in cache
      if: steps.cache.outputs.cache-hit != 'true'
      run: yarn build

    # Notice that we do not use a "restore" in this job: the build in our imaginary project can't reuse its own build files so restoring that before building would be a waste of time.
    - name: Save "build" to cache
      # Only execute if the build isn't already in cache
      if: steps.cache.outputs.cache-hit != 'true'
      uses: martijnhols/actions-cache/save@v3
      with:
        path: build
        key: ${{ runner.os }}-node_modules-${{ hashFiles('yarn.lock', 'patches', 'src', '.babelrc') }}

In order to be able to implement this, you'll need these changes in the actions/toolkit repository: actions/toolkit#1207

A required prop in the restore action that fails the step if the cache doesn't exist (based on https://github.com/MartijnHols/actions-cache#martijnholsactions-cacherestore). With being able to use the restore action to get caches across jobs to access the cache made in another job, you may want to move the dependency installing to a separate job. Then when you get to building, the restore step must succeed or the build will fail. By default the cache action ignores cache restoration failures. When required is set to true on restore, it should instead fail the entire job.

On my fork, I made a few common recipes to showcase possible use-cases. It would be great if all those would be supported so I can deprecate the fork. Looking forward to it!

ps. in your examples your use case 3 seems like it wouldn't work to me. Shouldn't this be key: ${{steps.restore-cache.output.key}}?

[eyal0]

I think that a more natural alternative is to have the restore step always fail when restore fails and let the users use continue-on-error if the user wants to ignore failure.

This seems more similar to existing actions.

[henrygab]

have the restore step always fail when restore fails

I agree that restore should fail when restore fails, but only if there is a corresponding "Check" action added.

MartijnHols' depot provides great functionality, and I've pointed folks to it due to the official depot failing to have these needed features (separated save vs. restore, check actions).

[henrygab]

Feedback on current beta branch:

Can you help me understand the reason why the cache-restore-key would not be unconditionally set when the restore process is successful?

If there are no compelling use cases that require it to be conditional, please consider exposing this value unconditionally.

Detailed justification (click to expand)

cache-hit boolean is already exposed

This clearly and unambiguously provides the supported means to detect if the primary key was the one restored.

Clients encode information into the prefix.

It's well-known that some clients of this action encode information into the prefix.

As the simplest example, consider a client that prefixes their key with a short id (Foo) + the ISO8601 Date. Thus, a cache created on December 24, 2022 would have a prefix starting with Foo-YYYY-MM-DD-.

Conditionally setting the value offloads logic to clients

Using the above simple example, consider the client which restores using the key (prefix) Foo-. That client then wants to unconditionally extract the date from the restored key.

With only conditional cache-restore-key existing, the client has to add extra if/else logic, and then either use cache-restore-key or primary-key. If the value was unconditionally present, then the client can just use the cache-restore-key value directly.

Plain meaning of cache-restore-key

The variable name cache-restore-key does not imply it only exists conditionally. Put another way, the name of the variable does not require that it exist conditionally, and allows the variable to exist unconditionally (at least on success).

---

Summary

If there are no compelling use cases to make this value exist only conditionally, please change it to exist for all successful runs of the restore action.

Snippet from the restore YAML (click to expand)

  cache-hit:
    description: 'A boolean value to indicate an exact match was found for the primary key'
  cache-primary-key:
    description: 'A resolved cache key for which cache match was attempted'
  cache-restore-key:
    description: 'Restore key which was used to
restore the cache. It will not be set in case there
was an exact match with primary key itself'

---

[henrygab]

UPDATE: Although not replied to directly here, it seems this feedback was integrated with c8d01fa.

Thank you, @kotewar!

[kotewar]

Sorry I couldn't reply to all the comments in time. We will try to fulfil as many use cases possible in an iterative manner. :)

[kotewar]

Hey everyone, 👋🏽

The beta version is released and available for everyone to test.
Please use the discussion for instructions on how to try the new actions and give your feedback. 🙇🏽‍♂️

[kotewar]

Hey everyone 👋🏽

Two new actions actions/cache/restore and actions/cache/save are now available with tag v3 to everyone for use. These can now be used to achieve granular control on the restore and save steps in the cache action.

Do try them and let us know your feedback. We hope these new actions will take care of your use cases. 🙇🏽