You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "arvindram03 (via GitHub)" <gi...@apache.org> on 2024/03/20 18:52:31 UTC
[PR] Implementing lull reporting at bundle level processing [beam]
arvindram03 opened a new pull request, #30693:
URL: https://github.com/apache/beam/pull/30693
Reverts apache/beam#30648
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566512976
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -139,8 +140,17 @@ public String getDescription() {
*/
private volatile long millisSinceLastTransition = 0;
+ /**
+ * The number of milliseconds since the {@link ExecutionStateTracker} initial state.
+ *
+ * <p>This variable is updated by the Sampling thread, and read by the Progress Reporting thread,
+ * thus it being marked volatile.
+ */
+ private volatile long millisSinceBundleStart = 0;
Review Comment:
Following similar pattern from millisSinceLastTransition
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2073917578
failures unrelated to current changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2082700812
Failure looks unrelated, merging
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1577090727
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutor.java:
##########
@@ -107,6 +107,8 @@ public void execute() throws Exception {
}
throw exn;
}
+ } finally {
+ executionStateTracker.reset();
Review Comment:
Ok. moved the state reset logic to deactivate instead of reset.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1578571107
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -153,7 +154,6 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
- inOrder.verify(stateTracker).reset();
Review Comment:
Since we are not explicitly calling the deactivate method, sot sure how to test this. Unable to find similar code pointers from runner tests. Can you please point me to something ? verifications on mocks are not working as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566527849
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/DataflowExecutionStateTrackerTest.java:
##########
@@ -123,12 +158,115 @@ private void assertElementProcessingTimeCounter(NameContext step, int millis, in
.build()));
}
- private ExecutionStateTracker createTracker() {
+ private DataflowExecutionStateTracker createTracker() {
return new DataflowExecutionStateTracker(
sampler,
new TestDataflowExecutionState(NameContext.forStage("test-stage"), "other"),
counterSet,
options,
"test-work-item-id");
}
+
+ @Test
+ public void testLullReportsRightTrace() throws Exception {
+ Thread mockThread = mock(Thread.class);
+ StackTraceElement[] doFnStackTrace =
+ new StackTraceElement[] {
+ new StackTraceElement(
+ "userpackage.SomeUserDoFn", "helperMethod", "SomeUserDoFn.java", 250),
+ new StackTraceElement("userpackage.SomeUserDoFn", "process", "SomeUserDoFn.java", 450),
+ new StackTraceElement(
+ SimpleDoFnRunner.class.getName(), "processElement", "SimpleDoFnRunner.java", 500),
+ };
+ when(mockThread.getStackTrace()).thenReturn(doFnStackTrace);
+ FixedClock clock = new FixedClock(Clock.SYSTEM.currentTimeMillis());
+ DataflowExecutionStateTracker tracker = createTracker(clock);
+ // Adding test for the full thread dump, but since we can't mock
+ // Thread.getAllStackTraces(), we are starting a background thread
+ // to verify the full thread dump.
+ Thread backgroundThread =
+ new Thread("backgroundThread") {
+ @Override
+ public void run() {
+ try {
+ Thread.sleep(Long.MAX_VALUE);
+ } catch (InterruptedException e) {
+ // exiting the thread
+ }
+ }
+ };
+
+ backgroundThread.start();
+ try {
+ // Full thread dump should be performed, because we never performed
+ // a full thread dump before, and the lull duration is more than 20
+ // minutes.
+ tracker.reportBundleLull(mockThread, 30 * 60 * 1000);
+ verifyLullLog();
+
+ // Full thread dump should not be performed because the last dump
+ // was only 5 minutes ago.
+ clock.setTime(clock.currentTimeMillis() + Duration.standardMinutes(5L).getMillis());
+ tracker.reportBundleLull(mockThread, 30 * 60 * 1000);
+ verifyLullLog();
+
+ // Full thread dump should not be performed because the lull duration
+ // is only 6 minutes.
+ clock.setTime(clock.currentTimeMillis() + Duration.standardMinutes(16L).getMillis());
+ tracker.reportBundleLull(mockThread, 6 * 60 * 1000);
+ verifyLullLog();
+
+ // Full thread dump should be performed, because it has been 21 minutes
+ // since the last dump, and the lull duration is more than 20 minutes.
+ clock.setTime(clock.currentTimeMillis() + Duration.standardMinutes(16L).getMillis());
+ tracker.reportBundleLull(mockThread, 30 * 60 * 1000);
+ verifyLullLog();
+ } finally {
+ // Cleaning up the background thread.
+ backgroundThread.interrupt();
+ backgroundThread.join();
+ }
+ }
+
+ private void verifyLullLog() throws IOException {
+ File[] files = logFolder.listFiles();
+ assertThat(files, Matchers.arrayWithSize(1));
+ File logFile = files[0];
+ List<String> lines = Files.readAllLines(logFile.toPath());
+
+ String warnLines =
+ Joiner.on("\n").join(Iterables.filter(lines, line -> line.contains("\"WARN\"")));
+ assertThat(
+ warnLines,
+ Matchers.allOf(
+ Matchers.containsString("Operation ongoing in bundle for at least"),
+ Matchers.containsString(" without completing"),
+ Matchers.containsString("Processing times in each step"),
+ Matchers.containsString(
+ "org.apache.beam.runners.dataflow.worker.DataflowExecutionContext$DataflowExecutionStateTracker"),
+ Matchers.containsString("userpackage.SomeUserDoFn.helperMethod"),
+ Matchers.not(Matchers.containsString(SimpleDoFnRunner.class.getName()))));
+
+ String infoLines =
+ Joiner.on("\n").join(Iterables.filter(lines, line -> line.contains("\"INFO\"")));
+ assertThat(
+ infoLines,
+ Matchers.not(
Review Comment:
Fixed the comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566528993
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowExecutionContext.java:
##########
@@ -310,12 +350,64 @@ public Closeable activate() {
}
}
+ private String getBundleLullMessage(Thread trackedThread, Duration lullDuration) {
+ StringBuilder message = new StringBuilder();
+ message
+ .append("Operation ongoing in bundle for at least ")
+ .append(DURATION_FORMATTER.print(lullDuration.toPeriod()))
+ .append(" without completing")
+ .append("\n");
+ synchronized (this) {
+ if (this.activeMessageMetadata != null) {
+ message.append(
+ "Current user step name: " + getActiveMessageMetadata().get().userStepName() + "\n");
+ message.append(
+ "Time spent in this step(millis): "
+ + (clock.currentTimeMillis() - getActiveMessageMetadata().get().startTime())
+ + "\n");
+ }
+ message.append("Processing times in each step(millis)\n");
+ for (Map.Entry<String, IntSummaryStatistics> entry :
+ this.processingTimesByStep.entrySet()) {
+ message.append("Step name: " + entry.getKey() + "\n");
+ message.append("Time spent in this step: " + entry.getValue().toString() + "\n");
+ }
+ }
+
+ if (trackedThread != null) {
+ message.append(StackTraceUtil.getStackTraceForLullMessage(trackedThread.getStackTrace()));
+ }
+ return message.toString();
+ }
+
@Override
protected void takeSampleOnce(long millisSinceLastSample) {
elementExecutionTracker.takeSample(millisSinceLastSample);
super.takeSampleOnce(millisSinceLastSample);
}
+ @Override
+ protected void reportBundleLull(Thread trackedThread, long millisElapsedSinceBundleStart) {
+ // If we're not logging warnings, nothing to report.
+ if (!LOG.isWarnEnabled()) {
+ return;
+ }
+
+ Duration lullDuration = Duration.millis(millisElapsedSinceBundleStart);
+
+ // Since the lull reporting executes in the sampler thread, it won't automatically inherit the
+ // context of the current step. To ensure things are logged correctly, we get the currently
+ // registered DataflowWorkerLoggingHandler and log directly in the desired context.
+ LogRecord logRecord =
+ new LogRecord(Level.WARNING, getBundleLullMessage(trackedThread, lullDuration));
+ logRecord.setLoggerName(DataflowExecutionStateTracker.LOG.getName());
+
+ // Publish directly in the context of this specific ExecutionState.
+ DataflowWorkerLoggingHandler dataflowLoggingHandler =
+ DataflowWorkerLoggingInitializer.getLoggingHandler();
+ dataflowLoggingHandler.publish(logRecord);
Review Comment:
We initially add full thread dump here which spammed the logs. We removed as part of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1578531638
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -321,6 +321,11 @@ public long getNextLullReportMs() {
return nextLullReportMs;
}
+ /** Return the time of the next bundle lull report. */
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1578571107
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -153,7 +154,6 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
- inOrder.verify(stateTracker).reset();
Review Comment:
Since we are not explicitly calling the method, sot sure how to test this. Unable to find similar code pointers from runner tests. Can you please point me to something ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle merged PR #30693:
URL: https://github.com/apache/beam/pull/30693
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2083243573
Thanks Sam.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "dustin12 (via GitHub)" <gi...@apache.org>.
dustin12 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1552236808
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -335,6 +352,19 @@ protected void takeSampleOnce(long millisSinceLastSample) {
transitionsAtLastSample = transitionsAtThisSample;
}
updateMillisSinceLastTransition(millisSinceLastSample, state);
+ updateMillisSinceBundleStart(millisSinceLastSample);
+ }
+
+ // Override this to implement bundle level lull reporting.
+ protected void reportBundleLull(Thread trackedThread, long millisSinceBundleStart) {}
+
+ @SuppressWarnings("NonAtomicVolatileUpdate")
+ private void updateMillisSinceBundleStart(long millisSinceLastSample) {
+ millisSinceBundleStart += millisSinceLastSample;
+ if (millisSinceBundleStart > nextBundleLullReportMs) {
+ reportBundleLull(trackedThread, millisSinceBundleStart);
+ nextBundleLullReportMs += BUNDLE_LULL_REPORT_MS;
+ }
}
Review Comment:
maybe a BEAM/Java developer can help here. Is there a reason we supress these warnings? Is it safe to do for some reason? It seems to indicate a possible data race which we should fix?
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StackTraceUtil.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.beam.runners.dataflow.worker;
+
+import org.apache.beam.runners.core.SimpleDoFnRunner;
+import org.apache.beam.sdk.annotations.Internal;
+import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableSet;
+
+/** Utility methods to print the stack traces of all the threads. */
+@Internal
+public final class StackTraceUtil {
+ private static final ImmutableSet<String> FRAMEWORK_CLASSES =
+ ImmutableSet.of(SimpleDoFnRunner.class.getName(), DoFnInstanceManagers.class.getName());
+
+ public static String getStackTraceForLullMessage(StackTraceElement[] stackTrace) {
+ StringBuilder message = new StringBuilder();
+ for (StackTraceElement e : stackTrace) {
+ if (FRAMEWORK_CLASSES.contains(e.getClassName())) {
+ break;
Review Comment:
where is this code from? It doesn't seem to have moved from another file and I'm not sure why it sbeing added. I'm also a bit confused as to why we would stop a stacktrace as soon as we got to a Framework class, it doesn't seem related to bundle lulls.
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -139,8 +140,17 @@ public String getDescription() {
*/
private volatile long millisSinceLastTransition = 0;
+ /**
+ * The number of milliseconds since the {@link ExecutionStateTracker} initial state.
+ *
+ * <p>This variable is updated by the Sampling thread, and read by the Progress Reporting thread,
+ * thus it being marked volatile.
+ */
+ private volatile long millisSinceBundleStart = 0;
Review Comment:
why is this marked as volatile? And thus needs the supress warning on line 361.
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowExecutionContext.java:
##########
@@ -310,12 +350,64 @@ public Closeable activate() {
}
}
+ private String getBundleLullMessage(Thread trackedThread, Duration lullDuration) {
+ StringBuilder message = new StringBuilder();
+ message
+ .append("Operation ongoing in bundle for at least ")
+ .append(DURATION_FORMATTER.print(lullDuration.toPeriod()))
+ .append(" without completing")
+ .append("\n");
+ synchronized (this) {
+ if (this.activeMessageMetadata != null) {
+ message.append(
+ "Current user step name: " + getActiveMessageMetadata().get().userStepName() + "\n");
+ message.append(
+ "Time spent in this step(millis): "
+ + (clock.currentTimeMillis() - getActiveMessageMetadata().get().startTime())
+ + "\n");
+ }
+ message.append("Processing times in each step(millis)\n");
+ for (Map.Entry<String, IntSummaryStatistics> entry :
+ this.processingTimesByStep.entrySet()) {
+ message.append("Step name: " + entry.getKey() + "\n");
+ message.append("Time spent in this step: " + entry.getValue().toString() + "\n");
+ }
+ }
+
+ if (trackedThread != null) {
+ message.append(StackTraceUtil.getStackTraceForLullMessage(trackedThread.getStackTrace()));
+ }
+ return message.toString();
+ }
+
@Override
protected void takeSampleOnce(long millisSinceLastSample) {
elementExecutionTracker.takeSample(millisSinceLastSample);
super.takeSampleOnce(millisSinceLastSample);
}
+ @Override
+ protected void reportBundleLull(Thread trackedThread, long millisElapsedSinceBundleStart) {
+ // If we're not logging warnings, nothing to report.
+ if (!LOG.isWarnEnabled()) {
+ return;
+ }
+
+ Duration lullDuration = Duration.millis(millisElapsedSinceBundleStart);
+
+ // Since the lull reporting executes in the sampler thread, it won't automatically inherit the
+ // context of the current step. To ensure things are logged correctly, we get the currently
+ // registered DataflowWorkerLoggingHandler and log directly in the desired context.
+ LogRecord logRecord =
+ new LogRecord(Level.WARNING, getBundleLullMessage(trackedThread, lullDuration));
+ logRecord.setLoggerName(DataflowExecutionStateTracker.LOG.getName());
+
+ // Publish directly in the context of this specific ExecutionState.
+ DataflowWorkerLoggingHandler dataflowLoggingHandler =
+ DataflowWorkerLoggingInitializer.getLoggingHandler();
+ dataflowLoggingHandler.publish(logRecord);
Review Comment:
should we do the logFullThreadDump if shouldLogFullThreadDump is true? I'm not sure who added that. Maybe Sam knows? If no one knows lets not add it but since it exists for single message it seems like someone added it on purpose.
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/DataflowExecutionStateTrackerTest.java:
##########
@@ -123,12 +158,115 @@ private void assertElementProcessingTimeCounter(NameContext step, int millis, in
.build()));
}
- private ExecutionStateTracker createTracker() {
+ private DataflowExecutionStateTracker createTracker() {
return new DataflowExecutionStateTracker(
sampler,
new TestDataflowExecutionState(NameContext.forStage("test-stage"), "other"),
counterSet,
options,
"test-work-item-id");
}
+
+ @Test
+ public void testLullReportsRightTrace() throws Exception {
+ Thread mockThread = mock(Thread.class);
+ StackTraceElement[] doFnStackTrace =
+ new StackTraceElement[] {
+ new StackTraceElement(
+ "userpackage.SomeUserDoFn", "helperMethod", "SomeUserDoFn.java", 250),
+ new StackTraceElement("userpackage.SomeUserDoFn", "process", "SomeUserDoFn.java", 450),
+ new StackTraceElement(
+ SimpleDoFnRunner.class.getName(), "processElement", "SimpleDoFnRunner.java", 500),
+ };
+ when(mockThread.getStackTrace()).thenReturn(doFnStackTrace);
+ FixedClock clock = new FixedClock(Clock.SYSTEM.currentTimeMillis());
+ DataflowExecutionStateTracker tracker = createTracker(clock);
+ // Adding test for the full thread dump, but since we can't mock
+ // Thread.getAllStackTraces(), we are starting a background thread
+ // to verify the full thread dump.
+ Thread backgroundThread =
+ new Thread("backgroundThread") {
+ @Override
+ public void run() {
+ try {
+ Thread.sleep(Long.MAX_VALUE);
+ } catch (InterruptedException e) {
+ // exiting the thread
+ }
+ }
+ };
+
+ backgroundThread.start();
+ try {
+ // Full thread dump should be performed, because we never performed
+ // a full thread dump before, and the lull duration is more than 20
+ // minutes.
+ tracker.reportBundleLull(mockThread, 30 * 60 * 1000);
+ verifyLullLog();
+
+ // Full thread dump should not be performed because the last dump
+ // was only 5 minutes ago.
+ clock.setTime(clock.currentTimeMillis() + Duration.standardMinutes(5L).getMillis());
+ tracker.reportBundleLull(mockThread, 30 * 60 * 1000);
+ verifyLullLog();
+
+ // Full thread dump should not be performed because the lull duration
+ // is only 6 minutes.
+ clock.setTime(clock.currentTimeMillis() + Duration.standardMinutes(16L).getMillis());
+ tracker.reportBundleLull(mockThread, 6 * 60 * 1000);
+ verifyLullLog();
+
+ // Full thread dump should be performed, because it has been 21 minutes
+ // since the last dump, and the lull duration is more than 20 minutes.
+ clock.setTime(clock.currentTimeMillis() + Duration.standardMinutes(16L).getMillis());
+ tracker.reportBundleLull(mockThread, 30 * 60 * 1000);
+ verifyLullLog();
+ } finally {
+ // Cleaning up the background thread.
+ backgroundThread.interrupt();
+ backgroundThread.join();
+ }
+ }
+
+ private void verifyLullLog() throws IOException {
+ File[] files = logFolder.listFiles();
+ assertThat(files, Matchers.arrayWithSize(1));
+ File logFile = files[0];
+ List<String> lines = Files.readAllLines(logFile.toPath());
+
+ String warnLines =
+ Joiner.on("\n").join(Iterables.filter(lines, line -> line.contains("\"WARN\"")));
+ assertThat(
+ warnLines,
+ Matchers.allOf(
+ Matchers.containsString("Operation ongoing in bundle for at least"),
+ Matchers.containsString(" without completing"),
+ Matchers.containsString("Processing times in each step"),
+ Matchers.containsString(
+ "org.apache.beam.runners.dataflow.worker.DataflowExecutionContext$DataflowExecutionStateTracker"),
+ Matchers.containsString("userpackage.SomeUserDoFn.helperMethod"),
+ Matchers.not(Matchers.containsString(SimpleDoFnRunner.class.getName()))));
+
+ String infoLines =
+ Joiner.on("\n").join(Iterables.filter(lines, line -> line.contains("\"INFO\"")));
+ assertThat(
+ infoLines,
+ Matchers.not(
Review Comment:
it seems from this we are always verifying that a full thread dump is not performed. The comments in the tests sometimes say we want a full thread dump although looking at code I don't see when we'd ever get one.
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java:
##########
@@ -296,55 +267,6 @@ public void reportLull(Thread trackedThread, long millis) {
DataflowWorkerLoggingHandler dataflowLoggingHandler =
DataflowWorkerLoggingInitializer.getLoggingHandler();
dataflowLoggingHandler.publish(this, logRecord);
-
- if (shouldLogFullThreadDump(lullDuration)) {
Review Comment:
why is reportLull changing so much? I don't think you call this since your using reportBundleLull instead. It seems like this would just remain the same.
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java:
##########
@@ -185,41 +181,19 @@ public abstract static class DataflowExecutionState extends ExecutionState {
private final ProfileScope profileScope;
private final @Nullable MetricsContainer metricsContainer;
- /** Clock used to either provide real system time or mocked to virtualize time for testing. */
- private final Clock clock;
-
public DataflowExecutionState(
NameContext nameContext,
String stateName,
@Nullable String requestingStepName,
@Nullable Integer inputIndex,
@Nullable MetricsContainer metricsContainer,
ProfileScope profileScope) {
- this(
- nameContext,
- stateName,
- requestingStepName,
- inputIndex,
- metricsContainer,
- profileScope,
- Clock.SYSTEM);
- }
-
- public DataflowExecutionState(
- NameContext nameContext,
- String stateName,
- @Nullable String requestingStepName,
- @Nullable Integer inputIndex,
- @Nullable MetricsContainer metricsContainer,
- ProfileScope profileScope,
- Clock clock) {
super(stateName);
Review Comment:
why the change here from this constructor to super and then setting each field? Maybe a Java thing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566524587
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java:
##########
@@ -185,41 +181,19 @@ public abstract static class DataflowExecutionState extends ExecutionState {
private final ProfileScope profileScope;
private final @Nullable MetricsContainer metricsContainer;
- /** Clock used to either provide real system time or mocked to virtualize time for testing. */
- private final Clock clock;
-
public DataflowExecutionState(
NameContext nameContext,
String stateName,
@Nullable String requestingStepName,
@Nullable Integer inputIndex,
@Nullable MetricsContainer metricsContainer,
ProfileScope profileScope) {
- this(
- nameContext,
- stateName,
- requestingStepName,
- inputIndex,
- metricsContainer,
- profileScope,
- Clock.SYSTEM);
- }
-
- public DataflowExecutionState(
- NameContext nameContext,
- String stateName,
- @Nullable String requestingStepName,
- @Nullable Integer inputIndex,
- @Nullable MetricsContainer metricsContainer,
- ProfileScope profileScope,
- Clock clock) {
super(stateName);
Review Comment:
Yes, we need to inititalize super class variables and set of variables specific to this class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2058398756
> LGTM after the 3 comments are addressed.
The comments are addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1577515789
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -153,7 +154,6 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
- inOrder.verify(stateTracker).reset();
Review Comment:
how about adding verification that activate/deactivate is called on the tracker to this test?
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -321,6 +321,11 @@ public long getNextLullReportMs() {
return nextLullReportMs;
}
+ /** Return the time of the next bundle lull report. */
Review Comment:
This sounds like an absolute time, while this is the duration after the bundle start that the log should occur. Maybe better name would be nextBundleLullDurationReportMs for method and variable?
this is just exposed for testing as well so you could remove the public and have it be package private if that works. or mark with the @VisibleForTesting annotaiton.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2076254704
failures are unrelated to current change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566525149
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StackTraceUtil.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.beam.runners.dataflow.worker;
+
+import org.apache.beam.runners.core.SimpleDoFnRunner;
+import org.apache.beam.sdk.annotations.Internal;
+import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableSet;
+
+/** Utility methods to print the stack traces of all the threads. */
+@Internal
+public final class StackTraceUtil {
+ private static final ImmutableSet<String> FRAMEWORK_CLASSES =
+ ImmutableSet.of(SimpleDoFnRunner.class.getName(), DoFnInstanceManagers.class.getName());
+
+ public static String getStackTraceForLullMessage(StackTraceElement[] stackTrace) {
+ StringBuilder message = new StringBuilder();
+ for (StackTraceElement e : stackTrace) {
+ if (FRAMEWORK_CLASSES.contains(e.getClassName())) {
+ break;
Review Comment:
This was moved from runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "dustin12 (via GitHub)" <gi...@apache.org>.
dustin12 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566540948
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -335,6 +352,19 @@ protected void takeSampleOnce(long millisSinceLastSample) {
transitionsAtLastSample = transitionsAtThisSample;
}
updateMillisSinceLastTransition(millisSinceLastSample, state);
+ updateMillisSinceBundleStart(millisSinceLastSample);
+ }
+
+ // Override this to implement bundle level lull reporting.
+ protected void reportBundleLull(Thread trackedThread, long millisSinceBundleStart) {}
+
+ @SuppressWarnings("NonAtomicVolatileUpdate")
+ private void updateMillisSinceBundleStart(long millisSinceLastSample) {
+ millisSinceBundleStart += millisSinceLastSample;
+ if (millisSinceBundleStart > nextBundleLullReportMs) {
+ reportBundleLull(trackedThread, millisSinceBundleStart);
+ nextBundleLullReportMs += BUNDLE_LULL_REPORT_MS;
+ }
}
Review Comment:
Add a comment that the ignore warning is bc. updateMillisSinceBundleStart will only ever be called by the thread doing the dofn processing. Reset is where multiple threads may call and there they are doing blind writes.
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -139,8 +140,17 @@ public String getDescription() {
*/
private volatile long millisSinceLastTransition = 0;
+ /**
+ * The number of milliseconds since the {@link ExecutionStateTracker} initial state.
+ *
+ * <p>This variable is updated by the Sampling thread, and read by the Progress Reporting thread,
+ * thus it being marked volatile.
+ */
+ private volatile long millisSinceBundleStart = 0;
Review Comment:
its because we want to write from multiple threads during reset.
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java:
##########
@@ -296,55 +267,6 @@ public void reportLull(Thread trackedThread, long millis) {
DataflowWorkerLoggingHandler dataflowLoggingHandler =
DataflowWorkerLoggingInitializer.getLoggingHandler();
dataflowLoggingHandler.publish(this, logRecord);
-
- if (shouldLogFullThreadDump(lullDuration)) {
Review Comment:
I don't think we should change the full thread dump for message level. It happens only if a single message is stuck for 20 minutes which seems like the pipeline is fully broken anyway. I agree leave it off for bundle level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1566528219
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java:
##########
@@ -296,55 +267,6 @@ public void reportLull(Thread trackedThread, long millis) {
DataflowWorkerLoggingHandler dataflowLoggingHandler =
DataflowWorkerLoggingInitializer.getLoggingHandler();
dataflowLoggingHandler.publish(this, logRecord);
-
- if (shouldLogFullThreadDump(lullDuration)) {
Review Comment:
We moved this content to StackTraceUtils to reuse the logic. We have also removed the full thread dump from here to avoid the spamming.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2082494509
some tests were cancelled for some reasno so closing/reopening to run them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2021697582
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @johnjcasey added as fallback since no labels match configuration
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle closed pull request #30693: Implementing lull reporting at bundle level processing
URL: https://github.com/apache/beam/pull/30693
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1574410652
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutor.java:
##########
@@ -107,6 +107,8 @@ public void execute() throws Exception {
}
throw exn;
}
+ } finally {
+ executionStateTracker.reset();
Review Comment:
Reset also resets additional state than just the bundle elapsed tracking and it is harder to verify it is correct.
How about instead clearing the bundle lull tracking in to ExecutionStateTracker::deactivate as well which is called by the stateCloser returned by activate above?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1580672896
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -154,6 +154,7 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
+ inOrder.verify(stateTracker).deactivate();
Review Comment:
would improve test to verify the activate happens before any the start methods, here and below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2051644369
Reminder, please take a look at this pr: @johnjcasey
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2010529055
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1580203193
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -153,7 +154,6 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
- inOrder.verify(stateTracker).reset();
Review Comment:
Thanks for the suggestion. Learned about the spy method from your comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "scwhittle (via GitHub)" <gi...@apache.org>.
scwhittle commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1579063822
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -153,7 +154,6 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
- inOrder.verify(stateTracker).reset();
Review Comment:
I was thinking to just add verifications on the stateTracker methods here (like you were doing with reset). So I was thinking of something like this
inOrder.verify(stateTracker).activate()
inOrder.verify(o3).start();
inOrder.verify(o2).start();
inOrder.verify(o1).start();
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
inOrder.verify(stateTracker).deactivate();
However I realize now that executor.execute() is just calling stateTracker.activate() which in the real object returns a closable that is deactivated but with the pure mock returns an arbitrary closable. You could set up an expectation that stateTracker.activate() returns a closable that calls stateTracker.deactivate() but instead I think it would be better to just use a spy of a real object.
ExecutionStateTracker stateTracker = Mockito.spy(ExecutionStateTracker.newForTest());
Then the tracker will be real and return the real closable that will call deactivate. The spy just lets you verify it is called.
It looks like activate() is public, deactivate is private, you might have to make it public with visiblefortesting annotation for it to work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "arvindram03 (via GitHub)" <gi...@apache.org>.
arvindram03 commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1581745688
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -154,6 +154,7 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
+ inOrder.verify(stateTracker).deactivate();
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implementing lull reporting at bundle level processing [beam]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #30693:
URL: https://github.com/apache/beam/pull/30693#issuecomment-2051644673
## [Codecov](https://app.codecov.io/gh/apache/beam/pull/30693?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 70.85%. Comparing base [(`6f6bbe1`)](https://app.codecov.io/gh/apache/beam/commit/6f6bbe1ca73682081469189a13d4ecfe94b0717a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`1a6a67d`)](https://app.codecov.io/gh/apache/beam/pull/30693?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 111 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #30693 +/- ##
=============================================
+ Coverage 54.32% 70.85% +16.53%
- Complexity 0 2987 +2987
=============================================
Files 307 1062 +755
Lines 38123 132673 +94550
Branches 0 3231 +3231
=============================================
+ Hits 20711 94012 +73301
- Misses 15779 35567 +19788
- Partials 1633 3094 +1461
```
| [Flag](https://app.codecov.io/gh/apache/beam/pull/30693/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [java](https://app.codecov.io/gh/apache/beam/pull/30693/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.56% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/beam/pull/30693?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org