Repository navigation
Conversation
…ts it QueryWriteStatus reported the compressed bytes received so far for an active compressed upload. A compressed upload can't be resumed, though: every Write starts a fresh decoder that only accepts offset 0. A client that queried while the server still held its dropped stream was sent back at that offset, and the new Write failed with InvalidArgument, which Bazel doesn't retry. Report 0 instead, so the retry starts over and succeeds.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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
1 participant
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.
What and why
If a compressed upload drops partway through and the client calls QueryWriteStatus before the server has noticed the old stream is gone, we report the compressed bytes received so far. The client then resumes at that offset, but a compressed upload can't actually be resumed: each Write starts a fresh decoder that only takes offset 0. So the retry fails with InvalidArgument, and Bazel treats that as fatal, which fails the build.
We hit this with large iOS artifacts uploading from a Mac with
--remote_cache_compression. This changes QueryWriteStatus to report 0 for an in-progress compressed upload, so the client starts over and the retry goes through. Uncompressed uploads are unchanged.Related to #2888 / #2889, which fix the uncompressed version of this. Compressed uploads go through
inner_write_compressedrather thancreate_or_join_upload_stream, so #2889 doesn't cover them.How was this verified?
Replaced
zstd_write_query_status_reports_compressed_wire_byteswithzstd_write_query_status_restarts_interrupted_upload_at_zero. It sends half a compressed blob on one stream and leaves it open, checks QueryWriteStatus says 0, then retries the whole blob at offset 0 on a second stream while the first is still open, and reads the stored blob back to compare bytes. On main it fails at the QueryWriteStatus check (it returns the half-chunk length); with the change it passes.cargo test -p nativelink-service --test bytestream_server_testpasses (32 tests) and clippy is clean for the crate.Not tested against a live deployment yet.
Risk
Low. It only changes what QueryWriteStatus returns for compressed uploads that are still in flight. A client that previously "resumed" at the reported offset was already failing, so nothing that worked before should break. The trade-off is that progress is no longer visible through QueryWriteStatus for compressed uploads; I don't think anything relied on it, since no offset other than 0 was usable anyway.
The cost is that an interrupted compressed upload re-sends the whole blob. Real resumption is possible in principle (keep the compressed bytes received so far, or the decoder state, and continue from there), but the server decodes on the fly and keeps neither once the stream ends, so supporting it would be a bigger change: where partial uploads live, when they expire, and so on. This PR just makes the answer honest; happy to open an issue for proper resumption if that's wanted.
AI assistance
Claude Code (an agent) wrote the code change, the test, and this description. I reviewed all of it.