Support bounded parallel chunk transfers#29341
Support bounded parallel chunk transfers#29341tyler-french wants to merge 1 commit intobazelbuild:masterfrom
Conversation
|
FYI @tjgq I think this was a follow-up from the original PR |
|
@bazel-io fork 9.2.0 |
sluongng
left a comment
There was a problem hiding this comment.
I'm a bit skeptical of the current approach, so I will skip on reading the window filling implementation right now. (though Codex does suggest there is a problem there)
Each invocation may have multiple actions/spawns running in parallel, each creates multiple blob uploads/downloads, and some of those blobs are chunked blobs. Adding parallelism on the blob level feels like a local optimization, and the new flag does not offer strong control over the total parallelism of the invocation.
I wonder if we need something higher-level that lets us effectively enforce a global parallelism for uploads and downloads🤔
8969051 to
33689c3
Compare
There was a problem hiding this comment.
Pull request overview
Enables bounded parallelism for content-defined chunk (CDC) blob uploads/downloads, improving throughput while avoiding unbounded per-blob fanout.
Changes:
- Implement sliding-window style, per-blob bounded concurrency for chunk uploads in
ChunkedBlobUploader. - Implement sliding-window style, per-blob bounded concurrency for chunk downloads (including in-flight dedup) in
ChunkedBlobDownloader. - Add/expand unit tests for window refill, cancellation, and failure propagation; add a JMH benchmark binary target.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/google/devtools/build/lib/remote/ChunkedBlobUploader.java |
Adds bounded in-flight chunk upload window and cancellation on failure. |
src/main/java/com/google/devtools/build/lib/remote/ChunkedBlobDownloader.java |
Adds bounded in-flight chunk download window with reassembly and in-flight dedup. |
src/test/java/com/google/devtools/build/lib/remote/ChunkedBlobUploaderTest.java |
Adds tests for window refill, cancellation, and failure handling for parallel uploads. |
src/test/java/com/google/devtools/build/lib/remote/ChunkedBlobDownloaderTest.java |
Updates tests for new download API and adds parallel-window behavior tests. |
src/test/java/com/google/devtools/build/lib/remote/ChunkedTransferBenchmark.java |
Introduces a JMH benchmark for chunked upload/download with latency + jitter. |
src/test/java/com/google/devtools/build/lib/remote/BUILD |
Adds a java_opt_binary target to run the new benchmark. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updated this in the follow-up direction you suggested: I removed the flag/plumbing and kept only a small hardcoded per-blob window of 32 as a guard against huge single-blob fanout. The actual global active RPC limit is still the shared gRPC channel pool ( |
33689c3 to
8363b37
Compare
8363b37 to
976a25f
Compare
Description
For
--experimental_remote_cache_chunkingimplemented in #28437This PR enables parallel uploads and downloads for chunked files, to improve performance. Since the concurrency is globally bounded already by GRPC total connectsion, we create a separate bound per file to prevent too-fast fanout. This is done using 32 which is a good balance, but not too high.
To prevent issues using batches, we create simple sliding-window style transfer managers.
RELNOTES: CDC chunk uploads and downloads can now happen in parallel within a large blob.
Benchmarking:
With our synthetic benchmark of network delays and simulated jitter, the parallelism leads to a 20x improvement, but of course, this doesn't always match realistic situations.
After Change:
Before Change:
Big File: