Add --experimental_remote_upload_mode flag for async cache uploads #27858
Conversation
Phase 1: Add RemoteUploadMode enum and --experimental_remote_upload_mode flag to control whether remote cache uploads block build completion. Part of bazelbuild#14638.
- Clean up temp files and RPC log after async uploads complete - Add blazeShutdown() to wait for pending uploads on server shutdown
github-actions
bot
added
team-Remote-Exec
brianduff
force-pushed
the
experimental-remote-upload-mode
branch
from
0323fc9 to
110d2bc
Compare
| } | ||
|
|
||
| @Option( | ||
| name = "experimental_remote_upload_mode", |
There was a problem hiding this comment.
Instead of adding a new flag, how about turning --remote_cache_async into an enum-valued flag with three states? A BoolOrEnumConverter would preserve backwards compatibility.
| // Only track futures for async upload modes | ||
| final boolean trackUpload = | ||
| remoteOptions.experimentalRemoteUploadMode != RemoteUploadMode.WAIT_FOR_UPLOAD_COMPLETE; | ||
| final SettableFuture<Void> uploadFuture = trackUpload ? SettableFuture.create() : null; |
There was a problem hiding this comment.
This is quite subtle, but I think that Futures and in particular SettableFutures are not the right tool here:
- When upload futures are canceled because they take too long, the cancelation should propagate to the actual download futures. But the
SettableFutures aren't linked to those in any way, so I don't think that would be the case. - Future cancelation is an immediate operation that doesn't wait for the canceled task to complete. This can be problematic if an upload continues for some time while the uploaded file is already being overwritten by the new invocation.
Instead, we could probably use awaitTermination on the backgroundTaskExecutor and call shutdownNow if it takes too long.
| Stopwatch stopwatch = Stopwatch.createStarted(); | ||
| try { | ||
| // Wait up to 30 seconds for uploads from previous invocation to complete | ||
| Uninterruptibles.getUninterruptibly(future, 30, SECONDS); |
There was a problem hiding this comment.
This should let the user know what we are waiting for. It should probably also be interruptible to respect Ctrl + C.
Instead of a fixed timeout, perhaps canceling downloads on the first Ctrl + C would be an option? That would be more difficult to implement though and could be done as follow-up work.
|
@brianduff Could you please take a look at the failing checks? |
bharadwaj08-one
added
awaiting-user-response
3 participants
This PR adds a new
--experimental_remote_upload_modeflag that controls when Bazel waits for remote cache uploads to complete, similar to--bes_upload_mode.Motivation
Currently, Bazel blocks at the end of every build waiting for all remote cache uploads to complete. For builds with large outputs, or on slow connections, this can add significant time to the build even though the uploads don't affect build correctness. Users who want faster build completion times should be able to defer this waiting.
Implementation
The flag supports two modes:
wait_for_upload_complete(default): Current behavior - block at end of buildnowait_for_upload_complete: Return immediately, wait at start of next commandWhen using
nowait_for_upload_complete:bazel shutdown) also waits up to 30 secondsChanges
RemoteOptions.java: AddRemoteUploadModeenum and--experimental_remote_upload_modeflagRemoteExecutionService.java: ModifyuploadOutputs()andshutdown()to accept upload modeRemoteModule.java: Add cross-invocation state (pendingUploadsFuture), wait logic inbeforeCommand(), resource cleanup, andblazeShutdown()handlingTesting
RemoteUploadModeIntegrationTest.javashutdown(RemoteUploadMode)inRemoteExecutionServiceTest.javaCloses #14638