Skip to content

Conversation

@amanomer
Copy link
Contributor

@amanomer amanomer commented Sep 19, 2019

What changes were proposed in this pull request?

Setting custom sort key for duration and execution time column.

Why are the changes needed?

Sorting on duration and execution time columns consider time as a string after converting into readable form which is the reason for wrong sort results as mentioned in SPARK-29053.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Test manually. Screenshots are attached.

After patch:
Duration
Duration
Execution time
Execution Time

@wangyum
Copy link
Member

wangyum commented Sep 19, 2019

ok to test

@SparkQA
Copy link

SparkQA commented Sep 19, 2019

Test build #111011 has finished for PR 25855 at commit f36a618.

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

@amanomer
Copy link
Contributor Author

cc @srowen @gatorsmile

Copy link
Member

@srowen srowen 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, but do other pages have a similar problem too? I would not be surprised.

@amanomer
Copy link
Contributor Author

No, I haven't found this issue anywhere else.

@amanomer
Copy link
Contributor Author

thanks @srowen

@SparkQA
Copy link

SparkQA commented Sep 19, 2019

Test build #4879 has finished for PR 25855 at commit f36a618.

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

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.

Hi, @amanomer . Thank you for your contribution. I'd like to recommend the followings first.

  1. Please attach your PR's screenshot. The current one is your internal one as you see the Spark version 2.3.2.020x.
  2. While you are here, could you fix the Execution Time which is the very left column to this? Execution Time has the same issue. If you build your PR branch, you can see that column.

After fixing the above two, you need to update the PR title and description and JIRA accordingly, too. Thanks!

@amanomer
Copy link
Contributor Author

@dongjoon-hyun thanks for your feedback.
I am working on this.

@dongjoon-hyun
Copy link
Member

Thanks, @amanomer . I'll review again after you update this PR.

@amanomer amanomer changed the title [SPARK-29053][WEBUI] Spark UI JDBC/ODBC Server tab sorting is not working on Duration column [SPARK-29053][WEBUI] Sort does not work on some columns Sep 20, 2019
Copy link
Member

@srowen srowen 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.

@SparkQA
Copy link

SparkQA commented Sep 20, 2019

Test build #111079 has finished for PR 25855 at commit d17c7b6.

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

@srowen srowen closed this in 93ac4e1 Sep 21, 2019
@srowen
Copy link
Member

srowen commented Sep 21, 2019

Merged to master. It didn't pick cleanly into 2.4. If we need it there, open another PR with the back-port.

@amanomer
Copy link
Contributor Author

Thanks @srowen .
I will open another PR to back-port this commit in 2.4.

amanomer added a commit to amanomer/spark that referenced this pull request Sep 21, 2019
Setting custom sort key for duration and execution time column.

Sorting on duration and execution time columns consider time as a string after converting into readable form which is the reason for wrong sort results as mentioned in [SPARK-29053](https://issues.apache.org/jira/browse/SPARK-29053).

No

Test manually. Screenshots are attached.

After patch:
**Duration**
![Duration](https://user-images.githubusercontent.com/40591404/65339861-93cc9800-dbea-11e9-95e6-63b107a5a372.png)
**Execution time**
![Execution Time](https://user-images.githubusercontent.com/40591404/65339870-97601f00-dbea-11e9-9d1d-690c59bc1bde.png)

Closes apache#25855 from amanomer/SPARK29053.

Authored-by: aman_omer <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@dongjoon-hyun
Copy link
Member

Thank you, @amanomer and @srowen !

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.

5 participants