Skip to content

Commit

Permalink
Remote: Postpone the block waiting in afterCommand to `BlockWaiting…
Browse files Browse the repository at this point in the history
…Module` (bazelbuild#14833)

When implementing async upload, we introduced a block waiting behaviour in `RemoteModule#afterCommand` so that uploads happened in the background can be waited before the whole build complete.

However, there are other block waiting code in other module's `afterCommand` method (e.g. BES module). Block waiting in remote module will prevent other modules' `afterCommand` from executing until it's completed. This causes issues like bazelbuild#14576.

This PR adds a new module `BlockWaitingModule` whose sole purpose is to accept tasks submitted by other modules in `afterCommand` and block waiting all the tasks in its own `afterCommand` method. So those tasks can be executed in parallel.

This PR only updates RemoteModule's `afterCommand` method to submit block waiting task. Other modules should be updated to use `BlockWaitingModule` as well but that's beyond the scope this this PR.

This PR along with 73a76a8 fix bazelbuild#14576.

Closes bazelbuild#14618.

PiperOrigin-RevId: 424295121
(cherry picked from commit 621649d)

Co-authored-by: Chi Wang <chiwang@google.com>
  • Loading branch information
brentleyjones and coeuvre authored Feb 16, 2022
1 parent 344e8f8 commit 0c74741
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ public final class Bazel {
com.google.devtools.build.lib.metrics.PostGCMemoryUseRecorder.GcAfterBuildModule.class,
com.google.devtools.build.lib.packages.metrics.PackageMetricsModule.class,
com.google.devtools.build.lib.metrics.MetricsModule.class,
BazelBuiltinCommandModule.class);
BazelBuiltinCommandModule.class,
// This module needs to be registered after any module submitting tasks with its {@code
// submit} method.
com.google.devtools.build.lib.runtime.BlockWaitingModule.class);

public static void main(String[] args) {
BlazeVersionInfo.setBuildInfo(tryGetBuildInfo());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.BlockWaitingModule;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.vfs.Path;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -211,7 +212,9 @@ void setFilesToDownload(ImmutableSet<ActionInput> topLevelOutputs) {

public void afterCommand() {
if (remoteExecutionService != null) {
remoteExecutionService.shutdown();
BlockWaitingModule blockWaitingModule =
checkNotNull(env.getRuntime().getBlazeModule(BlockWaitingModule.class));
blockWaitingModule.submit(() -> remoteExecutionService.shutdown());
} else {
if (remoteCache != null) {
remoteCache.release();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright 2022 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.runtime;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static java.util.concurrent.TimeUnit.SECONDS;

import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.util.AbruptExitException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import javax.annotation.Nullable;

/** A {@link BlazeModule} that waits for submitted tasks to terminate after every command. */
public class BlockWaitingModule extends BlazeModule {
@Nullable private ExecutorService executorService;

@Override
public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
checkState(executorService == null, "executorService must be null");

executorService =
Executors.newCachedThreadPool(
new ThreadFactoryBuilder().setNameFormat("block-waiting-%d").build());
}

@SuppressWarnings("FutureReturnValueIgnored")
public void submit(Runnable task) {
checkNotNull(executorService, "executorService must not be null");

executorService.submit(task);
}

@Override
public void afterCommand() throws AbruptExitException {
checkNotNull(executorService, "executorService must not be null");

executorService.shutdown();
try {
executorService.awaitTermination(Long.MAX_VALUE, SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}

executorService = null;
}
}

0 comments on commit 0c74741

Please sign in to comment.