diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index f264168e8f7cd9..a5c04e96ffc6b2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -116,6 +116,8 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -154,6 +156,7 @@ public class BazelRepositoryModule extends BlazeModule { private LockfileMode bazelLockfileMode = LockfileMode.OFF; private List allowedYankedVersions = ImmutableList.of(); private SingleExtensionEvalFunction singleExtensionEvalFunction; + private final ExecutorService repoFetchingWorkerThreadPool = Executors.newFixedThreadPool(100); @Nullable private CredentialModule credentialModule; @@ -312,6 +315,19 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { RepositoryOptions repoOptions = env.getOptions().getOptions(RepositoryOptions.class); if (repoOptions != null) { + switch (repoOptions.workerForRepoFetching) { + case OFF: + starlarkRepositoryFunction.setWorkerExecutorService(null); + break; + case PLATFORM: + starlarkRepositoryFunction.setWorkerExecutorService(repoFetchingWorkerThreadPool); + break; + case VIRTUAL: + throw new AbruptExitException( + detailedExitCode( + "using a virtual worker thread for repo fetching is not yet supported", + Code.BAD_DOWNLOADER_CONFIG)); + } downloadManager.setDisableDownload(repoOptions.disableDownload); if (repoOptions.repositoryDownloaderRetries >= 0) { downloadManager.setRetries(repoOptions.repositoryDownloaderRetries); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java index 4f28fac3bc6709..7be04e89df8b88 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java @@ -236,6 +236,31 @@ public class RepositoryOptions extends OptionsBase { + "give, and in this case multiple URLs will be returned.") public String downloaderConfig; + /** See {@link #workerForRepoFetching}. */ + public enum WorkerForRepoFetching { + OFF, + PLATFORM, + VIRTUAL; + + static class Converter extends EnumConverter { + public Converter() { + super(WorkerForRepoFetching.class, "worker for repo fetching"); + } + } + } + + @Option( + name = "experimental_worker_for_repo_fetching", + defaultValue = "off", + converter = WorkerForRepoFetching.Converter.class, + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "The threading mode to use for repo fetching. If set to 'off', no worker thread is used," + + " and the repo fetching is subject to restarts. Otherwise, uses a platform thread" + + " (i.e. OS thread) if set to 'platform' or a virtual thread if set to 'virtual'.") + public WorkerForRepoFetching workerForRepoFetching; + @Option( name = "ignore_dev_dependency", defaultValue = "false", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 51564c20b36ee8..62ed8b736c3f7a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD @@ -55,6 +55,7 @@ java_library( "//src/main/java/net/starlark/java/annot", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", + "//third_party:auto_value", "//third_party:error_prone_annotations", "//third_party:guava", "//third_party:java-diff-utils", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java new file mode 100644 index 00000000000000..55473ddf64b0df --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java @@ -0,0 +1,82 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.bazel.repository.starlark; + +import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Future; +import java.util.concurrent.SynchronousQueue; +import javax.annotation.Nullable; + +/** + * Captures state that persists across different invocations of {@link + * com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction}, specifically {@link + * StarlarkRepositoryFunction}. + * + *

This class is used to hold on to a worker thread (in reality just a {@link Future} object) + * when fetching repos using a worker thread is enabled. The worker thread uses a {@link + * SkyFunction.Environment} object acquired from the host thread, and can signal the host thread to + * restart to get a fresh environment object. + */ +class RepoFetchingSkyKeyComputeState implements SkyKeyComputeState { + + /** A signal that the worker thread can send to the host Skyframe thread. */ + enum Signal { + /** + * Indicates that the host thread should return {@code null}, causing a Skyframe restart. After + * sending this signal, the client will immediately block on {@code delegateEnvQueue}, waiting + * for the host thread to send a fresh {@link SkyFunction.Environment} over. + */ + RESTART, + /** + * Indicates that the worker thread has finished running, either yielding a result or an + * exception. + */ + DONE + } + + /** The channel for the worker thread to send a signal to the host Skyframe thread. */ + final BlockingQueue signalQueue = new SynchronousQueue<>(); + /** + * The channel for the host Skyframe thread to send fresh {@link SkyFunction.Environment} objects + * back to the worker thread. + */ + final BlockingQueue delegateEnvQueue = new SynchronousQueue<>(); + /** + * This future holds on to the worker thread in order to cancel it when necessary; it also serves + * to tell whether a worker thread is already running. + */ + // This is volatile since we set it to null to indicate the worker thread isn't running, and this + // could happen on multiple threads. Canceling a future multiple times is safe, though, so we + // only need to worry about nullness. Using a mutex/synchronization is an alternative but it means + // we might block in `close()`, which is potentially bad (see its javadoc). + @Nullable volatile Future workerFuture = null; + + SkyFunction.Environment signalForFreshEnv() throws InterruptedException { + signalQueue.put(Signal.RESTART); + return delegateEnvQueue.take(); + } + + @Override + public void close() { + var myWorkerFuture = workerFuture; + workerFuture = null; + if (myWorkerFuture != null) { + myWorkerFuture.cancel(true); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingWorkerSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingWorkerSkyFunctionEnvironment.java new file mode 100644 index 00000000000000..e9c38fa1529eb9 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingWorkerSkyFunctionEnvironment.java @@ -0,0 +1,185 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.bazel.repository.starlark; + +import com.google.common.collect.ImmutableList; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.SkyframeLookupResult; +import com.google.devtools.build.skyframe.Version; +import java.util.function.Supplier; +import javax.annotation.Nullable; + +/** + * A {@link SkyFunction.Environment} implementation designed to be used in a different thread than + * the corresponding SkyFunction runs in. It relies on a delegate Environment object to do + * underlying work. Its {@link #getValue} and {@link #getValueOrThrow} methods do not return {@code + * null} when the {@link SkyValue} in question is not available. Instead, it blocks and waits for + * the host Skyframe thread to restart, and replaces the delegate Environment with a fresh one from + * the restarted SkyFunction before continuing. (Note that those methods do return {@code + * null} if the SkyValue was evaluated but found to be in error.) + * + *

Crucially, the delegate Environment object must not be used by multiple threads at the same + * time. In effect, this is guaranteed by only one of the worker thread and host thread being active + * at any given time. + */ +class RepoFetchingWorkerSkyFunctionEnvironment + implements SkyFunction.Environment, ExtendedEventHandler, SkyframeLookupResult { + private final RepoFetchingSkyKeyComputeState state; + private SkyFunction.Environment delegate; + + RepoFetchingWorkerSkyFunctionEnvironment( + RepoFetchingSkyKeyComputeState state, SkyFunction.Environment delegate) { + this.state = state; + this.delegate = delegate; + } + + @Override + public boolean valuesMissing() { + return delegate.valuesMissing(); + } + + @Override + public SkyframeLookupResult getValuesAndExceptions(Iterable depKeys) + throws InterruptedException { + delegate.getValuesAndExceptions(depKeys); + if (!delegate.valuesMissing()) { + // Do NOT just return the return value of `delegate.getValuesAndExceptions` here! That would + // cause anyone holding onto the returned // result object to potentially use a stale version + // of it after a skyfunction restart. + return this; + } + // We null out `delegate` before blocking for the fresh env so that the old one becomes + // eligible for GC. + delegate = null; + delegate = state.signalForFreshEnv(); + delegate.getValuesAndExceptions(depKeys); + return this; + } + + @Nullable + @Override + public SkyValue getOrThrow( + SkyKey skyKey, Class e1, Class e2, Class e3) throws E1, E2, E3 { + return delegate.getLookupHandleForPreviouslyRequestedDeps().getOrThrow(skyKey, e1, e2, e3); + } + + @Override + public boolean queryDep(SkyKey key, QueryDepCallback resultCallback) { + return delegate.getLookupHandleForPreviouslyRequestedDeps().queryDep(key, resultCallback); + } + + @Nullable + @Override + public SkyValue getValue(SkyKey depKey) throws InterruptedException { + return getValuesAndExceptions(ImmutableList.of(depKey)).get(depKey); + } + + @Nullable + @Override + public SkyValue getValueOrThrow(SkyKey depKey, Class e1) + throws E1, InterruptedException { + return getValuesAndExceptions(ImmutableList.of(depKey)).getOrThrow(depKey, e1); + } + + @Nullable + @Override + public SkyValue getValueOrThrow( + SkyKey depKey, Class e1, Class e2) throws E1, E2, InterruptedException { + return getValuesAndExceptions(ImmutableList.of(depKey)).getOrThrow(depKey, e1, e2); + } + + @Nullable + @Override + public + SkyValue getValueOrThrow(SkyKey depKey, Class e1, Class e2, Class e3) + throws E1, E2, E3, InterruptedException { + return getValuesAndExceptions(ImmutableList.of(depKey)).getOrThrow(depKey, e1, e2, e3); + } + + @Nullable + @Override + public + SkyValue getValueOrThrow( + SkyKey depKey, Class e1, Class e2, Class e3, Class e4) + throws E1, E2, E3, E4, InterruptedException { + SkyValue value = delegate.getValueOrThrow(depKey, e1, e2, e3, e4); + if (value != null) { + return value; + } + // We null out `delegate` before blocking for the fresh env so that the old one becomes + // eligible for GC. + delegate = null; + delegate = state.signalForFreshEnv(); + return delegate.getValueOrThrow(depKey, e1, e2, e3, e4); + } + + @Override + public ExtendedEventHandler getListener() { + // Do NOT just return `delegate.getListener()` here! That would cause anyone holding onto the + // returned listener to potentially post events to a stale listener. + return this; + } + + @Override + public void post(Postable obj) { + delegate.getListener().post(obj); + } + + @Override + public void handle(Event event) { + delegate.getListener().handle(event); + } + + @Override + public void registerDependencies(Iterable keys) { + delegate.registerDependencies(keys); + } + + @Override + public boolean inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors() { + return delegate.inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors(); + } + + @Override + public void dependOnFuture(ListenableFuture future) { + delegate.dependOnFuture(future); + } + + @Override + public boolean restartPermitted() { + return delegate.restartPermitted(); + } + + @Override + public SkyframeLookupResult getLookupHandleForPreviouslyRequestedDeps() { + return delegate.getLookupHandleForPreviouslyRequestedDeps(); + } + + @Override + public T getState(Supplier stateSupplier) { + return delegate.getState(stateSupplier); + } + + @Nullable + @Override + public Version getMaxTransitiveSourceVersionSoFar() { + return delegate.getMaxTransitiveSourceVersionSoFar(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java index 140697445739bb..fc3a7e146c128b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java @@ -16,6 +16,7 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -23,7 +24,9 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.bazel.repository.RepositoryResolvedEvent; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; +import com.google.devtools.build.lib.bazel.repository.starlark.RepoFetchingSkyKeyComputeState.Signal; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.BazelStarlarkContext; import com.google.devtools.build.lib.packages.Rule; @@ -52,6 +55,8 @@ import java.io.IOException; import java.util.Map; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Mutability; @@ -61,11 +66,12 @@ import net.starlark.java.eval.StarlarkThread; /** A repository function to delegate work done by Starlark remote repositories. */ -public class StarlarkRepositoryFunction extends RepositoryFunction { +public final class StarlarkRepositoryFunction extends RepositoryFunction { static final String SEMANTICS = "STARLARK_SEMANTICS"; private final DownloadManager downloadManager; private double timeoutScaling = 1.0; + @Nullable private ExecutorService workerExecutorService = null; @Nullable private ProcessWrapper processWrapper = null; @Nullable private RepositoryRemoteExecutor repositoryRemoteExecutor; @Nullable private SyscallCache syscallCache; @@ -86,6 +92,10 @@ public void setSyscallCache(SyscallCache syscallCache) { this.syscallCache = checkNotNull(syscallCache); } + public void setWorkerExecutorService(@Nullable ExecutorService workerExecutorService) { + this.workerExecutorService = workerExecutorService; + } + static String describeSemantics(StarlarkSemantics semantics) { // Here we use the hash code provided by AutoValue. This is unique, as long // as the number of bits in the StarlarkSemantics is small enough. We will have to @@ -105,6 +115,24 @@ protected boolean verifySemanticsMarkerData(Map markerData, Envi return describeSemantics(starlarkSemantics).equals(markerData.get(SEMANTICS)); } + @Override + protected void setupRepoRootBeforeFetching(Path repoRoot) throws RepositoryFunctionException { + // DON'T delete the repo root here if we're using a worker thread, since when this SkyFunction + // restarts, fetching is still happening inside the worker thread. + if (workerExecutorService == null) { + setupRepoRoot(repoRoot); + } + } + + @Override + public void reportSkyframeRestart(Environment env, RepositoryName repoName) { + // DON'T report a "restarting." event if we're using a worker thread, since the actual fetch + // function run by the worker thread never restarts. + if (workerExecutorService == null) { + super.reportSkyframeRestart(env, repoName); + } + } + @Nullable @Override public RepositoryDirectoryValue.Builder fetch( @@ -115,6 +143,70 @@ public RepositoryDirectoryValue.Builder fetch( Map markerData, SkyKey key) throws RepositoryFunctionException, InterruptedException { + if (workerExecutorService == null) { + return fetchInternal(rule, outputDirectory, directories, env, markerData, key); + } + var state = env.getState(RepoFetchingSkyKeyComputeState::new); + var workerFuture = state.workerFuture; + if (workerFuture == null) { + // No worker is running yet, which means we're just starting to fetch this repo. Start with a + // clean slate, and create the worker. + setupRepoRoot(outputDirectory); + Environment workerEnv = new RepoFetchingWorkerSkyFunctionEnvironment(state, env); + workerFuture = + workerExecutorService.submit( + () -> { + try { + return fetchInternal( + rule, outputDirectory, directories, workerEnv, markerData, key); + } finally { + state.signalQueue.put(Signal.DONE); + } + }); + state.workerFuture = workerFuture; + } else { + // A worker is already running. This can only mean one thing -- we just had a Skyframe + // restart, and need to send over a fresh Environment. + state.delegateEnvQueue.put(env); + } + switch (state.signalQueue.take()) { + case RESTART: + return null; + case DONE: + try { + return workerFuture.get(); + } catch (ExecutionException e) { + Throwables.throwIfInstanceOf(e.getCause(), RepositoryFunctionException.class); + Throwables.throwIfUnchecked(e.getCause()); + throw new IllegalStateException( + "unexpected exception type: " + e.getClass(), e.getCause()); + } finally { + // Make sure we interrupt the worker thread if work on the Skyframe thread were cut short + // for any reason. + state.close(); + try { + // Synchronously wait for the worker thread to finish any remaining work. + workerFuture.get(); + } catch (ExecutionException e) { + // When this happens, we either already dealt with the exception (see `catch` clause + // above), or we're in the middle of propagating an InterruptedException in which case + // we don't care about the result of execution anyway. + } + } + } + // TODO(wyv): use a switch expression above instead and remove this. + throw new IllegalStateException(); + } + + @Nullable + private RepositoryDirectoryValue.Builder fetchInternal( + Rule rule, + Path outputDirectory, + BlazeDirectories directories, + Environment env, + Map markerData, + SkyKey key) + throws RepositoryFunctionException, InterruptedException { String defInfo = RepositoryResolvedEvent.getRuleDefinitionInformation(rule); env.getListener().post(new StarlarkRepositoryDefinitionLocationEvent(rule.getName(), defInfo)); diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index b52373016cd737..aa92bfd1a6db34 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -223,15 +223,6 @@ private static FileValue getWorkspaceFile(RootedPath directory, Environment env) return value; } - private void setupRepositoryRoot(Path repoRoot) throws RepositoryFunctionException { - try { - repoRoot.deleteTree(); - Preconditions.checkNotNull(repoRoot.getParentDirectory()).createDirectoryAndParents(); - } catch (IOException e) { - throw new RepositoryFunctionException(e, Transience.TRANSIENT); - } - } - @Nullable @Override public SkyValue compute(SkyKey skyKey, Environment env) @@ -401,7 +392,7 @@ private RepositoryDirectoryValue.Builder fetchRepository( Rule rule) throws InterruptedException, RepositoryFunctionException { - setupRepositoryRoot(repoRoot); + handler.setupRepoRootBeforeFetching(repoRoot); RepositoryName repoName = (RepositoryName) skyKey.argument(); env.getListener().post(RepositoryFetchProgress.ongoing(repoName, "starting")); @@ -425,7 +416,7 @@ private RepositoryDirectoryValue.Builder fetchRepository( } if (env.valuesMissing()) { - env.getListener().post(RepositoryFetchProgress.ongoing(repoName, "Restarting.")); + handler.reportSkyframeRestart(env, repoName); return null; } env.getListener().post(RepositoryFetchProgress.finished(repoName)); @@ -463,7 +454,7 @@ public String extractTag(SkyKey skyKey) { private RepositoryDirectoryValue setupOverride( PathFragment sourcePath, Environment env, Path repoRoot, String pathAttr) throws RepositoryFunctionException, InterruptedException { - setupRepositoryRoot(repoRoot); + RepositoryFunction.setupRepoRoot(repoRoot); RepositoryDirectoryValue.Builder directoryValue = symlinkRepoRoot( directories, diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index 55084b91d90d3a..1854bc12159c9d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.repository.ExternalPackageException; +import com.google.devtools.build.lib.repository.RepositoryFetchProgress; import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; import com.google.devtools.build.lib.skyframe.AlreadyReportedException; import com.google.devtools.build.lib.skyframe.PackageLookupFunction; @@ -133,6 +134,23 @@ public AlreadyReportedRepositoryAccessException(Exception e) { } } + public static void setupRepoRoot(Path repoRoot) throws RepositoryFunctionException { + try { + repoRoot.deleteTree(); + Preconditions.checkNotNull(repoRoot.getParentDirectory()).createDirectoryAndParents(); + } catch (IOException e) { + throw new RepositoryFunctionException(e, Transience.TRANSIENT); + } + } + + protected void setupRepoRootBeforeFetching(Path repoRoot) throws RepositoryFunctionException { + setupRepoRoot(repoRoot); + } + + public void reportSkyframeRestart(Environment env, RepositoryName repoName) { + env.getListener().post(RepositoryFetchProgress.ongoing(repoName, "Restarting.")); + } + /** * Fetch the remote repository represented by the given rule. * diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java index cdd262f12e1c85..d39bc3a89f5c19 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java @@ -396,6 +396,8 @@ interface SkyKeyComputeState extends AutoCloseable { * being held on to is approaching starvation, we currently don't do anything to alleviate * that pressure. So think *hard* before you start doing that! * + *

Implementations MUST be idempotent. + * *

Note also that this method should not perform any heavy work (especially blocking * operations). */ diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java index ea5298affa07ca..2110503065f065 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java @@ -102,7 +102,8 @@ static void validateExceptionType(Class exceptionClass) } } - static + public static < + E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception> void throwIfInstanceOf( @Nullable Exception e, @Nullable Class exceptionClass1, diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index cbf8f186311b2d..b2f450387cf766 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -2358,4 +2358,32 @@ EOF bazel build --repository_disable_download //:it || fail "Failed to build" } +function test_no_restarts_fetching_with_worker_thread() { + setup_starlark_repository + + echo foo > file1 + echo bar > file2 + + cat >test.bzl <& $TEST_log \ + || fail "Expected build to succeed" + expect_log_n "hello world!" 3 + + # platform worker thread, never restarts + bazel shutdown + bazel build @foo//:bar --experimental_worker_for_repo_fetching=platform >& $TEST_log \ + || fail "Expected build to succeed" + expect_log_n "hello world!" 1 +} + run_suite "local repository tests"