Skip to content

remote: add download idle timeout#29916

Open
sluongng wants to merge 4 commits into
bazelbuild:masterfrom
sluongng:sluongng/remote-grpc-idle-timeouts
Open

remote: add download idle timeout#29916
sluongng wants to merge 4 commits into
bazelbuild:masterfrom
sluongng:sluongng/remote-grpc-idle-timeouts

Conversation

@sluongng

Copy link
Copy Markdown
Contributor

Stacked on #29912.

The existing remote timeout applies to whole RPCs. That is awkward for large downloads because a healthy ByteStream.Read can take longer than short metadata calls, while a stuck read should still be retried quickly after response data stops arriving.

Add --remote_download_idle_timeout for ByteStream.Read. When non-zero, Bazel cancels the read if no response data arrives within the configured duration, allowing the normal retrier to issue a new request. Scope this to downloads rather than all streaming RPCs so long-running Execute/WaitExecution streams and ByteStream writes keep their existing deadline behavior.

ByteStream.Read is the only affected RPC today. The download-specific name leaves room to cover other server-streaming download RPCs in the future, such as SplitChunks from bazelbuild/remote-apis#377, without implying that all remote streams should use this timeout.

@sluongng sluongng marked this pull request as ready for review June 20, 2026 09:53
@sluongng sluongng requested a review from a team as a code owner June 20, 2026 09:53
@github-actions github-actions Bot added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jun 20, 2026
@meisterT meisterT requested a review from coeuvre June 25, 2026 07:28
Comment thread src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java Outdated
sluongng added 4 commits June 29, 2026 18:05
Remote cache users need a path to per-RPC timeout policy, but Bazel
currently applies --remote_timeout by setting deadlines directly on
individual remote stubs. This change is needed so timeout policy can
move to one channel-level mechanism before any new user-facing knob is
added.

Generate an equivalent service config from the existing remote timeout
setting and install it on remote gRPC channels. This preserves the
current user-facing flag while moving timeout configuration to the same
mechanism that later changes can extend for per-RPC policy.
Remote cache users need finer timeout control than the single
--remote_timeout value can express. This change lets advanced users
provide a restricted gRPC service config file with timeout values for
specific remote RPCs.

Only the documented timeout subset is supported initially. Bazel can
expand the accepted service config surface later without treating retry,
hedging, load balancing, or health-checking behavior as stable today.
The existing remote timeout applies to whole RPCs. That is awkward
for large downloads because a healthy ByteStream.Read can take longer
than short metadata calls, while a stuck read should still be retried
quickly after response data stops arriving.

Add --remote_download_idle_timeout for ByteStream.Read. When non-zero,
Bazel cancels the read if no response data arrives within the configured
duration, allowing the normal retrier to issue a new request. Scope this
to downloads rather than all streaming RPCs so long-running
Execute/WaitExecution streams and ByteStream writes keep their existing
deadline behavior.

The download-specific name also leaves room for future server-streamed
download RPCs such as SplitChunks, without implying that all remote
streams should use this timeout.
Coeuvre requested two changes on PR bazelbuild#29916 because the new idle timeout only applies to gRPC downloads, and callback forwarding should not be skipped if timeout bookkeeping throws. Rename the option to --remote_grpc_download_idle_timeout and wrap the forwarding points in finally blocks.

The rebase onto current master also exposed two ByteStreamUploaderTest calls that still passed the removed callTimeoutSecs parameter. Drop that stale argument so the remote test target compiles against the rebased uploader constructor.
@sluongng sluongng force-pushed the sluongng/remote-grpc-idle-timeouts branch from f43b752 to 50f6aa8 Compare June 29, 2026 21:25
@sluongng

sluongng commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

I will rebase + review this PR once more after #29912 is merged. Currently, it would be a pain having to rebase 2 PRs and trigger CI for each round of review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants