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

[FR] Add a way to disallow analysis cache discards #16804

Closed
alexeagle opened this issue Nov 19, 2022 · 4 comments
Closed

[FR] Add a way to disallow analysis cache discards #16804

alexeagle opened this issue Nov 19, 2022 · 4 comments
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request

Comments

@alexeagle
Copy link
Contributor

Description of the feature request:

Context: https://groups.google.com/g/bazel-discuss/c/hlJakHgizSo as well as discussions at BazelCon in the community day and rules authors SIG session.

I had agreement with @katre at BazelCon that it's a reasonable feature request to have a new flag that allows us to enforce that re-analysis isn't performed in a given build.

Of course, we should not fail the first time a target is analyzed, such as when a Bazel server is cold and doing the first build. I propose that the codepath which currently prints "INFO: Build option X has changed, discarding analysis cache" would simply fail the build under the new flag.

By using this new flag, Bazel users who intend to have fast, incremental rebuilds would be able to immediately discover that they've forgotten to pass the same Bazel options to every invocation of Bazel.

What underlying problem are you trying to solve with this feature?

This was one takeaway of the BazelCon community day session on slow builds due to re-analysis.

Currently it's easy to accidentally introduce a change that discards the analysis cache. Like non-determinism, this often creeps into a build silently, de-optimizing performance of CI pipelines in a way that maintainers only notice and repair after user complaints of "slow builds".

Flags like --run_under and --action_env are frequent causes as indicated below in the links.

Fixing the common pseudo-command in .bazelrc (#3054) would help with this problem but there are still cases where users pass flags on the command-line and did not mean to cause re-analysis.

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

Any other information, logs, or outputs that you want to share?

An alternative approach is to fundamentally address the expense of re-analysis, either by making it incremental, or as @aiuto proposed at BazelCon, eliminating configuration flags altogether. Those might be worth pursuing in the long term; this issue hopes to improve the situation for Bazel users immediately.

@JaredNeil indicated he would use this feature within his org.

Someone told me that @gregestren might be the reviewer of a PR to do this, so I'd like to confirm it would be accepted.

Note that there is a similar problem with Bazel server restarts with accidental changes to startup options, and also accidental external repository invalidations. I suspect it's better to address these problems separately but perhaps they should be considered in the same design effort.

@gregestren
Copy link
Contributor

Happy to consider this, and I see the attached PR.

I'm generally wary to add yet more flags to toggle how Bazel works. But I understand the impact in this case is real without obvious short-term broader solutions.

@gregestren
Copy link
Contributor

I'm sold on this case, thanks. It's also a nice sanity check tool for people who want to vet that various build invocation patterns match before being deployed.

@gregestren gregestren added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Dec 1, 2022
gregestren pushed a commit to gregestren/bazel that referenced this issue Sep 12, 2023
Adds `--allow_analysis_cache_discard` option that allows the build to exit early if the analysis cache would have been discarded.

The flag name is of course open to bikeshedding etc.

fixes bazelbuild#16804

Closes bazelbuild#16805.

PiperOrigin-RevId: 552575951
Change-Id: Ia336eb3a5b7d7e41665fd0e0adf3edc03ed50f18
gregestren pushed a commit to gregestren/bazel that referenced this issue Sep 12, 2023
Adds `--allow_analysis_cache_discard` option that allows the build to exit early if the analysis cache would have been discarded.

The flag name is of course open to bikeshedding etc.

fixes bazelbuild#16804

Closes bazelbuild#16805.

PiperOrigin-RevId: 552575951
Change-Id: Ia336eb3a5b7d7e41665fd0e0adf3edc03ed50f18
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. Thanks!

@Fil-Den
Copy link
Contributor

Fil-Den commented Sep 21, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants