scwhittle commented on code in PR #29882:
URL: https://github.com/apache/beam/pull/29882#discussion_r1440649469
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -97,6 +98,17 @@ public void onActivate(boolean pushing) {}
*/
public abstract void reportLull(Thread trackedThread, long millis);
+ /**
+ * Called when a lull has been detected since the start of bundle
processing. This indicates
+ * that more than {@link #BUNDLE_LULL_REPORT_MS} has been spent in the
same state without any
Review Comment:
"has been spent in the same state without any transitions" appears copied
from reportLull but in this case we maybe have made transitions we are just
still processing the same bundle.
how about "has been spent processing the same bundle (ie between
startBundle/finishBundle)."
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/SimpleExecutionState.java:
##########
@@ -149,11 +149,40 @@ public String getLullMessage(Thread trackedThread,
Duration millis) {
return message.toString();
}
+ @VisibleForTesting
+ public String getBundleLullMessage(Thread trackedThread, Duration millis) {
+ // TODO(ajamato): Share getLullMessage code with DataflowExecutionState.
+ String userStepName =
+
this.labelsMetadata.getOrDefault(MonitoringInfoConstants.Labels.PTRANSFORM,
null);
+ StringBuilder message = new StringBuilder();
+ message.append("Operation ongoing");
+ if (userStepName != null) {
+ message.append(" in bundle ").append(userStepName);
Review Comment:
the bundle will process across many steps, so this seems confusing.
It seems like instead it should show the fused stage here
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/SimpleExecutionState.java:
##########
@@ -149,11 +149,40 @@ public String getLullMessage(Thread trackedThread,
Duration millis) {
return message.toString();
}
+ @VisibleForTesting
+ public String getBundleLullMessage(Thread trackedThread, Duration millis) {
+ // TODO(ajamato): Share getLullMessage code with DataflowExecutionState.
Review Comment:
would be good to share within this class at least
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java:
##########
@@ -276,6 +276,23 @@ protected String getLullMessage(Thread trackedThread,
Duration lullDuration) {
return message.toString();
}
+ protected String getBundleLullMessage(Thread trackedThread, Duration
lullDuration) {
+ StringBuilder message = new StringBuilder();
+ message.append("Operation ongoing");
+ if (getStepName() != null) {
+ message.append(" in bundle ").append(getStepName().userName());
Review Comment:
ditto on step name being confusing for bundle and ditto about sharing code.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
