-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Make --experimental_remote_cache_async
not experimental and on by default
#21578
Comments
I'd like this to happen at some point before the Bazel 8 release, but I'm not treating it as a priority at the moment. |
(Moving it back to untriaged so we explicitly discuss it at the next team meeting.) |
related: #19273 |
I am aware of some open issues for this flag, we need to address them before make it default. Like @tjgq said, it's not a priority at the moment, but we should make this happen before Bazel 8. |
A known issue is that async uploading requires spawn outputs to not be moved/deleted until they've been uploaded. One situation where this isn't upheld is #22501. Test attempts are another (each attempt produces the test outputs at their expected paths, but then Bazel moves them into a per-attempt path before running the next attempt). |
As @fmeum found out in #23307, Java reduced classpath actions are another situation where files can disappear after spawn execution (causing problems for async uploading). My current plan for 8.0.0 is to default to async uploading except for actions that move/delete their outputs. In the longer term I'd like to find a more definitive solution that lets us remove the sync code path entirely. |
When an action may modify a spawn's outputs after execution, the upload of outputs to the cache and reuse for deduplicated actions need to happen synchronously directly after spawn execution to avoid a race. This commit implements this for cache uploads by marking all actions with this property and simply disabling async upload for all spawns executed by such actions. For output reuse, all executions deduplicated against the first one register atomically upon deduplication and cause the cache upload to wait for all of them to complete reuse. Fixes #22501 Fixes #23288 Work towards #21578 Closes #23307 (no longer needed) Closes #23382. PiperOrigin-RevId: 668101364 Change-Id: Ice75dbe14a7dd46e02ecb096d2b2a30940216356
*** Reason for rollback *** Made //src/test/shell/bazel/remote:remote_build_event_uploader_test flaky in CI, for reasons that are still unclear. *** Original change description *** Flip --remote_cache_async. Moves uploads to a disk or remote cache to the background by default. Some actions (specifically, those that modify spawn outputs after execution) are opted out, and still upload in a blocking manner. See ActionExecutionMetadata#mayModifySpawnOutputsAfterExecution and overrides. This is technically not an incompatible change because (in the absence of bugs) it doesn't affect user-visible behavior: asynchronous uploads cannot affect the behavior of hermetic actions. Fixes #21578. *** PiperOrigin-RevId: 670587546 Change-Id: I03eec8e55c16028854d5275694ae94ca13db796f
Reopening since we had to roll back the flag flip in b3cc928. I expect to roll forward again in time for Bazel 8. |
When an action may modify a spawn's outputs after execution, the upload of outputs to the cache and reuse for deduplicated actions need to happen synchronously directly after spawn execution to avoid a race. This commit implements this for cache uploads by marking all actions with this property and simply disabling async upload for all spawns executed by such actions. For output reuse, all executions deduplicated against the first one register atomically upon deduplication and cause the cache upload to wait for all of them to complete reuse. Fixes bazelbuild#22501 Fixes bazelbuild#23288 Work towards bazelbuild#21578 Closes bazelbuild#23307 (no longer needed) Closes bazelbuild#23382. PiperOrigin-RevId: 668101364 Change-Id: Ice75dbe14a7dd46e02ecb096d2b2a30940216356
Description of the feature request:
It would be nice if the
--experimental_remote_cache_async
flag was no longer experimental and on by default. If that's not possible now, then issues should be identified and fixed as well in order to allow the flip.Which category does this issue belong to?
Remote Execution
What underlying problem are you trying to solve with this feature?
Better build times when using a remote cache.
Which operating system are you running Bazel on?
No response
What is the output of
bazel info release
?No response
If
bazel info release
returnsdevelopment 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 HEAD
?No response
Have you found anything relevant by searching the web?
No response
Any other information, logs, or outputs that you want to share?
No response
The text was updated successfully, but these errors were encountered: