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-32598][Scheduler] Fix missing driver logs under UI App-Executors tab in standalone cluster mode #29644

Closed
wants to merge 1 commit into from

Conversation

KevinSmile
Copy link
Contributor

@KevinSmile KevinSmile commented Sep 4, 2020

What changes were proposed in this pull request?

Fix [SPARK-32598] (missing driver logs under UI-ApplicationDetails-Executors tab in standalone cluster mode) .

The direct bug is: the original author forgot to implement getDriverLogUrls in StandaloneSchedulerBackend

/**
* Get the URLs for the driver logs. These URLs are used to display the links in the UI
* Executors tab for the driver.
* @return Map containing the log names and their respective URLs
*/
def getDriverLogUrls: Option[Map[String, String]] = None

So we set DriverLogUrls as env in DriverRunner, and retrieve it at StandaloneSchedulerBackend.

Why are the changes needed?

Fix bug [SPARK-32598].

Does this PR introduce any user-facing change?

Yes. User will see driver logs (standalone cluster mode) under UI-ApplicationDetails-Executors tab now.

Before:
image

After:
image

How was this patch tested?

Re-check the real case in [SPARK-32598] and found this user-facing bug fixed.

@KevinSmile KevinSmile changed the title [SPARK-32598][Scheduler] fix missing driver logs in UI Executors tab in standalone mode [SPARK-32598][Scheduler] Fix missing driver logs under UI App-Executors tab in standalone mode Sep 7, 2020
@KevinSmile
Copy link
Contributor Author

@srowen Can you also help find someone review this PR? Thanks!

@srowen
Copy link
Member

srowen commented Sep 9, 2020

In standalone mode aren't these just the logs from the application driver, the stdout output? I don't know this part well (and not enough to review)

@KevinSmile
Copy link
Contributor Author

Yes, it's just stdout & stderr of driver, currently missing in web-ui. Thanks anyway! @srowen

@KevinSmile
Copy link
Contributor Author

@zsxwing @Ngone51 Do you have time to review this? Thanks!

@KevinSmile KevinSmile force-pushed the kw-dev-master branch 3 times, most recently from 219e16b to 6dcdcdf Compare September 12, 2020 11:41
Copy link
Member

@Ngone51 Ngone51 left a comment

Choose a reason for hiding this comment

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

IIUC, this only works for the cluster mode? If so, we could set the driver log URLs through the env variable in DriverRunner like ExecutorRunner does? Then, we can avoid RPC calls between driver and Master.

@KevinSmile KevinSmile force-pushed the kw-dev-master branch 2 times, most recently from 9f85ba3 to 2188497 Compare September 15, 2020 09:47
@KevinSmile KevinSmile changed the title [SPARK-32598][Scheduler] Fix missing driver logs under UI App-Executors tab in standalone mode [SPARK-32598][Scheduler] Fix missing driver logs under UI App-Executors tab in standalone cluster mode Sep 15, 2020
@KevinSmile
Copy link
Contributor Author

KevinSmile commented Sep 15, 2020

@Ngone51 Thanks for your suggestion! Yes you're right, I have updated this patch. I was thinking of using RPC to get full worker info may help further changes, seems no need.
Also I find it hard to have a standalone cluster mode in suites, and this patch just add some envs, so I didn't add ut here.

@Ngone51
Copy link
Member

Ngone51 commented Sep 22, 2020

Can you do some manual tests and paste screenshots of UI to show the fix result in the PR description?

@KevinSmile
Copy link
Contributor Author

Can you do some manual tests and paste screenshots of UI to show the fix result in the PR description?

Done

@Ngone51
Copy link
Member

Ngone51 commented Sep 23, 2020

LGTM. I also verified locally. It looks good.

@jiangxb1987 Could you also take a look?

Copy link
Contributor

@jiangxb1987 jiangxb1987 left a comment

Choose a reason for hiding this comment

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

LGTM

@KevinSmile
Copy link
Contributor Author

@HyukjinKwon Can you help let Jenkins test this?

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Oct 12, 2020

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

@SparkQA
Copy link

SparkQA commented Oct 12, 2020

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

@SparkQA
Copy link

SparkQA commented Oct 12, 2020

Test build #129656 has finished for PR 29644 at commit a0bdce9.

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

@SparkQA
Copy link

SparkQA commented Jan 15, 2021

Test build #134103 has finished for PR 29644 at commit a0bdce9.

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

@srowen srowen closed this in c75c29d Jan 15, 2021
srowen pushed a commit that referenced this pull request Jan 15, 2021
…rs tab in standalone cluster mode

### What changes were proposed in this pull request?
Fix  [SPARK-32598] (missing driver logs under UI-ApplicationDetails-Executors tab in standalone cluster mode) .

The direct bug is: the original author forgot to implement `getDriverLogUrls` in `StandaloneSchedulerBackend`

https://github.com/apache/spark/blob/1de272f98d0ff22d0dd151797f22b8faf310963a/core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala#L70-L75

So we set DriverLogUrls as env in `DriverRunner`, and retrieve it at `StandaloneSchedulerBackend`.

### Why are the changes needed?
Fix bug  [SPARK-32598].

### Does this PR introduce _any_ user-facing change?
Yes. User will see driver logs (standalone cluster mode) under UI-ApplicationDetails-Executors tab now.

Before:
![image](https://user-images.githubusercontent.com/17903517/93901055-b5de8600-fd28-11ea-879a-d97e6f70cc6e.png)

After:
![image](https://user-images.githubusercontent.com/17903517/93901080-baa33a00-fd28-11ea-8895-3787c5efbf88.png)

### How was this patch tested?
Re-check the real case in [SPARK-32598] and found this user-facing bug fixed.

Closes #29644 from KevinSmile/kw-dev-master.

Authored-by: KevinSmile <kevinwang013@hotmail.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
(cherry picked from commit c75c29d)
Signed-off-by: Sean Owen <srowen@gmail.com>
srowen pushed a commit that referenced this pull request Jan 15, 2021
…rs tab in standalone cluster mode

### What changes were proposed in this pull request?
Fix  [SPARK-32598] (missing driver logs under UI-ApplicationDetails-Executors tab in standalone cluster mode) .

The direct bug is: the original author forgot to implement `getDriverLogUrls` in `StandaloneSchedulerBackend`

https://github.com/apache/spark/blob/1de272f98d0ff22d0dd151797f22b8faf310963a/core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala#L70-L75

So we set DriverLogUrls as env in `DriverRunner`, and retrieve it at `StandaloneSchedulerBackend`.

### Why are the changes needed?
Fix bug  [SPARK-32598].

### Does this PR introduce _any_ user-facing change?
Yes. User will see driver logs (standalone cluster mode) under UI-ApplicationDetails-Executors tab now.

Before:
![image](https://user-images.githubusercontent.com/17903517/93901055-b5de8600-fd28-11ea-879a-d97e6f70cc6e.png)

After:
![image](https://user-images.githubusercontent.com/17903517/93901080-baa33a00-fd28-11ea-8895-3787c5efbf88.png)

### How was this patch tested?
Re-check the real case in [SPARK-32598] and found this user-facing bug fixed.

Closes #29644 from KevinSmile/kw-dev-master.

Authored-by: KevinSmile <kevinwang013@hotmail.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
(cherry picked from commit c75c29d)
Signed-off-by: Sean Owen <srowen@gmail.com>
@srowen
Copy link
Member

srowen commented Jan 15, 2021

Merged to master/3.1/3.0. It conflicted in 2.4; if we need it there, it needs another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants