all: cache go module downloads and other build and test artifacts

[leighmcculloch]

PR Checklist

PR Structure

• This PR has reasonably narrow scope (if not, break it down into smaller PRs).
• This PR avoids mixing refactoring changes with feature changes (split into two PRs
  otherwise).
• This PR's title starts with name of package that is most changed in the PR, ex.
  services/friendbot, or all or doc if the changes are broad or impact many
  packages.

Thoroughness

• This PR adds tests for the most critical parts of the new functionality or fixes.
• I've updated any docs (developer docs, .md
  files, etc... affected by this change). Take a look in the docs folder for a given service,
  like this one.

Release planning

• I've updated the relevant CHANGELOG (here for Horizon) if
  needed with deprecations, added features, breaking changes, and DB schema changes.
• I've decided if this PR requires a new major/minor version according to
  semver, or if it's mainly a patch change. The PR is targeted at the next
  release branch if it's not a patch change.

What

Cache Go module downloads and other build and test results so that for a single branch subsequent commits after the first commit use cached data.

The caching applies only to branches that aren't master.

On a branches first run it may use a cache from a different branch.

Why

To speed up GitHub Actions test runs. They appear to be somewhat slower than CircleCI test runs, and @bartekn pointed out this might be because in CircleCI test runs we cache the Go module downloads.

Caching module downloads is already what we do in our CircleCI builds, so adding that here is not controversial.

Caching test results is new though. The go toolchain has great support for test result caching. This is noticeable when running tests locally: Go will only run tests for packages that have changed, or some external input to the test has changed, such as an environment variable or a file the test reads.

Known limitations

N/A

[2opremio]

@leighmcculloch are you still pursuing this or aren't the preformance improvements good enough?

[leighmcculloch]

@leighmcculloch are you still pursuing this or aren't the preformance improvements good enough?

Yup, I am still working on this. There are concerns that @Shaptic raised offline about some tests we have some integration tests in Horizon that caching wouldn't be ideal. I plan to look into that before merging so I've marked the PR as draft again.

[leighmcculloch]

Reopening this PR because @2opremio DM'd me saying he thinks there would be value in adding this now. @Shaptic I don't think the concerns around integration tests will be impacted by this because integration tests do not run in this build.

[leighmcculloch]

Actually, I need to fix the mdate of files (fixing it will cause tests to be cached when fixtures change, which is not good!)

Can you elaborate on this? I think having a fixed mtime is okay. The contents of the file changing will cause the cache key to be different if the file contents change because we use GitHub's hashFiles in the cache key.

Unfortunately using git-restore-mtimes binds the useful of this action to Ubuntu only, and creates a weird dependency on depth=0.

[2opremio]

I am not sure I understand your comment. The cache key generated will be different, but we don't use the file hashes to restore the key:

      key: ${{ env.PREFIX }}-go-build-${{ github.ref }}-${{ hashFiles('**', '!.git') }}
      restore-keys: |
        ${{ env.PREFIX }}-go-build-${{ github.ref }}
        ${{ env.PREFIX }}-go-build

Thus, you can restore the build cache of another run with different fixture contents. If both files have the same modification time (and they will do if fix it identically for all runs), go will cache the test.

[leighmcculloch]

Ah yes, good point.

[leighmcculloch]

So what youre saying if the mtimes are the same, but file contents are different, Go will cache the test that reads the file? I thought it always used the file contents.

[2opremio]

No, I don't think it does (otherwise I guess there wouldn't be a need to use modification times). See https://cs.opensource.google/go/go/+/refs/tags/go1.17.7:src/cmd/go/internal/test/test.go;l=1583-1586;drc=refs%2Ftags%2Fgo1.17.7;bpv=0;bpt=1

[2opremio]

I am not sure I understand your comment. The cache key generated will be different, but we don't use the file hashes to restore the key:

BTW, is there a good reason to hash the files instead of using the commit hash?

[2opremio]

it turns out that git-restore-mtime doesn't change the mtime of some directories:

> "open /home/runner/work/go/go bb9598098875243c0dbd385a488fb5414ddfcd49f22602f81cf53ab3bfcd841c\n"
> "open /home/runner/work/go/go/services/horizon/internal 24e92446952b505a6dd3917d834b58f24e66d3f39e1bca77e5489ac11bbe3026\n"
> "stat /home/runner/work/go/go/.git 4a45c85e5426f757d802f36ed9b24761a2ee8e098f0bebbf5841636a22f1d660\n"
> "stat /home/runner/work/go/go/docs c21656fb0d652a1ed825aa3d8f6ff51fe7e4d96913887ba9124046fc5fc17743\n"
> "stat /home/runner/work/go/go/handlers 6cbcb56c1d83bb49c6127b72fc392957a73ce3e9287a87c25cece275c4740606\n"
> "stat /home/runner/work/go/go/services/horizon/internal/assets e8325ba2d8b8012ea7d9c76243cf502da96f1da1c152066aa2a91e9c39434de2\n"
> "stat /home/runner/work/go/go/services/horizon/internal/codes e8325ba2d8b8012ea7d9c76243cf502da96f1da1c152066aa2a91e9c39434de2\n"
> "stat /home/runner/work/go/go/services/horizon/internal/db2 50af35c929f6129e1d4633fc2ef9701e3c8f8885c855717cab98bcc9d3ea358f\n"
> "stat /home/runner/work/go/go/services/horizon/internal/errors 7a0c973de020bc0828080e6d689e800c6c1e27b12964d5b8d680d572b3babff9\n"
> "stat /home/runner/work/go/go/services/horizon/internal/reap eb634dba2c0cfc9db06be86a937d519eaf2194187a57fbaaba1709de76c1c0ab\n"
> "stat /home/runner/work/go/go/services/horizon/internal/render eb634dba2c0cfc9db06be86a937d519eaf2194187a57fbaaba1709de76c1c0ab\n"
> "stat /home/runner/work/go/go/services/horizon/internal/tls 038519f3ba2f0d1043a681d29b2a496f2ff508961dc77187e757b94150112b54\n"
> "stat /home/runner/work/go/go/services/horizon/internal/utf8 038519f3ba2f0d1043a681d29b2a496f2ff508961dc77187e757b94150112b54\n"
> "stat /home/runner/work/go/go/support 00b9d9e1980020205b34ef036b67bcdb5d92275aa6c6c8507add40535e7f2429\n"
> "stat /home/runner/work/go/go/toid 00b9d9e1980020205b34ef036b67bcdb5d92275aa6c6c8507add40535e7f2429\n"
> "stat /home/runner/work/go/go/tools 5c07b44606decb0197ce6806bb42bbafd7cf0847c494a9e6f8d5f0469825e116\n"

[leighmcculloch]

Ah directories are not stored in git, so that makes sense that they wouldn't have an mtime stored in git to restore.

[2opremio]

Ah directories are not stored in git, so that makes sense that they wouldn't have an mtime stored in git to restore.

But git-restore-mtime seems to be able to restore the mtime of directories (inferred from the mtime of files), so that seems to be a bug.

[2opremio]

Alright! This should be ready to go!

[leighmcculloch]

Awesome. Thanks for making this better and adding caching for integration tests.

I'm going to run some tests, then I'll merge this after I'm finished.

[leighmcculloch]

@bartekn You already approved this, but I accidentally merged this before you re-reviewed the changes. Please feel free to leave any feedback.