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-26044][WEBUI]Aggregated Metrics table sort based on executor ID #23024

Closed
wants to merge 2 commits into from

Conversation

shahidki31
Copy link
Contributor

@shahidki31 shahidki31 commented Nov 13, 2018

What changes were proposed in this pull request?

Aggregated Metrics table in the stage page is not sorted based on the executorID properly. Because executorID is string and also the logs of the executors are in the same column.
In this PR, I created a new column for executor logs.

How was this patch tested?

Before patch:
screenshot from 2018-11-14 02-05-12

After patch:
screenshot from 2018-11-14 02-05-29

@SparkQA
Copy link

SparkQA commented Nov 14, 2018

Test build #98798 has finished for PR 23024 at commit d6099e3.

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

@SparkQA
Copy link

SparkQA commented Nov 14, 2018

Test build #98800 has finished for PR 23024 at commit ad5669c.

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

@@ -70,6 +70,7 @@ private[ui] class ExecutorTable(stage: StageData, store: AppStatusStore) {
Blacklisted
</span>
</th>
<th>Logs</th>
Copy link
Member

Choose a reason for hiding this comment

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

What about putting the logs link after the ID column, to stay more consistent? or is it that the Logs column is last in other tables?

Copy link
Contributor Author

@shahidki31 shahidki31 Nov 14, 2018

Choose a reason for hiding this comment

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

In other tables, like Executors table, Logs column is in the last. That is why I followed the same here.
screenshot from 2018-11-14 23-29-43

@SparkQA
Copy link

SparkQA commented Nov 14, 2018

Test build #98835 has finished for PR 23024 at commit 58293b3.

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

@SparkQA
Copy link

SparkQA commented Nov 14, 2018

Test build #98834 has finished for PR 23024 at commit 55aebe6.

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

@srowen
Copy link
Member

srowen commented Nov 15, 2018

Merged to master

@asfgit asfgit closed this in cae5879 Nov 15, 2018
@shahidki31
Copy link
Contributor Author

Thank you @srowen

@shahidki31 shahidki31 deleted the AggSort branch November 15, 2018 22:12
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?
Aggregated Metrics table in the stage page is not sorted based on the executorID properly. Because executorID is string and also the logs of the executors are in the same column.
In this PR, I created a new column for executor logs.

## How was this patch tested?
Before patch:
![screenshot from 2018-11-14 02-05-12](https://user-images.githubusercontent.com/23054875/48441529-caa77580-e7b1-11e8-90ea-b16f63438102.png)

After patch:
![screenshot from 2018-11-14 02-05-29](https://user-images.githubusercontent.com/23054875/48441540-d2671a00-e7b1-11e8-9059-890bfe80c961.png)

Closes apache#23024 from shahidki31/AggSort.

Authored-by: Shahid <shahidki31@gmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants