Automated rollback of commit 353000ebc512e4f1654d123713b003011751e07e.
*** Reason for rollback ***
b/312694497
*** Original change description ***
Make the BesUploadMode per transport instead of per invocation.
As a result, `--build_event_[text|json|binary]_file_upload_mode` are introduced to set the upload mode for build event file respectively. The default value for them is `wait_for_upload_complete`. The implicit requirement on `--bes_upload_mode=wait_for_upload_complete` is also removed.
Alternate to https://github.com/bazelbuild/bazel/pull/19322.
PiperOrigin-RevId: 586076379
Change-Id: I020c969497fcf72fe77754a5438ff5353ab606cb
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
index 4304841..56991df 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.buildeventservice;
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.annotations.VisibleForTesting;
@@ -81,11 +80,9 @@
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Duration;
-import java.util.AbstractMap.SimpleEntry;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@@ -116,9 +113,6 @@
private boolean isRunsPerTestOverTheLimit;
private BuildEventArtifactUploaderFactory uploaderFactoryToCleanup;
- private BuildEventOutputStreamFactory buildEventOutputStreamFactory =
- file -> new BufferedOutputStream(Files.newOutputStream(Paths.get(file)));
-
/**
* Holds the close futures for the upload of each transport with timeouts attached to them using
* {@link #constructCloseFuturesMapWithTimeouts(ImmutableMap)} obtained from {@link
@@ -139,6 +133,8 @@
private ImmutableMap<BuildEventTransport, ListenableFuture<Void>>
halfCloseFuturesWithTimeoutsMap = ImmutableMap.of();
+ private BesUploadMode previousUploadMode = BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE;
+
// TODO(lpino): Use Optional instead of @Nullable for the members below.
@Nullable private OutErr outErr;
@Nullable private ImmutableSet<BuildEventTransport> bepTransports;
@@ -202,17 +198,6 @@
resetPendingUploads();
}
- private void removeFromPendingUploads(
- Map<BuildEventTransport, ListenableFuture<Void>> transportFutures) {
- transportFutures
- .values()
- .forEach(closeFuture -> closeFuture.cancel(/* mayInterruptIfRunning= */ true));
- closeFuturesWithTimeoutsMap =
- closeFuturesWithTimeoutsMap.entrySet().stream()
- .filter(entry -> !transportFutures.containsKey(entry.getKey()))
- .collect(toImmutableMap(Entry::getKey, Entry::getValue));
- }
-
private static boolean isTimeoutException(ExecutionException e) {
return e.getCause() instanceof TimeoutException;
}
@@ -234,31 +219,20 @@
return;
}
- ImmutableMap<BuildEventTransport, ListenableFuture<Void>> waitingFutureMap =
- closeFuturesWithTimeoutsMap.entrySet().stream()
- .map(
- entry -> {
- var transport = entry.getKey();
- var closeFuture = entry.getValue();
- ListenableFuture<Void> future = closeFuture;
- if (transport.getBesUploadMode() == BesUploadMode.FULLY_ASYNC) {
- future =
- isShutdown ? closeFuture : halfCloseFuturesWithTimeoutsMap.get(transport);
- if (future == null) {
- future = closeFuture;
- }
- }
- return new SimpleEntry<>(transport, future);
- })
- .collect(toImmutableMap(Entry::getKey, Entry::getValue));
- ImmutableMap<BuildEventTransport, ListenableFuture<Void>> cancelCloseFutures =
- closeFuturesWithTimeoutsMap.entrySet().stream()
- .filter(
- entry -> {
- var transport = entry.getKey();
- return transport.getBesUploadMode() != BesUploadMode.FULLY_ASYNC;
- })
- .collect(toImmutableMap(Entry::getKey, Entry::getValue));
+ ImmutableMap<BuildEventTransport, ListenableFuture<Void>> waitingFutureMap = null;
+ boolean cancelCloseFutures = true;
+ switch (previousUploadMode) {
+ case FULLY_ASYNC:
+ waitingFutureMap =
+ isShutdown ? closeFuturesWithTimeoutsMap : halfCloseFuturesWithTimeoutsMap;
+ cancelCloseFutures = false;
+ break;
+ case WAIT_FOR_UPLOAD_COMPLETE:
+ case NOWAIT_FOR_UPLOAD_COMPLETE:
+ waitingFutureMap = closeFuturesWithTimeoutsMap;
+ cancelCloseFutures = true;
+ break;
+ }
Stopwatch stopwatch = Stopwatch.createStarted();
try {
@@ -287,7 +261,7 @@
waitedMillis / 1000, waitedMillis % 1000);
reporter.handle(Event.warn(msg));
logger.atWarning().withCause(exception).log("%s", msg);
- cancelCloseFutures = closeFuturesWithTimeoutsMap;
+ cancelCloseFutures = true;
} catch (ExecutionException e) {
String msg;
// Futures.withTimeout wraps the TimeoutException in an ExecutionException when the future
@@ -307,12 +281,13 @@
}
reporter.handle(Event.warn(msg));
logger.atWarning().withCause(e).log("%s", msg);
- cancelCloseFutures = closeFuturesWithTimeoutsMap;
+ cancelCloseFutures = true;
} finally {
- cancelCloseFutures
- .values()
- .forEach(closeFuture -> closeFuture.cancel(/* mayInterruptIfRunning= */ true));
- resetPendingUploads();
+ if (cancelCloseFutures) {
+ cancelAndResetPendingUploads();
+ } else {
+ resetPendingUploads();
+ }
}
}
@@ -506,7 +481,8 @@
}
private void waitForBuildEventTransportsToClose(
- Map<BuildEventTransport, ListenableFuture<Void>> transportFutures)
+ Map<BuildEventTransport, ListenableFuture<Void>> transportFutures,
+ boolean besUploadModeIsSynchronous)
throws AbruptExitException {
final ScheduledExecutorService executor =
Executors.newSingleThreadScheduledExecutor(
@@ -543,7 +519,9 @@
e.getCause().getMessage()),
e);
} finally {
- removeFromPendingUploads(transportFutures);
+ if (besUploadModeIsSynchronous) {
+ cancelAndResetPendingUploads();
+ }
executor.shutdown();
}
}
@@ -601,16 +579,17 @@
}
private void closeBepTransports() throws AbruptExitException {
+ previousUploadMode = besOptions.besUploadMode;
closeFuturesWithTimeoutsMap =
constructCloseFuturesMapWithTimeouts(streamer.getCloseFuturesMap());
halfCloseFuturesWithTimeoutsMap =
constructCloseFuturesMapWithTimeouts(streamer.getHalfClosedMap());
+ boolean besUploadModeIsSynchronous =
+ besOptions.besUploadMode == BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE;
Map<BuildEventTransport, ListenableFuture<Void>> blockingTransportFutures = new HashMap<>();
for (Map.Entry<BuildEventTransport, ListenableFuture<Void>> entry :
closeFuturesWithTimeoutsMap.entrySet()) {
BuildEventTransport bepTransport = entry.getKey();
- boolean besUploadModeIsSynchronous =
- bepTransport.getBesUploadMode() == BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE;
if (!bepTransport.mayBeSlow() || besUploadModeIsSynchronous) {
blockingTransportFutures.put(bepTransport, entry.getValue());
} else {
@@ -620,7 +599,7 @@
}
}
if (!blockingTransportFutures.isEmpty()) {
- waitForBuildEventTransportsToClose(blockingTransportFutures);
+ waitForBuildEventTransportsToClose(blockingTransportFutures, besUploadModeIsSynchronous);
}
}
@@ -785,18 +764,16 @@
if (!Strings.isNullOrEmpty(besStreamOptions.buildEventTextFile)) {
try {
BufferedOutputStream bepTextOutputStream =
- buildEventOutputStreamFactory.create(besStreamOptions.buildEventTextFile);
+ new BufferedOutputStream(
+ Files.newOutputStream(Paths.get(besStreamOptions.buildEventTextFile)));
+
BuildEventArtifactUploader localFileUploader =
besStreamOptions.buildEventTextFilePathConversion
? uploaderSupplier.get()
: new LocalFilesArtifactUploader();
bepTransportsBuilder.add(
new TextFormatFileTransport(
- bepTextOutputStream,
- bepOptions,
- localFileUploader,
- artifactGroupNamer,
- besStreamOptions.buildEventTextFileUploadMode));
+ bepTextOutputStream, bepOptions, localFileUploader, artifactGroupNamer));
} catch (IOException exception) {
// TODO(b/125216340): Consider making this a warning instead of an error once the
// associated bug has been resolved.
@@ -814,18 +791,16 @@
if (!Strings.isNullOrEmpty(besStreamOptions.buildEventBinaryFile)) {
try {
BufferedOutputStream bepBinaryOutputStream =
- buildEventOutputStreamFactory.create(besStreamOptions.buildEventBinaryFile);
+ new BufferedOutputStream(
+ Files.newOutputStream(Paths.get(besStreamOptions.buildEventBinaryFile)));
+
BuildEventArtifactUploader localFileUploader =
besStreamOptions.buildEventBinaryFilePathConversion
? uploaderSupplier.get()
: new LocalFilesArtifactUploader();
bepTransportsBuilder.add(
new BinaryFormatFileTransport(
- bepBinaryOutputStream,
- bepOptions,
- localFileUploader,
- artifactGroupNamer,
- besStreamOptions.buildEventBinaryFileUploadMode));
+ bepBinaryOutputStream, bepOptions, localFileUploader, artifactGroupNamer));
} catch (IOException exception) {
// TODO(b/125216340): Consider making this a warning instead of an error once the
// associated bug has been resolved.
@@ -843,7 +818,8 @@
if (!Strings.isNullOrEmpty(besStreamOptions.buildEventJsonFile)) {
try {
BufferedOutputStream bepJsonOutputStream =
- buildEventOutputStreamFactory.create(besStreamOptions.buildEventJsonFile);
+ new BufferedOutputStream(
+ Files.newOutputStream(Paths.get(besStreamOptions.buildEventJsonFile)));
BuildEventArtifactUploader localFileUploader =
besStreamOptions.buildEventJsonFilePathConversion
? uploaderSupplier.get()
@@ -854,8 +830,7 @@
bepOptions,
localFileUploader,
artifactGroupNamer,
- makeJsonTypeRegistry(),
- besStreamOptions.buildEventJsonFileUploadMode));
+ makeJsonTypeRegistry()));
} catch (IOException exception) {
// TODO(b/125216340): Consider making this a warning instead of an error once the
// associated bug has been resolved.
@@ -902,11 +877,6 @@
protected abstract Set<String> allowedCommands(OptionsT besOptions);
- @VisibleForTesting
- void setBuildEventOutputStreamFactory(BuildEventOutputStreamFactory factory) {
- this.buildEventOutputStreamFactory = factory;
- }
-
protected ImmutableSet<String> getBesKeywords(
OptionsT besOptions, @Nullable OptionsParsingResult startupOptionsProvider) {
List<String> userKeywords = besOptions.besKeywords;
@@ -963,9 +933,4 @@
throw exception;
}
}
-
- @VisibleForTesting
- interface BuildEventOutputStreamFactory {
- BufferedOutputStream create(String file) throws IOException;
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
index 7511621..e47edc7 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
@@ -19,7 +19,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.eventbus.EventBus;
import com.google.common.util.concurrent.ListenableFuture;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
@@ -38,7 +37,6 @@
public class BuildEventServiceTransport implements BuildEventTransport {
private final BuildEventServiceUploader besUploader;
private final Duration besTimeout;
- private final BesUploadMode besUploadMode;
private BuildEventServiceTransport(
BuildEventServiceClient besClient,
@@ -51,8 +49,7 @@
EventBus eventBus,
Duration closeTimeout,
Sleeper sleeper,
- Timestamp commandStartTime,
- BesUploadMode besUploadMode) {
+ Timestamp commandStartTime) {
this.besTimeout = closeTimeout;
this.besUploader =
new BuildEventServiceUploader.Builder()
@@ -67,7 +64,6 @@
.eventBus(eventBus)
.commandStartTime(commandStartTime)
.build();
- this.besUploadMode = besUploadMode;
}
@Override
@@ -96,11 +92,6 @@
}
@Override
- public BesUploadMode getBesUploadMode() {
- return besUploadMode;
- }
-
- @Override
public void sendBuildEvent(BuildEvent event) {
besUploader.enqueueEvent(event);
}
@@ -197,8 +188,7 @@
checkNotNull(eventBus),
(besOptions.besTimeout != null) ? besOptions.besTimeout : Duration.ZERO,
sleeper != null ? sleeper : new JavaSleeper(),
- checkNotNull(commandStartTime),
- besOptions.besUploadMode);
+ checkNotNull(commandStartTime));
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD
index be3e787..8562920 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD
@@ -21,7 +21,6 @@
deps = [
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
- "//src/main/java/com/google/devtools/build/lib/buildeventservice:buildeventservice-options",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java
index 945afed..5b2d2b3 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventTransport.java
@@ -13,8 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.buildeventstream;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.ListenableFuture;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import java.time.Duration;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
@@ -24,12 +24,14 @@
*
* <p>All implementations need to be thread-safe. All methods are expected to return quickly.
*
- * <p>Notice that this interface does not provide any error handling API. A transport may choose to
- * log interesting errors to the command line and/or abort the whole build.
+ * <p>Notice that this interface does not provide any error handling API. A transport may choose
+ * to log interesting errors to the command line and/or abort the whole build.
*/
@ThreadSafe
public interface BuildEventTransport {
- /** The name of this transport as can be displayed to a user. */
+ /**
+ * The name of this transport as can be displayed to a user.
+ */
String name();
/**
@@ -85,9 +87,7 @@
*/
boolean mayBeSlow();
- /** Returns the desired {@link BesUploadMode} for the transport. */
- BesUploadMode getBesUploadMode();
-
+ @VisibleForTesting
@Nullable
BuildEventArtifactUploader getUploader();
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
index fcab7f7..b74731e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
@@ -15,7 +15,6 @@
name = "transports",
srcs = glob(["*.java"]),
deps = [
- "//src/main/java/com/google/devtools/build/lib/buildeventservice:buildeventservice-options",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
index c335338..c3a83c0 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.buildeventstream.transports;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
@@ -35,9 +34,8 @@
BufferedOutputStream outputStream,
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
- ArtifactGroupNamer namer,
- BesUploadMode besUploadMode) {
- super(outputStream, options, uploader, namer, besUploadMode);
+ ArtifactGroupNamer namer) {
+ super(outputStream, options, uploader, namer);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventStreamOptions.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventStreamOptions.java
index 15ef420..6d38019 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventStreamOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventStreamOptions.java
@@ -14,8 +14,6 @@
package com.google.devtools.build.lib.buildeventstream.transports;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadModeConverter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
@@ -48,6 +46,7 @@
name = "build_event_binary_file",
oldName = "experimental_build_event_binary_file",
defaultValue = "",
+ implicitRequirements = {"--bes_upload_mode=wait_for_upload_complete"},
documentationCategory = OptionDocumentationCategory.LOGGING,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
help =
@@ -60,6 +59,7 @@
name = "build_event_json_file",
oldName = "experimental_build_event_json_file",
defaultValue = "",
+ implicitRequirements = {"--bes_upload_mode=wait_for_upload_complete"},
documentationCategory = OptionDocumentationCategory.LOGGING,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
help =
@@ -68,54 +68,14 @@
public String buildEventJsonFile;
@Option(
- name = "build_event_text_file_upload_mode",
- defaultValue = "wait_for_upload_complete",
- converter = BesUploadModeConverter.class,
- documentationCategory = OptionDocumentationCategory.LOGGING,
- effectTags = {OptionEffectTag.EAGERNESS_TO_EXIT},
- help =
- "Specifies whether the Build Event Service upload for --build_event_text_file should"
- + " block the build completion or should end the invocation immediately and finish"
- + " the upload in the background. Either 'wait_for_upload_complete' (default),"
- + " 'nowait_for_upload_complete', or 'fully_async'.")
- public BesUploadMode buildEventTextFileUploadMode;
-
- @Option(
- name = "build_event_binary_file_upload_mode",
- defaultValue = "wait_for_upload_complete",
- converter = BesUploadModeConverter.class,
- documentationCategory = OptionDocumentationCategory.LOGGING,
- effectTags = {OptionEffectTag.EAGERNESS_TO_EXIT},
- help =
- "Specifies whether the Build Event Service upload for --build_event_binary_file should"
- + " block the build completion or should end the invocation immediately and finish"
- + " the upload in the background. Either 'wait_for_upload_complete' (default),"
- + " 'nowait_for_upload_complete', or 'fully_async'.")
- public BesUploadMode buildEventBinaryFileUploadMode;
-
- @Option(
- name = "build_event_json_file_upload_mode",
- defaultValue = "wait_for_upload_complete",
- converter = BesUploadModeConverter.class,
- documentationCategory = OptionDocumentationCategory.LOGGING,
- effectTags = {OptionEffectTag.EAGERNESS_TO_EXIT},
- help =
- "Specifies whether the Build Event Service upload for --build_event_json_file should"
- + " block the build completion or should end the invocation immediately and finish"
- + " the upload in the background. Either 'wait_for_upload_complete' (default),"
- + " 'nowait_for_upload_complete', or 'fully_async'.")
- public BesUploadMode buildEventJsonFileUploadMode;
-
- @Option(
name = "build_event_text_file_path_conversion",
oldName = "experimental_build_event_text_file_path_conversion",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.LOGGING,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
- help =
- "Convert paths in the text file representation of the build event protocol to more "
- + "globally valid URIs whenever possible; if disabled, the file:// uri scheme will "
- + "always be used")
+ help = "Convert paths in the text file representation of the build event protocol to more "
+ + "globally valid URIs whenever possible; if disabled, the file:// uri scheme will "
+ + "always be used")
public boolean buildEventTextFilePathConversion;
@Option(
@@ -124,10 +84,9 @@
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.LOGGING,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
- help =
- "Convert paths in the binary file representation of the build event protocol to more "
- + "globally valid URIs whenever possible; if disabled, the file:// uri scheme will "
- + "always be used")
+ help = "Convert paths in the binary file representation of the build event protocol to more "
+ + "globally valid URIs whenever possible; if disabled, the file:// uri scheme will "
+ + "always be used")
public boolean buildEventBinaryFilePathConversion;
@Option(
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
index da3c2b7..7bc427d 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
@@ -24,7 +24,6 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
@@ -67,7 +66,6 @@
private final BuildEventArtifactUploader uploader;
private final SequentialWriter writer;
private final ArtifactGroupNamer namer;
- private final BesUploadMode besUploadMode;
private final ScheduledExecutorService timeoutExecutor =
MoreExecutors.listeningDecorator(
@@ -78,14 +76,12 @@
BufferedOutputStream outputStream,
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
- ArtifactGroupNamer namer,
- BesUploadMode besUploadMode) {
+ ArtifactGroupNamer namer) {
this.uploader = uploader;
this.options = options;
this.writer =
new SequentialWriter(outputStream, this::serializeEvent, uploader, timeoutExecutor);
this.namer = namer;
- this.besUploadMode = besUploadMode;
}
@ThreadSafe
@@ -174,7 +170,8 @@
// Print a more useful error message when the upload times out.
// An {@link ExecutionException} may be wrapping a {@link TimeoutException} if the
// Future was created with {@link Futures#withTimeout}.
- if (e instanceof ExecutionException && e.getCause() instanceof TimeoutException) {
+ if (e instanceof ExecutionException
+ && e.getCause() instanceof TimeoutException) {
message = "Unable to write all BEP events to file due to timeout";
} else {
message =
@@ -323,11 +320,6 @@
}
@Override
- public BesUploadMode getBesUploadMode() {
- return besUploadMode;
- }
-
- @Override
public BuildEventArtifactUploader getUploader() {
return uploader;
}
@@ -337,3 +329,4 @@
return writer.getFlushInterval();
}
}
+
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java
index b6033d8..5d7af3a 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java
@@ -18,7 +18,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.flogger.GoogleLogger;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
@@ -48,9 +47,8 @@
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
ArtifactGroupNamer namer,
- TypeRegistry typeRegistry,
- BesUploadMode besUploadMode) {
- super(outputStream, options, uploader, namer, besUploadMode);
+ TypeRegistry typeRegistry) {
+ super(outputStream, options, uploader, namer);
jsonPrinter =
JsonFormat.printer().usingTypeRegistry(typeRegistry).omittingInsignificantWhitespace();
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
index 02086a8..34e26e6 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
@@ -16,7 +16,6 @@
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
@@ -36,9 +35,8 @@
BufferedOutputStream outputStream,
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
- ArtifactGroupNamer namer,
- BesUploadMode besUploadMode) {
- super(outputStream, options, uploader, namer, besUploadMode);
+ ArtifactGroupNamer namer) {
+ super(outputStream, options, uploader, namer);
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD b/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD
index 6ec644c..0c2a557 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BUILD
@@ -74,7 +74,6 @@
"@googleapis//:google_devtools_build_v1_build_events_java_proto",
"@googleapis//:google_devtools_build_v1_publish_build_event_java_grpc",
"@googleapis//:google_devtools_build_v1_publish_build_event_java_proto",
- "@maven//:com_google_testparameterinjector_test_parameter_injector",
"@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto",
],
)
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
index 1115c36..dfea5f7 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
@@ -37,7 +37,6 @@
import com.google.devtools.build.lib.bugreport.Crash;
import com.google.devtools.build.lib.bugreport.CrashContext;
import com.google.devtools.build.lib.buildeventservice.BazelBuildEventServiceModule.BackendConfig;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceModule.BuildEventOutputStreamFactory;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Aborted;
@@ -75,8 +74,6 @@
import com.google.devtools.build.v1.PublishLifecycleEventRequest;
import com.google.devtools.build.v1.StreamId;
import com.google.protobuf.Empty;
-import com.google.testing.junit.testparameterinjector.TestParameter;
-import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import io.grpc.ManagedChannel;
import io.grpc.Metadata;
import io.grpc.Server;
@@ -87,15 +84,11 @@
import io.grpc.inprocess.InProcessServerBuilder;
import io.grpc.stub.StreamObserver;
import io.grpc.util.MutableHandlerRegistry;
-import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.OutputStream;
import java.lang.Thread.UncaughtExceptionHandler;
-import java.nio.file.Files;
-import java.nio.file.Paths;
import java.time.Duration;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -109,7 +102,6 @@
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
import org.junit.After;
@@ -119,9 +111,10 @@
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/** Tests for {@link BazelBuildEventServiceModule}. */
-@RunWith(TestParameterInjector.class)
+@RunWith(JUnit4.class)
public final class BazelBuildEventServiceModuleTest extends BuildIntegrationTestCase {
private static final Duration WAIT_FOR_LAST_INVOCATION_TIMEOUT = Duration.ofSeconds(2);
@@ -137,8 +130,6 @@
@Rule public TemporaryFolder tmpFolder = new TemporaryFolder();
- @Nullable private BuildEventOutputStreamFactory buildEventOutputStreamFactory;
-
@Override
protected BlazeModule getConnectivityModule() {
return connectivityModule;
@@ -176,9 +167,6 @@
private void runBuildWithOptions(String... options) throws Exception {
addOptions(options);
besModule = runtimeWrapper.getRuntime().getBlazeModule(BazelBuildEventServiceModule.class);
- if (buildEventOutputStreamFactory != null) {
- besModule.setBuildEventOutputStreamFactory(buildEventOutputStreamFactory);
- }
runtimeWrapper.newCommand();
buildTarget();
}
@@ -455,65 +443,6 @@
events.assertNoWarningsOrErrors();
}
- enum BuildEventFile {
- TEXT,
- JSON,
- BINARY;
-
- String getBuildEventFileFlag(String file) {
- switch (this) {
- case TEXT:
- return "--build_event_text_file=" + file;
- case JSON:
- return "--build_event_json_file=" + file;
- case BINARY:
- return "--build_event_binary_file=" + file;
- }
- throw new IllegalStateException();
- }
-
- String getBuildEventFileUploadModeFlag(String mode) {
- switch (this) {
- case TEXT:
- return "--build_event_text_file_upload_mode=" + mode;
- case JSON:
- return "--build_event_json_file_upload_mode=" + mode;
- case BINARY:
- return "--build_event_binary_file_upload_mode=" + mode;
- }
- throw new IllegalStateException();
- }
- }
-
- @Test
- public void testAfterCommand_buildEventFile_waitForUploadComplete(
- @TestParameter BuildEventFile buildEventFile) throws Exception {
- AtomicReference<DelayingCloseBufferedOutputStream> outRef = new AtomicReference<>(null);
- buildEventOutputStreamFactory =
- (file) -> {
- var out =
- new DelayingCloseBufferedOutputStream(
- Files.newOutputStream(Paths.get(file)), Duration.ofSeconds(1));
- outRef.set(out);
- return out;
- };
- buildEventService.setDelayBeforeClosingStream(Duration.ofSeconds(10));
- var file = tmpFolder.newFile();
-
- runBuildWithOptions(
- "--bes_backend=inprocess",
- "--bes_upload_mode=FULLY_ASYNC",
- "--bes_timeout=1s",
- buildEventFile.getBuildEventFileFlag(file.getAbsolutePath()),
- buildEventFile.getBuildEventFileUploadModeFlag("wait_for_upload_complete"));
- afterBuildCommand();
-
- assertThat(outRef.get().isClosed()).isTrue();
- // Expect Bazel doesn't wait for uploading to bes_backend, otherwise there will be a timeout
- // error.
- events.assertNoWarningsOrErrors();
- }
-
@Test
public void testAfterCommand_fullyAsync_slowHalfCloseIgnored() throws Exception {
buildEventService.setDelayBeforeHalfClosingStream(Duration.ofSeconds(10));
@@ -1126,26 +1055,4 @@
new StatusRuntimeException(Status.DATA_LOSS.withDescription(errorMessage)));
}
}
-
- private static final class DelayingCloseBufferedOutputStream extends BufferedOutputStream {
- private final Duration delay;
- private final AtomicBoolean closed = new AtomicBoolean(false);
-
- DelayingCloseBufferedOutputStream(OutputStream out, Duration delay) {
- super(out);
- this.delay = delay;
- this.out = out;
- }
-
- @Override
- public void close() throws IOException {
- Uninterruptibles.sleepUninterruptibly(delay);
- super.close();
- closed.set(true);
- }
-
- public boolean isClosed() {
- return closed.get();
- }
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
index 1272740..3caaa10 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
@@ -19,7 +19,6 @@
test_class = "com.google.devtools.build.lib.AllTests",
runtime_deps = ["//src/test/java/com/google/devtools/build/lib:test_runner"],
deps = [
- "//src/main/java/com/google/devtools/build/lib/buildeventservice:buildeventservice-options",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/transports",
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
index 916d563..527bcce 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
@@ -25,7 +25,6 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
@@ -108,8 +107,7 @@
outputStream,
defaultOpts,
new LocalFilesArtifactUploader(),
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ artifactGroupNamer);
transport.sendBuildEvent(buildEvent);
BuildEventStreamProtos.BuildEvent progress =
@@ -159,12 +157,7 @@
BufferedOutputStream outputStream =
new BufferedOutputStream(Files.newOutputStream(Paths.get(output.getAbsolutePath())));
BinaryFormatFileTransport transport =
- new BinaryFormatFileTransport(
- outputStream,
- defaultOpts,
- uploader,
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ new BinaryFormatFileTransport(outputStream, defaultOpts, uploader, artifactGroupNamer);
transport.sendBuildEvent(event1);
ExecutionException expected =
@@ -196,8 +189,7 @@
outputStream,
defaultOpts,
new LocalFilesArtifactUploader(),
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ artifactGroupNamer);
transport.close().get();
@@ -228,8 +220,7 @@
outputStream,
defaultOpts,
new LocalFilesArtifactUploader(),
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ artifactGroupNamer);
transport.sendBuildEvent(buildEvent);
Future<Void> closeFuture = transport.close();
@@ -276,12 +267,7 @@
BufferedOutputStream outputStream =
new BufferedOutputStream(Files.newOutputStream(Paths.get(output.getAbsolutePath())));
BinaryFormatFileTransport transport =
- new BinaryFormatFileTransport(
- outputStream,
- defaultOpts,
- uploader,
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ new BinaryFormatFileTransport(outputStream, defaultOpts, uploader, artifactGroupNamer);
transport.sendBuildEvent(event1);
transport.sendBuildEvent(event2);
transport.close().get();
@@ -321,12 +307,7 @@
BufferedOutputStream outputStream =
new BufferedOutputStream(Files.newOutputStream(Paths.get(output.getAbsolutePath())));
BinaryFormatFileTransport transport =
- new BinaryFormatFileTransport(
- outputStream,
- defaultOpts,
- uploader,
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ new BinaryFormatFileTransport(outputStream, defaultOpts, uploader, artifactGroupNamer);
transport.sendBuildEvent(event1);
transport.close().get();
@@ -364,12 +345,7 @@
BufferedOutputStream outputStream =
new BufferedOutputStream(Files.newOutputStream(Paths.get(output.getAbsolutePath())));
BinaryFormatFileTransport transport =
- new BinaryFormatFileTransport(
- outputStream,
- defaultOpts,
- uploader,
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ new BinaryFormatFileTransport(outputStream, defaultOpts, uploader, artifactGroupNamer);
transport.sendBuildEvent(event);
ListenableFuture<Void> closeFuture = transport.close();
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
index 3217f89..fe9dd01 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
@@ -18,7 +18,6 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.Mockito.when;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
@@ -91,8 +90,7 @@
defaultOpts,
new LocalFilesArtifactUploader(),
artifactGroupNamer,
- SPAWN_EXEC_TYPE_REGISTRY,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ SPAWN_EXEC_TYPE_REGISTRY);
}
@Test
@@ -173,8 +171,7 @@
defaultOpts,
new LocalFilesArtifactUploader(),
artifactGroupNamer,
- SPAWN_EXEC_TYPE_REGISTRY,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ SPAWN_EXEC_TYPE_REGISTRY);
BuildEventStreamProtos.BuildEvent started =
BuildEventStreamProtos.BuildEvent.newBuilder()
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java
index 146076f..a307a12 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java
@@ -19,7 +19,6 @@
import com.google.common.base.Joiner;
import com.google.common.io.Files;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
@@ -89,8 +88,7 @@
outputStream,
defaultOpts,
new LocalFilesArtifactUploader(),
- artifactGroupNamer,
- BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE);
+ artifactGroupNamer);
transport.sendBuildEvent(buildEvent);
BuildEventStreamProtos.BuildEvent progress =
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BUILD b/src/test/java/com/google/devtools/build/lib/runtime/BUILD
index fd00301..cff0cf3 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BUILD
@@ -51,7 +51,6 @@
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/bazel/rules",
"//src/main/java/com/google/devtools/build/lib/bugreport",
- "//src/main/java/com/google/devtools/build/lib/buildeventservice:buildeventservice-options",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/transports",
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
index 648d789..d224ac5 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
@@ -45,7 +45,6 @@
import com.google.devtools.build.lib.analysis.config.FragmentFactory;
import com.google.devtools.build.lib.analysis.config.FragmentRegistry;
import com.google.devtools.build.lib.bugreport.BugReport;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceOptions.BesUploadMode;
import com.google.devtools.build.lib.buildeventstream.AnnounceBuildEventTransportsEvent;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
@@ -174,11 +173,6 @@
}
@Override
- public BesUploadMode getBesUploadMode() {
- return BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE;
- }
-
- @Override
public synchronized void sendBuildEvent(BuildEvent event) {
events.add(event);
try {
