-
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
not all outputs were created or valid (Remote caching, all platforms) #8125
Comments
Basically the problem is zero answer for action result is perfectly fine (??) and that leads to 0 outputs of an action, which is downloading without an error. ef7fe3d fixes that, but the problem is more general. I can guess Bazel shall have some kind of tolerance to network errors. Maybe some checksums for cache entries? |
/cc @buchgr Our remote caching/execution expert. |
Yes it's fine for an action to not produce an output file, but then it must also not declare an output file or else you'll see this error.
We do have that, but it looks like that this is not the problem here. Would have a reproducer for me to take a look at? |
|
@excitoon how does the //some:thing target look like? |
@buchgr Any cacheable target with outputs. E.g. for bazel itself:
|
@buchgr about a target without outputs and without output file, what is the caching strategy for it? Shall Bazel ask cache for that absence of files? |
@excitoon aww I see what you mean - that's indeed a bug. thanks!
This statement is not fully correctly. what toxy seems to do is send a 200 OK response with an empty body. So that's a badly behaving server. That's different from the server just terminating the connection. I think the proper fix would be to add a check that the generated outputs are a super set of the declared outputs. |
@buchgr why not exact set?
…On Wed, Apr 24, 2019, 14:34 Jakob Buchgraber ***@***.***> wrote:
@excitoon <https://github.com/excitoon> aww I see what you mean - that's
indeed a bug.
This issue happens every time when cache server (
https://github.com/neitanod/toxy) terminates connection without an answer:
This statement is not fully correctly. what toxy seems to do is send a 200
OK response with an empty body. So that's a badly behaving server. That's
different from the server just terminating the connection.
I think the proper fix would be to add a check that the generated outputs
are a super set of the declared outputs.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8125 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARGSB2HQVQPI4XM6OJYK4DPSBAVDANCNFSM4HIAHNZQ>
.
|
@excitoon it's totally legal for a command to create more output files than declared in Bazel (i.e. debug output). |
@buchgr Content-Length is not a mandatory header, so theoretically, Bazel (with server which do not set this header) can confuse accidental disconnect from graceful shutdown and fall to this bug, as I can guess.
…On Wed, Apr 24, 2019, 14:34 Jakob Buchgraber ***@***.***> wrote:
@excitoon <https://github.com/excitoon> aww I see what you mean - that's
indeed a bug.
This issue happens every time when cache server (
https://github.com/neitanod/toxy) terminates connection without an answer:
This statement is not fully correctly. what toxy seems to do is send a 200
OK response with an empty body. So that's a badly behaving server. That's
different from the server just terminating the connection.
I think the proper fix would be to add a check that the generated outputs
are a super set of the declared outputs.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8125 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARGSB2HQVQPI4XM6OJYK4DPSBAVDANCNFSM4HIAHNZQ>
.
|
That thing that 0 bytes protobuf is a valid entity is indeed interesting. |
@excitoon it's a bit unfortunate. 0 bytes is a valid protobuf in that all fields in the protobuf will have its default values. We check the |
Mostly yes. Open a PR (with a test) and we can discuss it further? I added a comment on the commit. Thanks! |
Changing the API to require a positively set field (so an empty proto isn't valid) is also bazelbuild/remote-apis#6 . I don't think it can happen until V3, since it'd be a breaking change, but would be a long-term defense against this. |
I believe Bazel should be able to handle it client side independent of the API. To the best of my knowledge if the output is of type |
Any status on this, we are hitting this problem when using
When bazel asks for that action, |
@ob: you might want to try upgrading bazel-remote, I added some ActionResult validation when implementing the gRPC interface, and then refactored the code so that the HTTP interface also has this validation. |
Duh, this part of the validation I added to bazel-remote is obviously out of spec (followup in buchgr/bazel-remote#121). |
Starting with 4.0.0 release, bazel hangs when using |
Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team ( |
This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team ( |
0.23.2
@meteorcloudy
This issue happens every time when cache server (https://github.com/neitanod/toxy) terminates connection without an answer:
The text was updated successfully, but these errors were encountered: