Disable bwc distribution caching in master branch #46686
Conversation
Signed-off-by: Mark Vieira <[email protected]>
|
Pinging @elastic/es-core-infra |
Signed-off-by: Mark Vieira <[email protected]>
|
@elasticmachine test this please |
| outputs.cacheIf { true } | ||
| outputs.cacheIf { | ||
| // Don't bother caching in 'master' since the BWC branches move too quickly to make this cost worthwhile | ||
| project.ext.isCi && System.getenv('GIT_BRANCH')?.endsWith("master") == false |
There was a problem hiding this comment.
I don't love the fact that we have a CI specific conditional here, but don't have any alternatives to suggest.
Maybe some generic way to turn of caching for arbitrary tasks and configure that from CI ?
Also this won't work correctly for PRs that target master, guess that's not as big of an issue because those don't write to the cache ?
It would be so great if we could come up with a solution that does not involve special casing master.
There was a problem hiding this comment.
Also this won't work correctly for PRs that target master, guess that's not as big of an issue because those don't write to the cache ?
Correct, it doesn't really matter. They'll still make cache requests that will miss but that's pretty minimal.
Special logic for "if I'm in CI do this" is really commonplace and I'm honestly surprised we don't have instances of this already. To me, I'd much rather have this kind of thing in the build than in CI config (that is complicated enough) and it also allows us to do this branch-specific (CI config is the same for all branches). To me the more we can push logic out of JJB and into the build we should strive to do that. It's easier to reason about, cuts down on JJB and as mentioned, allows us to do branch-specific logic which we're limited in until we move to JJBB.
My preference was to somehow base this on version information the build had but I don't think that's possible. The 7x branch has no idea there is another newer major version that exists ahead of it.
There was a problem hiding this comment.
It's not so much that we do something for CI specifically that bother me, and I hear you very well on the JJb complexity, it's more how hard to find it is, and how hard it will be to reason about if we keep adding these.
It's fine if we want to configure it in the build but it would be worth to do in a way that produces an overview of what the differences are.
For example including a build script conditionally on cI and having all the configuration there with wrapped in project(":distribution:bwc") {}
There was a problem hiding this comment.
Yeah, there are pros/cons to that approach as well. As if I'm looking at a particular plugin or build script, I also have to consider if there's any CI-specific config in this separate script. This approach would be predicated on use assuming there's CI-specific config at play w/ regards to any particular issue.
Personally, if I'm tracking down a build config issue I'm naturally going to go to the source of that logic. I think I'd rather have any conditionals regarding CI or otherwise to live there. Again, I've dealt with may projects with CI-specific conditionally logic in this way and it's generally not been a problem or source of confusion.
To help out here specifically I've added added a description to this cacheIf condition (as we really should do in all cases) which will make this specific condition very obvious in build scans.
I'm not sure if I want one more place where I have to look in order to assemble the whole picture of build logic for CI. I already have to consider the build scripts/buildSrc itself, plus the .ci init scripts, plus whatever lives in the Jenkins job definition. Not sure having another location to look for stuff that might affect the behavior of the build will improve the situation.
|
@elasticmachine update branch |
Signed-off-by: Mark Vieira <[email protected]>
Signed-off-by: Mark Vieira <[email protected]>
This commit disables caching of BWC snapshot distributions in the "trunk" (aka master) branch. Since the previous major release branches move quickly we rarely get cache hits for these tasks, and the artifacts themselves are very large. This means the overhead here is high and savings basically zero. We conditionally disable task output caching in this scenario in CI to avoid excessive build cache overhead as well as causing too much turn in the cache itself which would lead to lots of cache entry evictions.
Looking at cache reuse in our
masterBWC builds I've noticed that we essentially never get a hit for BWC distributions. This isn't a logic issue since we do get decent reusing for7.xbuilds. The reason here is that the6.8branch is much less active than the7.*branches so it's much more likely to have multiple builds run where the6.8HEAD has not changed.The fallout from this is we are paying several costs here.
There is still potential for the build cache to benefit building these distributions though. It's sensible that the stuff we are building here has been built before by the original CI builds for that branch. So one thing we can do is enable the build cache for the BWC builds themselves with the intent they'll get cache hits there as the intake builds for that commit have almost certainly already run.