From 43cb21ac44bbe4a7ab4a589afa71a2c59bf1d79c Mon Sep 17 00:00:00 2001 From: chiwang Date: Tue, 17 May 2022 07:58:50 -0700 Subject: [PATCH] Manage temporary files inside AbstractActionInputPrefetcher so that we can share this code among implementations. Also, always delete tempDir after build PiperOrigin-RevId: 449221017 --- .../remote/AbstractActionInputPrefetcher.java | 38 +++++++++++++++++++ .../build/lib/remote/RemoteModule.java | 13 ------- .../build/lib/remote/RemoteOutputService.java | 10 ++++- .../lib/remote/util/TempPathGenerator.java | 2 - 4 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index 836022b18681fe..cc2675f5fc3467 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java @@ -28,12 +28,20 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.remote.util.AsyncTaskCache; import com.google.devtools.build.lib.remote.util.RxUtils.TransferResult; import com.google.devtools.build.lib.remote.util.TempPathGenerator; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution; +import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code; +import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import io.reactivex.rxjava3.core.Completable; @@ -60,6 +68,36 @@ protected AbstractActionInputPrefetcher(Path execRoot, TempPathGenerator tempPat this.tempPathGenerator = tempPathGenerator; } + public void startBuild(EventHandler eventHandler) throws AbruptExitException { + Path tempDir = tempPathGenerator.getTempDir(); + if (tempDir.exists()) { + eventHandler.handle(Event.warn("Found stale downloads from previous build, deleting...")); + try { + tempDir.deleteTree(); + } catch (IOException e) { + throw new AbruptExitException( + DetailedExitCode.of( + ExitCode.LOCAL_ENVIRONMENTAL_ERROR, + FailureDetail.newBuilder() + .setMessage( + String.format("Failed to delete stale downloads: %s", e.getMessage())) + .setRemoteExecution( + RemoteExecution.newBuilder() + .setCode(Code.DOWNLOADED_INPUTS_DELETION_FAILURE)) + .build())); + } + } + } + + public void finalizeBuild() { + Path tempDir = tempPathGenerator.getTempDir(); + try { + tempDir.deleteTree(); + } catch (IOException ignored) { + // Intentionally left empty. + } + } + protected abstract boolean shouldDownloadInput( ActionInput input, @Nullable FileArtifactValue metadata); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index afe2666401d41b..1931782d39b5a5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -921,19 +921,6 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode; if (!remoteOutputsMode.downloadAllOutputs() && actionContextProvider.getRemoteCache() != null) { Path tempDir = env.getActionTempsDirectory().getChild("remote"); - try { - if (tempDir.exists() - && (!tempDir.isDirectory() || !tempDir.getDirectoryEntries().isEmpty())) { - env.getReporter() - .handle(Event.warn("Found incomplete downloads from previous build, deleting...")); - tempDir.deleteTree(); - } - } catch (IOException e) { - throw createExitException( - e.getMessage(), - ExitCode.LOCAL_ENVIRONMENTAL_ERROR, - Code.DOWNLOADED_INPUTS_DELETION_FAILURE); - } actionInputFetcher = new RemoteActionInputFetcher( env.getBuildRequestId(), diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index b4dbebf1240764..8b5a90c71a969a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.BatchStat; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.ModifiedFileSet; @@ -76,13 +77,18 @@ public String getFilesSystemName() { @Override public ModifiedFileSet startBuild( - EventHandler eventHandler, UUID buildId, boolean finalizeActions) { + EventHandler eventHandler, UUID buildId, boolean finalizeActions) throws AbruptExitException { + if (actionInputFetcher != null) { + actionInputFetcher.startBuild(eventHandler); + } return ModifiedFileSet.EVERYTHING_MODIFIED; } @Override public void finalizeBuild(boolean buildSuccessful) { - // Intentionally left empty. + if (actionInputFetcher != null) { + actionInputFetcher.finalizeBuild(); + } } @Override diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/TempPathGenerator.java b/src/main/java/com/google/devtools/build/lib/remote/util/TempPathGenerator.java index 321d686248044f..553c95dd47a373 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/TempPathGenerator.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/TempPathGenerator.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.remote.util; -import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.vfs.Path; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.concurrent.ThreadSafe; @@ -33,7 +32,6 @@ public Path generateTempPath() { return tempDir.getChild(index.getAndIncrement() + ".tmp"); } - @VisibleForTesting public Path getTempDir() { return tempDir; }