Conversation
Terraform Test Results2 tests 1 ✅ 35s ⏱️ For more details on these failures, see this check. Results for commit 40b9f76. ♻️ This comment has been updated with latest results. |
Skip Go setup, patching, and binary compilation when the cache hits. Also cache the Go vendor directory keyed on go.sum. Caches are saved immediately after build using explicit save/restore so they persist even when tests fail.
dmacvicar
force-pushed
the
optimize/cache-test-binaries
branch
from
9edbfd7 to
caa1566
Compare
Read the Go version from go.mod before the cache restore step so it is available for the key. This ensures binaries are rebuilt when the Go version changes.
alexrashed
left a comment
alexrashed
left a comment
There was a problem hiding this comment.
Nice idea to implement more aggressive caching!
I guess for me the biggest question is how often the cache is going to be invalidated / how often we will have a cache hit. If we are going to update the submodule hash every week, and only run the action once a week, there is no need to introduce these caches. However, if there are multiple runs on the same terraform test codebase, this looks very promising! 🤩
| key: test-bin-go${{ steps.submodule-info.outputs.go-version }}-${{ steps.submodule-info.outputs.sha }}-${{ hashFiles('terraform_pytest/patches/**') }}-${{ matrix.service_partition.service }} | ||
|
|
||
| - uses: actions/setup-go@v6 | ||
| if: steps.cache-binary.outputs.cache-hit != 'true' |
There was a problem hiding this comment.
question: The setup-go action takes ~10 seconds to run (at least in this run). Would it maybe make sense to avoid the complexity of determining the go version manually, and just run the go setup always?
| - name: Restore test binary cache | ||
| id: cache-binary | ||
| uses: actions/cache/restore@v4 | ||
| with: | ||
| path: test-bin/${{ matrix.service_partition.service }}.test | ||
| key: test-bin-go${{ steps.submodule-info.outputs.go-version }}-${{ steps.submodule-info.outputs.sha }}-${{ hashFiles('terraform_pytest/patches/**') }}-${{ matrix.service_partition.service }} |
There was a problem hiding this comment.
question: I am not sure, but is it really necessary to explicitly run restore and save? Usually you only use actions/cache and the action itself runs the restore directly and registers a post-step. Is this because the cache should be stored even if the workflow run fails?
| - name: Restore test binary cache | ||
| id: cache-binary | ||
| uses: actions/cache/restore@v4 | ||
| with: | ||
| path: test-bin/${{ matrix.service_partition.service }}.test | ||
| key: test-bin-go${{ steps.submodule-info.outputs.go-version }}-${{ steps.submodule-info.outputs.sha }}-${{ hashFiles('terraform_pytest/patches/**') }}-${{ matrix.service_partition.service }} |
There was a problem hiding this comment.
question: There are 3 different caches with different cache keys, each using the keys in the restore and save actions. I think that might be the weakest point here in terms of complexity. It will be very easy in the future to break the caching because the keys are manipulated or diverge. Would it maybe make sense to have one step clearly defining these keys in variables that are then being used for the steps (maybe with additional context why the keys are composed the way they are)?
2 participants
This PR explores an idea to go beyond the Go setup action caching and do caching per test binary.
While a test binary can take up to 17m to build, with the Go action cache this goes down to something between 30s to 2m depending on the test. This is not much, but if we test 100+ services, this scales quickly to 30+ minutes per run.
This approach caches the service binary, and brings it down to ~5s to restore the binary cache. The cache is done on top of the go-action layer, so it cascades on miss/hit of that cache.
The key is composed of:
I'd love to get feedback if the complexity is worth the gain.