search

pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.33k stars 638 forks source link

Consider (optionally?) skipping action cache input uploads #12432

Closed stuhood closed 3 years ago

stuhood commented 3 years ago

Currently when we write to the action cache, we upload the inputs of the action: https://github.com/pantsbuild/pants/blob/f6b99aeb8c25757efa3d68b4b28485ae985da168/src/rust/engine/process_execution/src/remote_cache.rs#L448-L465 But it is not necessary for the inputs to exist in the store for us to hit the cache (only the outputs), so we should evaluate whether we need to upload inputs to be spec compliant, and skip it if not.

stuhood commented 3 years ago

The spec seems to require that all inputs be uploaded: https://github.com/pantsbuild/pants/blob/54fb09d1e1c38fd99ea524322d62917099594c5b/src/rust/engine/bazel_protos/protos/bazelbuild_remote-apis/build/bazel/remote/execution/v2/remote_execution.proto#L161-L171 ... so assuming that "upload the Command" implies "uploading all of the inputs to the Command", I think that if we implement this, it would need to be optional.

Thoughts @tdyas, @illicitonion ?

illicitonion commented 3 years ago

The way I read that, it's specifically saying you must upload the content of the Action proto, and the Command proto (likely totalling hundreds to thousands of bytes), and doesn't say anything about the input root referenced by the Action proto itself?

Eric-Arellano commented 3 years ago

That makes sense, @illicitonion. Maybe we could post an issue in the REAPI repo to confirm? Or the mailing group.

stuhood commented 3 years ago

Asked in: https://groups.google.com/g/remote-execution-apis/c/P80gfX4G6WU/m/64xquaJEBgAJ