Skip to content

Commit 9fa9ca8

Browse files
tjgqcopybara-github
authored andcommitted
Remove EventHandler from the RemoteAnalysisCacheClient interface.
So that it can be made part of the SC/LC interface, where only simple types are allowed. The preexisting `bailOutCallback` parameter is not an issue from an SC/LC interface standpoint, but it seems pointless now that exceptions are getting bubbled up. PiperOrigin-RevId: 862734134 Change-Id: I1faf23398ade9d24b505b0d291c16c4f7a9fb3b2
1 parent c1924ee commit 9fa9ca8

File tree

2 files changed

+32
-15
lines changed

2 files changed

+32
-15
lines changed

src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
import com.google.devtools.build.lib.skyframe.serialization.analysis.ClientId.LongVersionClientId;
134134
import com.google.devtools.build.lib.skyframe.serialization.analysis.FrontierSerializer;
135135
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheClient;
136+
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheClient.LookupTopLevelTargetsResult;
136137
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider;
137138
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingEventListener;
138139
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingOptions;
@@ -1730,16 +1731,30 @@ public void queryMetadataAndMaybeBailout() throws InterruptedException {
17301731
if (skycacheMetadataParams.getTargets().isEmpty()) {
17311732
eventHandler.handle(
17321733
Event.warn(
1733-
"Skycache: Not querying Skycache metadata because invocation has no" + " targets"));
1734+
"Skycache: Not querying Skycache metadata because invocation has no targets"));
17341735
} else {
1735-
getAnalysisCacheClient()
1736-
.lookupTopLevelTargets(
1737-
skycacheMetadataParams.getEvaluatingVersion(),
1738-
skycacheMetadataParams.getConfigurationHash(),
1739-
skycacheMetadataParams.getUseFakeStampData(),
1740-
skycacheMetadataParams.getBazelVersion(),
1741-
eventHandler,
1742-
() -> bailedOut = true);
1736+
try {
1737+
LookupTopLevelTargetsResult result =
1738+
getAnalysisCacheClient()
1739+
.lookupTopLevelTargets(
1740+
skycacheMetadataParams.getEvaluatingVersion(),
1741+
skycacheMetadataParams.getConfigurationHash(),
1742+
skycacheMetadataParams.getUseFakeStampData(),
1743+
skycacheMetadataParams.getBazelVersion());
1744+
1745+
Event event =
1746+
switch (result.status()) {
1747+
case MATCH_STATUS_MATCH -> Event.info("Skycache: " + result.statusMessage());
1748+
case MATCH_STATUS_FAILURE -> Event.warn("Skycache: " + result.statusMessage());
1749+
default -> {
1750+
bailedOut = true;
1751+
yield Event.warn("Skycache: " + result.statusMessage());
1752+
}
1753+
};
1754+
eventHandler.handle(event);
1755+
} catch (ExecutionException | TimeoutException e) {
1756+
eventHandler.handle(Event.warn("Skycache: Error with metadata store: " + e.getMessage()));
1757+
}
17431758
}
17441759
}
17451760

src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCacheClient.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,20 @@
1515
package com.google.devtools.build.lib.skyframe.serialization.analysis;
1616

1717
import com.google.common.util.concurrent.ListenableFuture;
18-
import com.google.devtools.build.lib.events.EventHandler;
1918
import com.google.devtools.build.lib.skyframe.serialization.analysis.proto.TopLevelTargetsMatchStatus;
2019
import com.google.protobuf.ByteString;
20+
import java.util.concurrent.ExecutionException;
21+
import java.util.concurrent.TimeoutException;
2122

2223
/** Interface to the remote analysis cache. */
2324
public interface RemoteAnalysisCacheClient {
2425

2526
/** Timeout when accessing the future in order to shutdown the client. */
2627
int SHUTDOWN_TIMEOUT_IN_SECONDS = 5;
2728

29+
/** The result of a top-level targets lookup. */
30+
record LookupTopLevelTargetsResult(TopLevelTargetsMatchStatus status, String statusMessage) {}
31+
2832
/** Usage statistics. */
2933
record Stats(
3034
long bytesSent,
@@ -40,12 +44,10 @@ record Stats(
4044
Stats getStats();
4145

4246
/** Looks up the targets in the metadata table */
43-
void lookupTopLevelTargets(
47+
LookupTopLevelTargetsResult lookupTopLevelTargets(
4448
long evaluatingVersion,
4549
String configurationHash,
4650
boolean useFakeStampData,
47-
String bazelVersion,
48-
EventHandler eventHandler,
49-
Runnable bailOutCallback)
50-
throws InterruptedException;
51+
String bazelVersion)
52+
throws ExecutionException, TimeoutException, InterruptedException;
5153
}

0 commit comments

Comments
 (0)