Skip to content

[SPARK-29053][WEBUI][2.4] Sort does not work on some columns - #25882

Closed
amanomer wants to merge 3 commits into
apache:branch-2.4from
amanomer:BP29053
Closed

[SPARK-29053][WEBUI][2.4] Sort does not work on some columns#25882
amanomer wants to merge 3 commits into
apache:branch-2.4from
amanomer:BP29053

Conversation

@amanomer

@amanomeramanomer commented Sep 21, 2019•
edited
Loading

Copy link
Copy Markdown
Contributor

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.

Back-port of commit 93ac4e1

@amanomer
[SPARK-29053][WEBUI] Sort does not work on some columns
b1978d2
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)
Closesapache#25855 from amanomer/SPARK29053.
Authored-by: aman_omer <amanomer1996@gmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
@amanomer

amanomer commented Sep 21, 2019

Copy link
Copy Markdown
ContributorAuthor

cc @srowen

@HyukjinKwon

HyukjinKwon commented Sep 21, 2019

Copy link
Copy Markdown
Member

ok to test

@SparkQA

SparkQA commented Sep 21, 2019

Copy link
Copy Markdown

Test build #111121 has finished for PR 25882 at commit b1978d2.

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

@wangyum

wangyum commented Sep 22, 2019

Copy link
Copy Markdown
Member

@amanomer Could you rename PR title to [SPARK-29053][WEBUI][2.4] Sort does not work on some columns?

@amanomeramanomer changed the title [SPARK-29053][WEBUI] Sort does not work on some columns[SPARK-29053][WEBUI][2.4] Sort does not work on some columnsSep 22, 2019
@amanomer

amanomer commented Sep 22, 2019

Copy link
Copy Markdown
ContributorAuthor

Thanks @wangyum

@amanomeramanomer mentioned this pull request Sep 22, 2019
Closed
@wangyum

wangyum commented Sep 22, 2019

Copy link
Copy Markdown
Member

@amanomer Please add [SPARK-28599] to PR title.

@amanomeramanomer changed the title [SPARK-29053][WEBUI][2.4] Sort does not work on some columns[SPARK-29053][SPARK-28599][WEBUI][2.4] Sort does not work on some columnsSep 22, 2019
@SparkQA

SparkQA commented Sep 22, 2019

Copy link
Copy Markdown

Test build #111157 has finished for PR 25882 at commit f788acf.

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

@srowen

srowen commented Sep 22, 2019

Copy link
Copy Markdown
Member

@wangyum does this really address SPARK-28599? this seems to just sort the current page? which brings up an issue of whether that's confusing (or maybe I misunderstood). In any event, this one is just a back-port; does the original PR address it too?

wangyum
wangyum reviewed Sep 22, 2019
View reviewed changes
Comment thread...erver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerSessionPage.scala
<td>{info.groupId}</td>
<td>{formatDate(info.startTimestamp)}</td>
<td>{formatDate(info.finishTimestamp)}</td>
<td>{formatDurationOption(Some(info.totalTime))}</td>

@wangyumwangyumSep 22, 2019•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@srowen This change to backport SPARK-28599.
Or we should revert this change and make another PR to make it clear?

@srowensrowenSep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see, these are really duplicates? I would first merge #25892 then, but, hm, how is it not a merge conflict? I'm missing something.

My larger point was: does this actually mean you sort all of the entries when there are many pages of them? or just the current page?

@wangyumwangyumSep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

#25855 for ThriftServerPage, #25892 for ThriftServerSessionPage. We can support all pages in ThriftServerTab after this PR.

@srowensrowenSep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh I see. Well, want to at least merge #25892 first and then include it here? we can back-port it separately, too.

@dongjoon-hyundongjoon-hyunSep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll review #25892 now and merge it soon. And, +1 for backporting separately.

@amanomeramanomerSep 22, 2019•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ok. I have reverted my last commit and created another PR #25893 to back-port #25892 to branch-2.4.
Kindly review it

@amanomeramanomer changed the title [SPARK-29053][SPARK-28599][WEBUI][2.4] Sort does not work on some columns[SPARK-29053][WEBUI][2.4] Sort does not work on some columnsSep 22, 2019
@amanomer

amanomer commented Sep 22, 2019

Copy link
Copy Markdown
ContributorAuthor

@srowen@dongjoon-hyun

@SparkQA

SparkQA commented Sep 22, 2019

Copy link
Copy Markdown

Test build #111167 has finished for PR 25882 at commit 52edb8d.

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

srowen pushed a commit that referenced this pull request Sep 23, 2019
@amanomer@srowen
[SPARK-29053][WEBUI][2.4] Sort does not work on some columns
bc78f98
### 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](https://issues.apache.org/jira/browse/SPARK-29053).
### Does this PR introduce any user-facing change?
No.
### How was this patch tested?
Test manually.
Back-port of commit 93ac4e1Closes#25882 from amanomer/BP29053.
Authored-by: aman_omer <amanomer1996@gmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
@srowen

srowen commented Sep 23, 2019

Copy link
Copy Markdown
Member

Merged to 2.4

@srowensrowen closed this Sep 23, 2019
@amanomer

amanomer commented Sep 23, 2019

Copy link
Copy Markdown
ContributorAuthor

Thank you @srowen

@dongjoon-hyun

dongjoon-hyun commented Sep 23, 2019

Copy link
Copy Markdown
Member

Thank you all!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srowensrowensrowen left review comments
@wangyumwangyumwangyum left review comments
@dongjoon-hyundongjoon-hyundongjoon-hyun left review comments

Assignees

No one assigned

Labels

WEB UI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@amanomer@HyukjinKwon@SparkQA@wangyum@srowen@dongjoon-hyun