Abacn commented on code in PR #36730:
URL: https://github.com/apache/beam/pull/36730#discussion_r2495318844
##########
runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineExecutionEnvironment.java:
##########
@@ -161,6 +167,36 @@ public PipelineResult executePipeline() throws Exception {
}
}
+ /** Prevents ThreadGroup destruction while Flink cleanup threads are still
running. */
+ private void ensureFlinkCleanupComplete(Object executionEnv) {
+ String javaVersion = System.getProperty("java.version");
+ if (javaVersion == null || !javaVersion.startsWith("1.8")) {
+ return;
Review Comment:
Busy wait is generally not desired but for band-aid fix I think it's fine.
We should narrow the scenario that introduced 2 seconds latency
- this only affects in-memory flink clusters, and also only affects batch
jobs (':runners:flink:1.19:validatesRunnerBatch'.). Can we call wait only for
batch?
- Can we check whether FlinkPipelineOptions.getFlinkMaster is a
"org.apache.flink.api.java.ExecutionEnvironment.LocalEnvironment" and only do
the busy wait for local environment?
##########
runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineExecutionEnvironment.java:
##########
@@ -161,6 +167,36 @@ public PipelineResult executePipeline() throws Exception {
}
}
+ /** Prevents ThreadGroup destruction while Flink cleanup threads are still
running. */
+ private void ensureFlinkCleanupComplete(Object executionEnv) {
Review Comment:
there is an executionEnv but not used. What is the consideration here?
--
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]
