Skip to content

Commit b587be3

Browse files
coeuvrecoeuvre
authored
Remote: Use Action's salt field to differentiate cache across workspaces. (bazelbuild#14320)
Resolves bazelbuild#13339 (comment). Closes bazelbuild#14184. PiperOrigin-RevId: 410407819
1 parent e53ae63 commit b587be3

File tree

8 files changed

+58
-36
lines changed

8 files changed

+58
-36
lines changed

‎src/main/java/com/google/devtools/build/lib/analysis/platform/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ java_library(
4444
srcs = ["PlatformUtils.java"],
4545
deps = [
4646
"//src/main/java/com/google/devtools/build/lib/actions",
47-
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
4847
"//src/main/java/com/google/devtools/build/lib/remote/options",
4948
"//src/main/protobuf:failure_details_java_proto",
5049
"//third_party:guava",

‎src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.google.common.base.Strings;
2020
import com.google.common.collect.ImmutableSortedMap;
2121
import com.google.common.collect.Ordering;
22-
import com.google.devtools.build.lib.actions.ExecutionRequirements;
2322
import com.google.devtools.build.lib.actions.Spawn;
2423
import com.google.devtools.build.lib.actions.UserExecException;
2524
import com.google.devtools.build.lib.remote.options.RemoteOptions;
@@ -115,16 +114,6 @@ public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions rem
115114
}
116115
}
117116

118-
String workspace =
119-
spawn.getExecutionInfo().get(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE);
120-
if (workspace != null) {
121-
platformBuilder.addProperties(
122-
Property.newBuilder()
123-
.setName("bazel-differentiate-workspace-cache")
124-
.setValue(workspace)
125-
.build());
126-
}
127-
128117
sortPlatformProperties(platformBuilder);
129118
return platformBuilder.build();
130119
}

‎src/main/java/com/google/devtools/build/lib/remote/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ java_library(
4848
"//src/main/java/com/google/devtools/build/lib/actions",
4949
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
5050
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
51+
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
5152
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
5253
"//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink",
5354
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",

‎src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
7474
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
7575
import com.google.devtools.build.lib.actions.ExecException;
76+
import com.google.devtools.build.lib.actions.ExecutionRequirements;
7677
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
7778
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
7879
import com.google.devtools.build.lib.actions.MetadataProvider;
@@ -307,6 +308,11 @@ public String getActionId() {
307308
return actionKey.getDigest().getHash();
308309
}
309310

311+
/** Returns underlying {@link Action} of this remote action. */
312+
public Action getAction() {
313+
return action;
314+
}
315+
310316
/**
311317
* Returns a {@link SortedMap} which maps from input paths for remote action to {@link
312318
* ActionInput}.
@@ -420,6 +426,22 @@ public MerkleTree uncachedBuildMerkleTreeVisitor(
420426
return MerkleTree.merge(subMerkleTrees, digestUtil);
421427
}
422428

429+
@Nullable
430+
private static ByteString buildSalt(Spawn spawn) {
431+
String workspace =
432+
spawn.getExecutionInfo().get(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE);
433+
if (workspace != null) {
434+
Platform platform =
435+
Platform.newBuilder()
436+
.addProperties(
437+
Platform.Property.newBuilder().setName("workspace").setValue(workspace).build())
438+
.build();
439+
return platform.toByteString();
440+
}
441+
442+
return null;
443+
}
444+
423445
/** Creates a new {@link RemoteAction} instance from spawn. */
424446
public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context)
425447
throws IOException, UserExecException, ForbiddenActionInputException {
@@ -442,7 +464,8 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
442464
merkleTree.getRootDigest(),
443465
platform,
444466
context.getTimeout(),
445-
Spawns.mayBeCachedRemotely(spawn));
467+
Spawns.mayBeCachedRemotely(spawn),
468+
buildSalt(spawn));
446469

447470
ActionKey actionKey = digestUtil.computeActionKey(action);
448471

‎src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,13 @@ public ExecutionResult execute(
132132
Digest commandHash = digestUtil.compute(command);
133133
MerkleTree merkleTree = MerkleTree.build(inputFiles, digestUtil);
134134
Action action =
135-
buildAction(commandHash, merkleTree.getRootDigest(), platform, timeout, acceptCached);
135+
buildAction(
136+
commandHash,
137+
merkleTree.getRootDigest(),
138+
platform,
139+
timeout,
140+
acceptCached,
141+
/*salt=*/ null);
136142
Digest actionDigest = digestUtil.compute(action);
137143
ActionKey actionKey = new ActionKey(actionDigest);
138144
CachedActionResult cachedActionResult;

‎src/main/java/com/google/devtools/build/lib/remote/util/Utils.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,8 @@ public static Action buildAction(
433433
Digest inputRoot,
434434
@Nullable Platform platform,
435435
java.time.Duration timeout,
436-
boolean cacheable) {
436+
boolean cacheable,
437+
@Nullable ByteString salt) {
437438
Action.Builder action = Action.newBuilder();
438439
action.setCommandDigest(command);
439440
action.setInputRootDigest(inputRoot);
@@ -446,6 +447,9 @@ public static Action buildAction(
446447
if (platform != null) {
447448
action.setPlatform(platform);
448449
}
450+
if (salt != null) {
451+
action.setSalt(salt);
452+
}
449453
return action.build();
450454
}
451455

‎src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformUtilsTest.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import build.bazel.remote.execution.v2.Platform;
2020
import com.google.common.collect.ImmutableList;
2121
import com.google.common.collect.ImmutableMap;
22-
import com.google.devtools.build.lib.actions.ExecutionRequirements;
2322
import com.google.devtools.build.lib.actions.Spawn;
2423
import com.google.devtools.build.lib.exec.util.SpawnBuilder;
2524
import com.google.devtools.build.lib.remote.options.RemoteOptions;
@@ -99,26 +98,6 @@ public void testParsePlatformSortsProperties_execProperties() throws Exception {
9998
assertThat(PlatformUtils.getPlatformProto(s, null)).isEqualTo(expected);
10099
}
101100

102-
@Test
103-
public void testGetPlatformProto_differentiateWorkspace() throws Exception {
104-
Spawn s =
105-
new SpawnBuilder("dummy")
106-
.withExecutionInfo(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "aa")
107-
.build();
108-
109-
Platform expected =
110-
Platform.newBuilder()
111-
.addProperties(Platform.Property.newBuilder().setName("a").setValue("1"))
112-
.addProperties(Platform.Property.newBuilder().setName("b").setValue("2"))
113-
.addProperties(
114-
Platform.Property.newBuilder()
115-
.setName("bazel-differentiate-workspace-cache")
116-
.setValue("aa"))
117-
.build();
118-
// execProperties are sorted by key
119-
assertThat(PlatformUtils.getPlatformProto(s, remoteOptions())).isEqualTo(expected);
120-
}
121-
122101
@Test
123102
public void getPlatformProto_mergeTargetExecPropertiesWithPlatform() throws Exception {
124103
Spawn spawn = new SpawnBuilder("dummy").withExecProperties(ImmutableMap.of("c", "3")).build();

‎src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import build.bazel.remote.execution.v2.OutputDirectory;
4141
import build.bazel.remote.execution.v2.OutputFile;
4242
import build.bazel.remote.execution.v2.OutputSymlink;
43+
import build.bazel.remote.execution.v2.Platform;
4344
import build.bazel.remote.execution.v2.RequestMetadata;
4445
import build.bazel.remote.execution.v2.SymlinkNode;
4546
import build.bazel.remote.execution.v2.Tree;
@@ -59,6 +60,7 @@
5960
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
6061
import com.google.devtools.build.lib.actions.ArtifactRoot;
6162
import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
63+
import com.google.devtools.build.lib.actions.ExecutionRequirements;
6264
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
6365
import com.google.devtools.build.lib.actions.ResourceSet;
6466
import com.google.devtools.build.lib.actions.SimpleSpawn;
@@ -76,6 +78,7 @@
7678
import com.google.devtools.build.lib.events.Reporter;
7779
import com.google.devtools.build.lib.events.StoredEventHandler;
7880
import com.google.devtools.build.lib.exec.util.FakeOwner;
81+
import com.google.devtools.build.lib.exec.util.SpawnBuilder;
7982
import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction;
8083
import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult;
8184
import com.google.devtools.build.lib.remote.common.BulkTransferException;
@@ -164,6 +167,24 @@ public final void setUp() throws Exception {
164167
remoteActionExecutionContext = RemoteActionExecutionContext.create(metadata);
165168
}
166169

170+
@Test
171+
public void buildRemoteAction_differentiateWorkspace_generateActionSalt() throws Exception {
172+
Spawn spawn =
173+
new SpawnBuilder("dummy")
174+
.withExecutionInfo(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "aa")
175+
.build();
176+
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
177+
RemoteExecutionService service = newRemoteExecutionService();
178+
179+
RemoteAction remoteAction = service.buildRemoteAction(spawn, context);
180+
181+
Platform expected =
182+
Platform.newBuilder()
183+
.addProperties(Platform.Property.newBuilder().setName("workspace").setValue("aa"))
184+
.build();
185+
assertThat(remoteAction.getAction().getSalt()).isEqualTo(expected.toByteString());
186+
}
187+
167188
@Test
168189
public void downloadOutputs_outputFiles_executableBitIgnored() throws Exception {
169190
// Test that executable bit of downloaded output files are ignored since it will be chmod 555

0 commit comments

Comments
 (0)