remoting: check action cache for cached result before submitting a request

[tdyas]

Problem

The REv2 specification does not explicitly require servers to return cached results from the Execute method of the Execution service. The specification hints that servers might do so, but stops short of actually mandating it, stating that:

When a server completes execution of an Action, it MAY choose to cache the result in the ActionCache unless do_not_cache is true. Clients SHOULD expect the server to do so. By default, future calls to Execute the same Action will also serve their results from the cache. Clients must take care to understand the caching behaviour.

The RE client in Pants does not currently call GetActionResult to check for cached results before submitting an execution request. This is not problematic if the server checks the cache and short-circuits Execute calls with any cached results. This may be the case for RBE, but is intentionally not the case for BuildBarn. Bazel always calls GetActionResult, and so the BuildBarn team determined it to be duplicative to have Execute also check the cache a second time (which would negatively impact performance). (I have not audited what Buildgrid and Buildfarm do.) See discussion at https://buildteamworld.slack.com/archives/CD6HZC750/p1593720268111500.

Thus, at least when submitting work to BuildBarn, Pants will not take advantage of the Action Cache and the server will redo work that has already been performed and stored in the Action Cache.

Solution

Pants should call GetActionResult to check for a cached result before it attempts to upload and submit the Action and Command protos for execution.

Testing: The test framework's mock server now answers GetActionResult requests. The new test successful_served_from_action_cache tests that Pants checks the Action Cache first. (The other unit tests handle the fact that GetActionResult calls are made, but assume nothing was cached.)

Result

I used an early iteration of this PR to run tests of Toolchain's internal repo against BuildBarn. With this PR, the second run is cached and completes in about a minute. Without this PR, that run actually submits execution requests and duplicates work with a significant impact on build performance.

[stuhood]

Awesome. Thanks!

[tdyas]

Some of the CI shards failed due to the RBE issue, and will need restarting.

[stuhood]

@tdyas : It's currently disabled, but let's see what #10284 says about re-enabling.