Abacn commented on code in PR #35120:
URL: https://github.com/apache/beam/pull/35120#discussion_r2132855403
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ExecutionStateSampler.java:
##########
@@ -65,6 +66,7 @@ public class ExecutionStateSampler {
private static final Logger LOG =
LoggerFactory.getLogger(ExecutionStateSampler.class);
private static final int DEFAULT_SAMPLING_PERIOD_MS = 200;
private static final long MAX_LULL_TIME_MS = TimeUnit.MINUTES.toMillis(5);
+ private static final int MIN_LULL_TIME_MINUTE_FOR_RESTART = 10; // Open to
change.
Review Comment:
we may document in pipeline options that only values >= 10 are honored
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java:
##########
@@ -424,4 +424,16 @@ public String create(PipelineOptions options) {
return String.format("%s/%s", info.getName(),
info.getVersion()).replace(" ", "_");
}
}
+
+ /**
+ * The time limit (minute) that an SDK worker allows for a PTransform
operation before signaling
+ * the runner harness to restart the SDK worker.
+ */
+ @Default.Integer(Integer.MAX_VALUE)
Review Comment:
It sounds like this better fits in
https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/options/SdkHarnessOptions.java?
Large number is still finite number technically (here it is 4,000 y). Things
will get a little bit more complicated in Python as tehre isn't
Integer.MAX_VALUE for Python native int
Consider set default to 0 and handle it in implementation?
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ExecutionStateSampler.java:
##########
@@ -357,6 +377,19 @@ private void takeSample(long currentTimeMillis, long
millisSinceLastSample) {
transitionsAtLastSample = transitionsAtThisSample;
} else {
long lullTimeMs = currentTimeMillis - lastTransitionTimeMillis.get();
+
+ try {
Review Comment:
People who are more familiar with this path may answer:
Will `lullTimeMs` give the intended value? It sounds to me this is some
sample interval, not necessary PTransform latency
--
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]
