Skip to content

[FLINK-31652][k8s] Handle the deleted event in case pod is deleted during the pending phase - #22311

Closed
SinBex wants to merge 2 commits into
apache:masterfrom
SinBex:handle_delete_event_in_k8s
Closed

[FLINK-31652][k8s] Handle the deleted event in case pod is deleted during the pending phase#22311
SinBex wants to merge 2 commits into
apache:masterfrom
SinBex:handle_delete_event_in_k8s

Conversation

@SinBex

@SinBex SinBex commented Mar 31, 2023

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Currently, in kubernetes deployment, if the taskmanager pod is deleted in 'Pending' phase, the flink job will get stuck and keep waiting for the pod scheduled. The cause reason is that the pod status will not be updated in time, so the KubernetesResourceManagerDriver won't detect the pod is terminated, but should handle the deleted event.

Brief change log

  • The method KubernetesResourceManagerDriver#handlePodEventsInMainThread handle the deleted event

Verifying this change

  • Add UT in KubernetesResourceManagerDriverTest#testOnPodDeletedWithDeletedEvent

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes)
  • The S3 file system connector: ( no)

Documentation

  • Does this pull request introduce a new feature? (no)

@flinkbot

flinkbot commented Mar 31, 2023

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@SinBex

SinBex commented Mar 31, 2023

Copy link
Copy Markdown
Contributor Author

@xintongsong Could you please help to review this PR in your free time? Thanks.

@huwh huwh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @SinBex , I have two minor comments, PTAL

import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;

import static io.fabric8.kubernetes.client.Watcher.Action.ADDED;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's better not use fabric classes in KubernetesResourceManagerDriver since these are Flink internal logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion. It is indeed better not to introduce an external class, I will use an internal enum to replace it.

() -> {
for (KubernetesPod pod : pods) {
if (pod.isTerminated()) {
if (action == DELETED || pod.isTerminated()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need some annotations to explain why we need this logic.

@SinBex

SinBex commented Mar 31, 2023

Copy link
Copy Markdown
Contributor Author

Thank you so much for your review @huwh , I have updated the code, PTAL~

@xintongsong xintongsong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @SinBex for fixing this. The PR LGTM. I have only 1 minor comment, which I'll address myself while merging this.

}

@Test
void testOnPodDeletedWithDeletedEvent() throws Exception {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
void testOnPodDeletedWithDeletedEvent() throws Exception {
void testOnPodDeletedBeforeScheduled() throws Exception {

xintongsong pushed a commit that referenced this pull request Apr 3, 2023
xintongsong pushed a commit that referenced this pull request Apr 3, 2023
RocMarshal pushed a commit to RocMarshal/flink that referenced this pull request May 9, 2024
wfrong pushed a commit to tongcheng-elong/flink that referenced this pull request Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants