Skip to content
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.

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

[SPARK-33029][CORE][WEBUI] Fix the UI executor page incorrectly marking the driver as excluded #30954

Closed
wants to merge 3 commits into from

Conversation

baohe-zhang
Copy link

What changes were proposed in this pull request?

Filter out the driver entity when updating the exclusion status of live executors(including the driver), so the UI won't be marked as excluded in the UI even if the node that hosts the driver has been marked as excluded.

Why are the changes needed?

Before this change, if we run spark with the standalone mode and with spark.blacklist.enabled=true. The driver will be marked as excluded when the host that hosts that driver has been marked as excluded. While it's incorrect because the exclude list feature will exclude executors only and the driver is still active.
image
After the fix, the driver won't be marked as excluded.
image

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manual test. Reopen the UI and see the driver is no longer marked as excluded.

@github-actions github-actions bot added the CORE label Dec 28, 2020
@baohe-zhang
Copy link
Author

cc @tgravescs

@SparkQA
Copy link

SparkQA commented Dec 28, 2020

Test build #133458 has finished for PR 30954 at commit 00036d7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

To retriever GitHub Action, could you rebase to the master branch, @baohe-zhang ?

@baohe-zhang
Copy link
Author

To retriever GitHub Action, could you rebase to the master branch, @baohe-zhang ?

Done.

@tgravescs
Copy link
Contributor

it looks like you need to fix up the test:

[info] - executor node excludeOnFailure *** FAILED *** (220 milliseconds)

I think we had the file have the driver as excluded

@baohe-zhang
Copy link
Author

@tgravescs The unit tests are fixed.

@SparkQA
Copy link

SparkQA commented Jan 5, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38212/

@SparkQA
Copy link

SparkQA commented Jan 5, 2021

Test build #133623 has finished for PR 30954 at commit 8880718.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 5, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38212/

Copy link
Contributor

@tgravescs tgravescs left a comment

Choose a reason for hiding this comment

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

looks good pending tests run, the last one was unrelated so kicked it again.

@dongjoon-hyun
Copy link
Member

Thank you, @baohe-zhang and @tgravescs .
cc @HyukjinKwon since this affects 3.0/3.1/3.2.

@HyukjinKwon
Copy link
Member

Thanks @dongjoon-hyun. cc @gengliangwang and @sarutak FYI

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master/3.1.

dongjoon-hyun pushed a commit that referenced this pull request Jan 6, 2021
…ng the driver as excluded

### What changes were proposed in this pull request?
Filter out the driver entity when updating the exclusion status of live executors(including the driver), so the UI won't be marked as excluded in the UI even if the node that hosts the driver has been marked as excluded.

### Why are the changes needed?
Before this change, if we run spark with the standalone mode and with spark.blacklist.enabled=true. The driver will be marked as excluded when the host that hosts that driver has been marked as excluded. While it's incorrect because the exclude list feature will exclude executors only and the driver is still active.
![image](https://user-images.githubusercontent.com/26694233/103238740-35c05180-4911-11eb-99a2-c87c059ba0cf.png)
After the fix, the driver won't be marked as excluded.
![image](https://user-images.githubusercontent.com/26694233/103238806-6f915800-4911-11eb-80d5-3c99266cfd0a.png)

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Manual test. Reopen the UI and see the driver is no longer marked as excluded.

Closes #30954 from baohe-zhang/SPARK-33029.

Authored-by: Baohe Zhang <baohe.zhang@verizonmedia.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
(cherry picked from commit 2951082)
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
@dongjoon-hyun
Copy link
Member

Could you make a backport to branch-3.0, @baohe-zhang ?

@baohe-zhang
Copy link
Author

@dongjoon-hyun Sure I will make a backport.

@dongjoon-hyun
Copy link
Member

Thanks!

dongjoon-hyun pushed a commit that referenced this pull request Jan 6, 2021
…marking the driver as blacklisted

This is a backport of #30954

### What changes were proposed in this pull request?
Filter out the driver entity when updating the exclusion status of live executors(including the driver), so the driver won't be marked as blacklisted in the UI even if the node that hosts the driver has been marked as blacklisted.

### Why are the changes needed?
Before this change, if we run spark with the standalone mode and with spark.blacklist.enabled=true. The driver will be marked as blacklisted when the host that hosts that driver has been marked as blacklisted. While it's incorrect because the exclude list feature will exclude executors only and the driver is still active.
![image](https://user-images.githubusercontent.com/26694233/103732959-3494c180-4fae-11eb-9da0-2c906309ea83.png)
After the fix, the driver won't be marked as blacklisted.
![image](https://user-images.githubusercontent.com/26694233/103732974-3fe7ed00-4fae-11eb-90d1-7ee44d4ed7c9.png)

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Manual test. Reopen the UI and see the driver is no longer marked as blacklisted.

Closes #31057 from baohe-zhang/SPARK-33029-3.0.

Authored-by: Baohe Zhang <baohe.zhang@verizonmedia.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants