Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Share cache entry between matching Jobs in different workflows #1017

Closed
kslesinski-godaddy opened this issue Dec 28, 2023 · 10 comments
Closed
Milestone

Comments

@kslesinski-godaddy
Copy link

Context:

We have multiple workflows:

  • ci:develop, runs on develop branch, writes multiple caches (one cache per job)
  • ci:feature, runs on feature/ branches, reads multiple (read-only) caches (one cache per job)
  • ci:firebase, run on any branch, reads a single cache

All workflows perform very similar operations (build, test). However, they do differ slightly: extra jobs/steps, i.e. uploading artifacts, comparing code coverage vs. develop, etc.

All of these workflows define matching job ids (that allowed cache entry matching):

  • ci-build
  • ci-test
  • etc

It was allowing ci:feature / ci:firebase jobs to use matching cache entries from ci:develop jobs (i.e. ci:feature/ci-test was using ci:develop/ci-test cache, ci:firebase/ci-build was using ci:develop/ci-build cache, etc).

Problem:

We've noticed that we are getting cache misses and our ci:feature workflow is now using random/latest caches from develop branch, instead of the most appropriate / "matching" entries.
This is probably related to changes introduced in #699 that started taking workflow name into consideration (as per the official docs).
It seems that the cache (or multiple caches) can now only be easily re-used within a single workflow.

Question:

We were previously able to reuse cache entries between different workflows by naming jobs appropriately.
Is a similar functionality still possible?
It seems that passing restore-keys manually is not possible. If I am not mistaken, using the undocumented GRADLE_BUILD_ACTION_CACHE_KEY_JOB env variable might be a possible workaround (using a hardcoded cache entry name), but I am not sure if this is the best solution (it's undocumented and would completely disable the default cache entry matching mechanism)?

Are there any general recommendations on how gradle cache should be handled across multiple GH workflows now?
I.e. not sharing cache between different workflows at all? Populating a single gradle cache (by running all gradle tasks in a single job) that all other workflows will fallback to?

@bigdaz
Copy link
Member

bigdaz commented Jan 2, 2024

Thanks for the report.

Yes, now that the workflow name is part of the cache key you will no longer get a match for entries running with the same Job id in different workflows. This was intentional to avoid name collisions when Jobs in different workflows are coincidentally named the same.

The standard mechanism will work well for sharing cache entries if you:

  1. Execute the same workflow in both the develop and feature/ branches
  2. OR allow cache entries to be written from the feature/ branches

But I presume there are good reasons that this won't work for you.
In that case, I think that setting the GRADLE_BUILD_ACTION_CACHE_KEY_JOB environment variable for each of those "shared" jobs will work well. As you can see here, this value will be used instead of ${workflowName}-${jobId} in the cache key (and restore keys). If you set this value to the Job ID, then cache entry matching should perform just like it did before workflow.name was added to the cache key.

@bigdaz
Copy link
Member

bigdaz commented Jan 2, 2024

This issue raises an interesting point: I've generally thought of each Workflow as something quite standalone, but it makes sense that users could be composing different workflows in different ways from common set of Job actions.

If Job.ID is something meaningful in the context of multiple workflows, then perhaps we could extend the cache restore process to something like:

  • An exact match on OS, workflow, job, matrix and Git SHA
  • The most recent entry saved for the same OS, workflow, job and matrix values
  • The most recent entry saved for the same OS, workflow and job
  • << NEW >> The most recent entry saved for the same OS and job
  • The most recent entry saved for the same OS

@bigdaz bigdaz changed the title cache entry matching between two workflows Share cache entry between matching Jobs in different workflows Jan 2, 2024
@bigdaz bigdaz added this to the 3.0.0 milestone Jan 2, 2024
@kslesinski-godaddy
Copy link
Author

kslesinski-godaddy commented Jan 3, 2024

Thank you for the feedback and a thoughtful response!

This issue raises an interesting point: I've generally thought of each Workflow as something quite standalone, but it makes sense that users could be composing different workflows in different ways from common set of Job actions.

💯 That's exactly our use case. On top of explicitly defining multiple jobs with the same id, jobs can also be easily reused across multiple workflows by means of reusable workflows, which, according to the docs, are executed in the context of the caller workflow.

If Job.ID is something meaningful in the context of multiple workflows, then perhaps we could extend the cache restore process

This would probably solve the problem of reading cache entries by jobs with matching ids (both in case of jobs from different workflows and in case of jobs defined in a reusable workflow) 👍

As a side note, however, I would like to point out the possible ramifications of adding workflow name to the cache entry hash.
Previously, reusable workflows/jobs would produce a single cache entry per job (per SHA/matrix params), no matter how many caller workflows were using it. Adding workflow name to the hash unfortunately changed that. All reusable workflows will now generate cache entries per caller workflow as well, I assume? A reusable workflow's cache (or any job's cache) is not, and can't be, encapsulated and decoupled from caller workflows anymore (even using the env variable), as workflow names now "leak" into cache key hash.
Is this an intended consequence?
It seems to me that gradle-build-action was intended to manage cache per job and the only reason for including workflow name in cache key/hash was to decrease chances of unintended key collisions for jobs with non-unique ids that should actually not share cache entries (as it is probably a much easier solution than asking developers to assign unique job ids across all workflows, where needed).

Taking the above into consideration, would it be a good idea to initialize cache key with some sensible (for the majority of use cases and developers), sufficiently unique defaults (i.e. job id + workflow), while still allowing some form of control in specific (i.e. reusable) jobs (the same way it was previously possible by manipulating job id or GRADLE_BUILD_ACTION_CACHE_KEY_JOB)?

@bigdaz bigdaz closed this as completed in 32bab5b Jan 4, 2024
@bigdaz
Copy link
Member

bigdaz commented Jan 4, 2024

Thanks for your feedback.

As a side note, however, I would like to point out the possible ramifications of adding workflow name to the cache entry hash.
Previously, reusable workflows/jobs would produce a single cache entry per job (per SHA/matrix params), no matter how many caller workflows were using it.

You're correct, and this can be problematic. The fundamental limitation is that GitHub actions cache is write-once, so once an entry is written for a key that entry cannot be modified or overwritten. Imagine the following scenario:

  • Reusable workflow (Workflow X) has a 'build' Job that executes a Gradle build: one of the workflow inputs is the Gradle task to execute.
  • Workflow A calls Workflow X with 'help' task
  • Workflow B calls Workflow X with 'assemble' task

If these 2 'build' jobs share a cache key, then whichever workflow runs first will "win" and generate the key. If Workflow A runs first, then the cache entry won't contain any of the required dependencies for the 'assemble' task. This will mean that Workflow B will need to download all of it's dependencies on every execution.

Taking the above into consideration, would it be a good idea to initialize cache key with some sensible (for the majority of use cases and developers), sufficiently unique defaults (i.e. job id + workflow),

Yes, this is exactly the intent. The idea is that the cache keys are sufficiently unique, but that the entries are de-duplicated to avoid redundancy.

while still allowing some form of control in specific (i.e. reusable) jobs (the same way it was previously possible by manipulating job id or GRADLE_BUILD_ACTION_CACHE_KEY_JOB)?

Yes, I think it makes sense to allow this sort of control. For now the only mechanism is undocumented and crude (GRADLE_BUILD_ACTION_CACHE_KEY_JOB), and we could certainly consider support that is more "first class".

@bigdaz
Copy link
Member

bigdaz commented Jan 4, 2024

I've just pushed v3.0.0-beta.3, which allows entries to be restored by Job ID without matching workflow name.

You can try this out with gradle/gradle-build-action@v3-beta. Any feedback is appreciated.

@jaloszek
Copy link

jaloszek commented Jan 9, 2024

Hi @bigdaz , I was testing v3.0.0-beta.3 and I was excepting such cache restore process:

  • An exact match on OS, workflow, job, matrix and Git SHA
  • The most recent entry saved for the same OS, workflow, job and matrix values
  • The most recent entry saved for the same OS, workflow and job
  • << NEW >> The most recent entry saved for the same OS and job
  • The most recent entry saved for the same OS

Before:

  Requesting Gradle User Home with
      key:v8-gradle|Linux|.github/workflows/_push_develop.yml-build_qa[37a6259cc0c1dae299a7866489dff0bd]-536f8240dc757c938b725204182239dd78e7c6e6
      restoreKeys:[v8-gradle|Linux|.github/workflows/_push_develop.yml-build_qa[37a6259cc0c1dae299a7866489dff0bd],v8-gradle|Linux|.github/workflows/_push_develop.yml-build_qa,v8-gradle|Linux]

After (v3 beta):

  Requesting Gradle User Home with
      key:v9-gradle|Linux|build_qa[3ec2bade30acd7b90232d0ccef358a54]-0fa77d3390159f60554abb4732f8e85ea1cdfb1f
      restoreKeys:[v9-gradle|Linux|build_qa[3ec2bade30acd7b90232d0ccef358a54],v9-gradle|Linux|build_qa,v9-gradle|Linux]

v9-gradle|Linux|build_qa - this looks like the new key that @kslesinski-godaddy was asking for and it seems to be working as expected on my end.

From my testing it seems that the workflow part (github/workflows/_push_develop.yml-) is completely missing in restoreKeys on v3 version, It would be nice to double check if some important logic for unnamed workflows was removed in #1025 (like sanitizes workflow name in cache key) on purpose or by the accident.

@bigdaz
Copy link
Member

bigdaz commented Jan 9, 2024

@jaloszek During development, I opted against adding a new cache-restore-key. The updated cache-restore process is documented here: https://github.com/gradle/gradle-build-action?tab=readme-ov-file#finding-a-matching-cache-entry

The workflow name is now encoded with the matrix values, which is why you no longer see it explicitly in the cache key.

@jaloszek
Copy link

jaloszek commented Jan 9, 2024

@bigdaz Thanks for explanation, it make sense.

I believe that it would be worth to explain somewhere that hash includes workflow name and it is not part of the cache key in explicite way, it is rather hidden in the hashcode.
Current docs show example of a cache key that contains workflow name like that:

${cache-protocol}-gradle|${runner-os}|${workflow-name}-${job-id}[${hash-of-job-matrix}]-${git-sha}

available here: https://github.com/gradle/gradle-build-action?tab=readme-ov-file#cache-keys

Would be nice to at least remove ${workflow-name} part from the example.

@bigdaz
Copy link
Member

bigdaz commented Jan 10, 2024

@jaloszek Thanks for pointing that out. I've updated the docs to reflect the new cache key format.

@kslesinski-godaddy
Copy link
Author

You can try this out with gradle/gradle-build-action@v3-beta. Any feedback is appreciated.

The new release is working as expected 👍 Great job, thank you!

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

No branches or pull requests

3 participants