Skip to content

Commit

Permalink
Reclassify RemoteActionFileSystem as an in-memory filesystem, and ren…
Browse files Browse the repository at this point in the history
…ame the ActionFileSystemType enum values to better match reality.

RemoteActionFileSystem, the action filesystem implementation used by Bazel, became an in-memory filesystem in revision 2876569. Its ActionFileSystemType must be recognized as such so that Skyframe creates output directories prior to local action execution.

The correspondence between old -> new enum values is as follows:

DISABLED -> DISABLED
STAGE_REMOTE_FILES -> REMOTE_FILE_SYSTEM
IN_MEMORY_FILE_SYSTEM -> IN_MEMORY_ONLY_FILE_SYSTEM
IN_MEMORY_FILE_SYSTEM_AND_STAGE_REMOTE_FILES -> STAGE_REMOTE_FILES_FILE_SYSTEM

Also fix a call site where a capability check was made by comparing with the enum value instead of calling the accessor.

PiperOrigin-RevId: 482270459
Change-Id: I8ba6a0f32d8570e81281e1e1a29b232df2ac6c85
  • Loading branch information
tjgq authored and copybara-github committed Oct 19, 2022
1 parent f8d516e commit dfccbf9
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public void prepareForExecution(UUID buildId)
startLocalOutputBuild();
}
}
if (outputService == null || outputService.actionFileSystemType().supportLocalActions()) {
if (outputService == null || outputService.actionFileSystemType().supportsLocalActions()) {
// Must be created after the output path is created above.
createActionLogDirectory();
}
Expand Down Expand Up @@ -388,7 +388,7 @@ void executeBuild(
}
}

if (outputService == null || outputService.actionFileSystemType().supportLocalActions()) {
if (outputService == null || outputService.actionFileSystemType().supportsLocalActions()) {
// Must be created after the output path is created above.
createActionLogDirectory();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void setActionInputFetcher(RemoteActionInputFetcher actionInputFetcher) {
@Override
public ActionFileSystemType actionFileSystemType() {
return actionInputFetcher != null
? ActionFileSystemType.STAGE_REMOTE_FILES
? ActionFileSystemType.REMOTE_FILE_SYSTEM
: ActionFileSystemType.DISABLED;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.SkyFunction;
Expand Down Expand Up @@ -254,9 +253,7 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env)
CheckInputResults checkedInputs = null;
NestedSet<Artifact> allInputs =
state.allInputs.getAllInputs(
!skyframeActionExecutor
.actionFileSystemType()
.equals(ActionFileSystemType.STAGE_REMOTE_FILES));
skyframeActionExecutor.actionFileSystemType().supportsInputDiscovery());

if (!state.actionInputCollectedEventSent) {
env.getListener()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ private enum DirectoryState {
}

/**
* Creates output directories for an {@link
* com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#IN_MEMORY_FILE_SYSTEM}.
* The action-local filesystem starts empty, so we expect the output directory creation to always
* Creates output directories for an in-memory action file system ({@link
* com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#inMemoryFileSystem}). The
* action-local filesystem starts empty, so we expect the output directory creation to always
* succeed. There can be no interference from state left behind by prior builds or other actions
* intra-build.
*/
Expand Down Expand Up @@ -96,8 +96,8 @@ void createActionFsOutputDirectories(
* rewinding, actions that output tree artifacts need to recreate the directories because they are
* deleted as part of the {@link com.google.devtools.build.lib.actions.Action#prepare} step.
*
* <p>Note that this does not need to be called if using {@link
* com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#IN_MEMORY_FILE_SYSTEM}.
* <p>Note that this does not need to be called if using an in-memory action file system ({@link
* com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#inMemoryFileSystem}).
*/
void invalidateTreeArtifactDirectoryCreation(ImmutableSet<Artifact> actionOutputs) {
for (Artifact output : actionOutputs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ public ActionStepOrResult run(Environment env)
statusReporter.updateStatus(event);
}
env.getListener().post(event);
if (actionFileSystemType().supportLocalActions()) {
if (actionFileSystemType().supportsLocalActions()) {
try (SilentCloseable d = profiler.profile(ProfilerTask.INFO, "action.prepare")) {
// This call generally deletes any files at locations that are declared outputs of the
// action, although some actions perform additional work, while others intentionally
Expand Down
37 changes: 22 additions & 15 deletions src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,35 +48,42 @@ public interface OutputService {
/** Properties of the action file system implementation provided by this output service. */
enum ActionFileSystemType {

/** Action file system is disabled */
/** The action file system is disabled. */
DISABLED,

/**
* The action file system implementation does not take over the output base but complements the
* file system by being able to stage remote outputs accessed as inputs by local actions, as
* used by Bazel.
* The action file system implementation is purely in-memory, taking full control of the output
* base. It's not able to stage remote outputs accessed as inputs by local actions, but is able
* to do input discovery. Used by Blaze.
*/
STAGE_REMOTE_FILES,
IN_MEMORY_ONLY_FILE_SYSTEM,

/**
* The action file system implementation is fully featured in-memory file system implementation
* and takes full control of the output base, as used by Blaze.
* The action file system implementation mixes an in-memory and a local file system. It uses the
* in-memory filesystem for in-process and remote actions, but is also aware of outputs from
* local actions. It's able to stage remote outputs accessed as inputs by local actions and to
* do input discovery. Used by Blaze.
*/
IN_MEMORY_FILE_SYSTEM,
STAGE_REMOTE_FILES_FILE_SYSTEM,

/**
* The action file system implementation mixes in-memory and local file system. It uses
* in-memory filesystem for in-process and remote actions but is also aware of outputs from
* local actions. It's able to stage remote outputs accessed as inputs by local actions.
* The action file system implementation mixes an in-memory and a local file system. It uses the
* in-memory filesystem for in-process and remote actions, but is also aware of outputs from
* local actions. It's able to stage remote outputs accessed as inputs by local actions, but
* unable to do input discovery. Used by Bazel.
*/
IN_MEMORY_FILE_SYSTEM_AND_STAGE_REMOTE_FILES;
REMOTE_FILE_SYSTEM;

public boolean inMemoryFileSystem() {
return this == IN_MEMORY_FILE_SYSTEM || this == IN_MEMORY_FILE_SYSTEM_AND_STAGE_REMOTE_FILES;
return this != DISABLED;
}

public boolean supportsLocalActions() {
return this != IN_MEMORY_ONLY_FILE_SYSTEM;
}

public boolean supportLocalActions() {
return this != IN_MEMORY_FILE_SYSTEM;
public boolean supportsInputDiscovery() {
return this != REMOTE_FILE_SYSTEM;
}

public boolean isEnabled() {
Expand Down

0 comments on commit dfccbf9

Please sign in to comment.