Skip to content

Commit

Permalink
Fixing #3552: re-execute cached orphaned Actions.
Browse files Browse the repository at this point in the history
A bug was introduced in patch 9626bb4, where a cache miss would not result in action re-execution, making the cache miss non-recoverable.

RELNOTES: fixes #3552
PiperOrigin-RevId: 165434579
  • Loading branch information
olaola authored and iirina committed Aug 17, 2017
1 parent d21d5d6 commit 65e0f78
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy)
try {
return downloadRemoteResults(cachedResult, policy.getFileOutErr());
} catch (CacheNotFoundException e) {
// Intentionally left empty. No cache hit, so we fall through to local or
// remote execution.
// No cache hit, so we fall through to local or remote execution.
// We set acceptCachedResult to false in order to force the action re-execution.
acceptCachedResult = false;
}
}
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -728,4 +728,82 @@ public void read(ReadRequest request, StreamObserver<ReadResponse> responseObser
assertThat(((CacheNotFoundException) t).getMissingDigest()).isEqualTo(stdOutDigest);
}
}

@Test
public void remotelyReExecuteOrphanedCachedActions() throws Exception {
final Digest stdOutDigest = Digests.computeDigestUtf8("stdout");
final ActionResult actionResult =
ActionResult.newBuilder().setStdoutDigest(stdOutDigest).build();
serviceRegistry.addService(
new ActionCacheImplBase() {
@Override
public void getActionResult(
GetActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
responseObserver.onNext(actionResult);
responseObserver.onCompleted();
}
});
serviceRegistry.addService(
new ByteStreamImplBase() {
@Override
public void read(ReadRequest request, StreamObserver<ReadResponse> responseObserver) {
// All reads are a cache miss.
responseObserver.onError(Status.NOT_FOUND.asRuntimeException());
}

@Override
public StreamObserver<WriteRequest> write(
StreamObserver<WriteResponse> responseObserver) {
return new StreamObserver<WriteRequest>() {
@Override
public void onNext(WriteRequest request) {}

@Override
public void onCompleted() {
responseObserver.onCompleted();
}

@Override
public void onError(Throwable t) {
fail("An error occurred: " + t);
}
};
}
});
serviceRegistry.addService(
new ExecutionImplBase() {
@Override
public void execute(ExecuteRequest request, StreamObserver<Operation> responseObserver) {
assertThat(request.getSkipCacheLookup()).isTrue(); // Action will be re-executed.
responseObserver.onNext(
Operation.newBuilder()
.setDone(true)
.setResponse(
Any.pack(ExecuteResponse.newBuilder().setResult(actionResult).build()))
.build());
responseObserver.onCompleted();
}
});
serviceRegistry.addService(
new ContentAddressableStorageImplBase() {
@Override
public void findMissingBlobs(
FindMissingBlobsRequest request,
StreamObserver<FindMissingBlobsResponse> responseObserver) {
// Nothing is missing.
responseObserver.onNext(FindMissingBlobsResponse.getDefaultInstance());
responseObserver.onCompleted();
}
});

try {
client.exec(simpleSpawn, simplePolicy);
fail("Expected an exception");
} catch (EnvironmentalExecException expected) {
assertThat(expected).hasMessageThat().contains("Failed to download from remote cache");
Throwable t = expected.getCause();
assertThat(t).isInstanceOf(CacheNotFoundException.class);
assertThat(((CacheNotFoundException) t).getMissingDigest()).isEqualTo(stdOutDigest);
}
}
}

0 comments on commit 65e0f78

Please sign in to comment.