Retry build when RemoteActionFileSystem encounters a missing digest#25358
Closed
fmeum wants to merge 1 commit intobazelbuild:masterfrom
Closed
Retry build when RemoteActionFileSystem encounters a missing digest#25358fmeum wants to merge 1 commit intobazelbuild:masterfrom
RemoteActionFileSystem encounters a missing digest#25358fmeum wants to merge 1 commit intobazelbuild:masterfrom
Conversation
fmeum
force-pushed
the
build-rewinding-jdeps
branch
4 times, most recently
from
7d8cbef to
9cb500c
Compare
fmeum
changed the title
RemoteActionFileSystem encounters a missing digest
fmeum
changed the title
RemoteActionFileSystem encounters a missing digestRemoteActionFileSystem encounters a missing digest
github-actions
Bot
added
team-Performance
github-actions
Bot
added
the
awaiting-review
Collaborator
Author
|
@justinhorvitz Could you review the interaction with the action rewinding machinery, including the changes I had to make to |
fmeum
commented
Feb 25, 2025
| catastrophe = true; | ||
| } else if (remoteCacheFailed) { | ||
| } else if (BulkTransferException.allCausedByCacheNotFoundException(exception)) { | ||
| // At this point, cache evictions that affect uploaded inputs have already been handled. |
Collaborator
Author
There was a problem hiding this comment.
@coeuvre This is a change in behavior, but I think it's for the better as it avoids retries that are very unlikely to succeed.
fmeum
commented
Feb 25, 2025
|
|
||
| # Incremental build in toplevel build triggers remote cache eviction error | ||
| # but Bazel doesn't automatically retry the build yet. | ||
| # TODO: This documents the current behavior, but it's not intended. |
Collaborator
Author
There was a problem hiding this comment.
@justinhorvitz To fix this, I would need to thread information about lost inputs obtained in
ImportantOutputHandler. I think I roughly understand what that would require, but the two calls to informImportantOutputHandler further above and the mentioning of error bubbling tell me that this is probably pretty difficult to get right. Do you have any advice for me?
coeuvre
requested changes
Feb 26, 2025
Member
coeuvre
left a comment
coeuvre
left a comment
There was a problem hiding this comment.
Thanks for the PR! I like the direction that you make build rewinding more similar to action rewinding.
If it is not too difficult to do, I would like you to split this PR into 3 PRs for easier reviews:
- A PR that overhauls the build rewinding mechanism.
- A PR that fixes build rewinding for jdeps.
- A PR that contains remaining changes in this PR that don't belong to above 2.
fmeum
force-pushed
the
build-rewinding-jdeps
branch
4 times, most recently
from
7f44f95 to
d4de887
Compare
Collaborator
Author
|
I split off #25396 with the refactoring, this PR is now stacked on it. |
fmeum
force-pushed
the
build-rewinding-jdeps
branch
3 times, most recently
from
12b24a3 to
f3fbba5
Compare
fmeum
force-pushed
the
build-rewinding-jdeps
branch
from
f3fbba5 to
29c67f1
Compare
fmeum
force-pushed
the
build-rewinding-jdeps
branch
from
29c67f1 to
2064a9c
Compare
Collaborator
Author
|
@coeuvre This is no longer stacked and ready for another review. |
coeuvre
approved these changes
Mar 10, 2025
Member
|
Importing. |
Collaborator
Author
|
@bazel-io fork 8.2.0 |
github-actions
Bot
removed
the
awaiting-review
iancha1992
pushed a commit
to iancha1992/bazel
that referenced
this pull request
Mar 12, 2025
Cache evictions encountered during reads of remote files in `RemoteActionFileSystem` now result in the build being retried when `--experimental_remote_cache_eviction_retries` is set to a positive value (the default). This is enabled by implementing the `checkForLostInputs` method on the `RemoteOutputService`, building on the refactoring performed in bazelbuild#25396. Closes bazelbuild#25358. PiperOrigin-RevId: 736064349 Change-Id: Idc70d55138c3366d22ece7f0579b242b3c217da8
iancha1992
pushed a commit
to iancha1992/bazel
that referenced
this pull request
Mar 13, 2025
Cache evictions encountered during reads of remote files in `RemoteActionFileSystem` now result in the build being retried when `--experimental_remote_cache_eviction_retries` is set to a positive value (the default). This is enabled by implementing the `checkForLostInputs` method on the `RemoteOutputService`, building on the refactoring performed in bazelbuild#25396. Closes bazelbuild#25358. PiperOrigin-RevId: 736064349 Change-Id: Idc70d55138c3366d22ece7f0579b242b3c217da8
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Mar 26, 2025
…g digest (#25538) Cache evictions encountered during reads of remote files in `RemoteActionFileSystem` now result in the build being retried when `--experimental_remote_cache_eviction_retries` is set to a positive value (the default). This is enabled by implementing the `checkForLostInputs` method on the `RemoteOutputService`, building on the refactoring performed in #25396. Closes #25358. PiperOrigin-RevId: 736064349 Change-Id: Idc70d55138c3366d22ece7f0579b242b3c217da8 Commit 85d9cc9 Co-authored-by: Fabian Meumertzheim <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cache evictions encountered during reads of remote files in
RemoteActionFileSystemnow result in the build being retried when--experimental_remote_cache_eviction_retriesis set to a positive value (the default).This is enabled by implementing the
checkForLostInputsmethod on theRemoteOutputService, building on the refactoring performed in #25396.