Skip to content

Fix Flink IllegalThreadStateException on Java 8 #36730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Abacn merged 4 commits into from
Nov 6, 2025
Merged

Fix Flink IllegalThreadStateException on Java 8 #36730

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3b9091d
Fix Flink IllegalThreadStateException on Java 8
aIbrahiim Nov 5, 2025
57c07f6
Fixed both Spotless formatting issues
aIbrahiim Nov 5, 2025
1860c25
Applied Flink ThreadGroup fix to Java 8 batch jobs in local environme…
aIbrahiim Nov 5, 2025
77c0bc6
Added executionEnv param
aIbrahiim Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .../flink/src/main/java/org/apache/beam/runners/flink/FlinkPipelineExecutionEnvironment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkNotNull;

import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.beam.runners.core.metrics.MetricsPusher;
import org.apache.beam.sdk.Pipeline;
import org.apache.beam.sdk.PipelineResult;
Expand Down Expand Up @@ -52,6 +54,8 @@ class FlinkPipelineExecutionEnvironment {
private static final Logger LOG =
LoggerFactory.getLogger(FlinkPipelineExecutionEnvironment.class);

private static final Set<ThreadGroup> protectedThreadGroups = ConcurrentHashMap.newKeySet();

private final FlinkPipelineOptions options;

/**
Expand Down Expand Up @@ -143,6 +147,7 @@ public PipelineResult executePipeline() throws Exception {
if (flinkBatchEnv != null) {
if (options.getAttachedMode()) {
JobExecutionResult jobExecutionResult = flinkBatchEnv.execute(jobName);
ensureFlinkCleanupComplete(flinkBatchEnv);
return createAttachedPipelineResult(jobExecutionResult);
} else {
JobClient jobClient = flinkBatchEnv.executeAsync(jobName);
Expand All @@ -151,6 +156,7 @@ public PipelineResult executePipeline() throws Exception {
} else if (flinkStreamEnv != null) {
if (options.getAttachedMode()) {
JobExecutionResult jobExecutionResult = flinkStreamEnv.execute(jobName);
ensureFlinkCleanupComplete(flinkStreamEnv);
return createAttachedPipelineResult(jobExecutionResult);
} else {
JobClient jobClient = flinkStreamEnv.executeAsync(jobName);
Expand All @@ -161,6 +167,36 @@ public PipelineResult executePipeline() throws Exception {
}
}

/** Prevents ThreadGroup destruction while Flink cleanup threads are still running. */
private void ensureFlinkCleanupComplete(Object executionEnv) {
Copy link
Contributor

@Abacn Abacn Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an executionEnv but not used. What is the consideration here?

Copy link
Contributor Author

@aIbrahiim aIbrahiim Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh i added for any future checks like stream or batching, or local env and so on.

String javaVersion = System.getProperty("java.version");
if (javaVersion == null || !javaVersion.startsWith("1.8")) {
return;
Copy link
Contributor

@Abacn Abacn Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@aIbrahiim aIbrahiim Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I agree we can narrow the scope to wait for batch jobs and detect the environment and apply it for local env.

Copy link
Contributor

@Abacn Abacn Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry realize this parameter is actually useful. We can do

if (executionEnv instanceof org.apache.flink.streaming.api.environment.StreamExecutionEnvironment.LocalStreamEnvironment || executionEnv instanceof org.apache.flink.api.java.ExecutionEnvironment.LocalEnvironment)

this is more reliable than check getFlinkMaster below

Copy link
Contributor Author

@aIbrahiim aIbrahiim Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the parameter

}

ThreadGroup currentThreadGroup = Thread.currentThread().getThreadGroup();
if (currentThreadGroup == null) {
return;
}

protectedThreadGroups.add(currentThreadGroup);

Thread cleanupReleaser =
new Thread(
() -> {
try {
Thread.sleep(2000); // 2 seconds should be enough for Flink cleanup
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} finally {
protectedThreadGroups.remove(currentThreadGroup);
}
},
"FlinkCleanupReleaser");
cleanupReleaser.setDaemon(true);
cleanupReleaser.start();
}

private FlinkDetachedRunnerResult createDetachedPipelineResult(
JobClient jobClient, FlinkPipelineOptions options) {
LOG.info("Pipeline submitted in detached mode");
Expand Down
Loading