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-15591] [WEBUI] Paginate Stage Table in Stages tab #13708

Closed
wants to merge 6 commits into from

Conversation

nblintao
Copy link
Contributor

What changes were proposed in this pull request?

This patch adds pagination support for the Stage Tables in the Stage tab. Pagination is provided for all of the four Job Tables (active, pending, completed, and failed). Besides, the paged stage tables are also used in JobPage (the detail page for one job) and PoolPage.

Interactions (jumping, sorting, and setting page size) for paged tables are also included.

How was this patch tested?

Tested manually by using checking the Web UI after completing and failing hundreds of jobs. Same as the testings for Paginate Job Table in Jobs tab.

This shows the pagination for completed stages:
paged stage table

@SparkQA
Copy link

SparkQA commented Jun 16, 2016

Test build #60645 has finished for PR 13708 at commit 4255c09.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 16, 2016

Test build #60646 has finished for PR 13708 at commit 4de6d68.

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

@ajbozarth
Copy link
Member

I haven't been able to look at the code yet but I checked it out and ran it and found a bug. When you change one table (completed or failed) it resets the other table. I'll also check if this happens on your jobs page pr as well

@nblintao
Copy link
Contributor Author

@ajbozarth Thanks for pointing it out. It is true for both the stages page and the job page. It doesn't keep track of the status of other tables when one table is changed.

I will try to fix it recently.

@nblintao
Copy link
Contributor Author

@ajbozarth Could you please try again to see whether or not have my new commit fixed the problem you mentioned above?
Also, regarding the bug you mentioned in #13620, are the stage tables working properly in the history servers?
Thanks!

@ajbozarth
Copy link
Member

Thanks, I'll check it out when I have a chance today. And I'll double check the history server, but I'm pretty sure it was both.

@SparkQA
Copy link

SparkQA commented Jun 21, 2016

Test build #60945 has finished for PR 13708 at commit 246432d.

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

@SparkQA
Copy link

SparkQA commented Jun 22, 2016

Test build #61046 has started for PR 13708 at commit 7ccda86.

@nblintao
Copy link
Contributor Author

I have fixed some linking bugs in stage tables (including the history server problem reported by @ajbozarth ). I have also prevented displaying empty tables in pool pages.
I have tested the links for the AllStagesPage and the JobPage in both the normal UI and the history UI, and tested the PoolPage in the normal UI.
Please have a review of my code and check if it works properly. I will fix the corresponding bugs in #13620 after this PR is merged.

@SparkQA
Copy link

SparkQA commented Jun 22, 2016

Test build #61058 has finished for PR 13708 at commit 4af0630.

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

@ajbozarth
Copy link
Member

I finally had a chance to checkout and test your updates and it looks great on both Web UI and History server (I did't check Pool Page because I've never used it). One small thing though, how are the >> and << arrows supposed to work? Because my assumption would be that they go to the first or last page overall, but you have them implemented to skip 10 pages forward or backward. Just wondering if this was your intent and if so why?

@nblintao
Copy link
Contributor Author

@ajbozarth Thanks. That was implemented in the pageNavigation of the PagedTable by @zsxwing from PR #7399. I didn't change it, and it seems useful to me. It works like a pagination for the page navigator itself, since the navigator displays 10 links at once.

@nblintao
Copy link
Contributor Author

Jenkins, retest this please

@ajbozarth
Copy link
Member

@nblintao thanks for that PR link, that had a great explanation for why he decided it should work that way.

I haven't looked at the code yet, but from a UI perspective this is great.

@SparkQA
Copy link

SparkQA commented Jun 23, 2016

Test build #61086 has finished for PR 13708 at commit 4af0630.

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

parent.basePath, parent.progressListener, isFairScheduler = parent.isFairScheduler,
killEnabled = false)
new StageTableBase(request, pendingStages, "pendingStage", parent.basePath, subPath,
parent.progressListener, parent.isFairScheduler, false, false)
Copy link
Member

@zsxwing zsxwing Jul 5, 2016

Choose a reason for hiding this comment

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

nit: could you add the parameter names for falses back to improve the readability?

@zsxwing
Copy link
Member

zsxwing commented Jul 5, 2016

@nblintao thanks for your contribution. Looks pretty good except some nits. Could you fix them, please?

@SparkQA
Copy link

SparkQA commented Jul 6, 2016

Test build #61817 has finished for PR 13708 at commit 3a98c4d.

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

@nblintao
Copy link
Contributor Author

nblintao commented Jul 6, 2016

Thanks, @zsxwing . I have fixed them in the commit above.
P.S.:
My computer's time setting was incorrect when it is committed, which lead to the weird order on the GitHub page of this PR (the commit appear below the test and this message). To be clear, the commit I am saying is 3a98c4d.

@zsxwing
Copy link
Member

zsxwing commented Jul 6, 2016

LGTM. @nblintao Thanks. Merging to master.

@asfgit asfgit closed this in 478b71d Jul 6, 2016
asfgit pushed a commit that referenced this pull request Dec 24, 2016
## What changes were proposed in this pull request?

This issue was reported by wangyum.

In the AllJobsPage, JobPage and StagePage, the description length was limited before like as follows.

![ui-2 0 0](https://cloud.githubusercontent.com/assets/4736016/21319673/8b225246-c651-11e6-9041-4fcdd04f4dec.gif)

But recently, the limitation seems to have been accidentally removed.

![ui-2 1 0](https://cloud.githubusercontent.com/assets/4736016/21319825/104779f6-c652-11e6-8bfa-dfd800396352.gif)

The cause is that some tables are no longer `sortable` class although they were, and `sortable` class does not only mark tables as sortable but also limited the width of their child `td` elements.
The reason why now some tables are not `sortable` class is because another sortable mechanism was introduced by #13620 and #13708 with pagination feature.

To fix this issue, I've introduced new class `table-cell-width-limited` which limits the description cell width and the description is like what it was.

<img width="1260" alt="2016-12-20 1 00 34" src="https://cloud.githubusercontent.com/assets/4736016/21320478/89141c7a-c654-11e6-8494-f8f91325980b.png">

## How was this patch tested?

Tested manually with my browser.

Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp>

Closes #16338 from sarutak/SPARK-18837.
asfgit pushed a commit that referenced this pull request Dec 24, 2016
## What changes were proposed in this pull request?

This issue was reported by wangyum.

In the AllJobsPage, JobPage and StagePage, the description length was limited before like as follows.

![ui-2 0 0](https://cloud.githubusercontent.com/assets/4736016/21319673/8b225246-c651-11e6-9041-4fcdd04f4dec.gif)

But recently, the limitation seems to have been accidentally removed.

![ui-2 1 0](https://cloud.githubusercontent.com/assets/4736016/21319825/104779f6-c652-11e6-8bfa-dfd800396352.gif)

The cause is that some tables are no longer `sortable` class although they were, and `sortable` class does not only mark tables as sortable but also limited the width of their child `td` elements.
The reason why now some tables are not `sortable` class is because another sortable mechanism was introduced by #13620 and #13708 with pagination feature.

To fix this issue, I've introduced new class `table-cell-width-limited` which limits the description cell width and the description is like what it was.

<img width="1260" alt="2016-12-20 1 00 34" src="https://cloud.githubusercontent.com/assets/4736016/21320478/89141c7a-c654-11e6-8494-f8f91325980b.png">

## How was this patch tested?

Tested manually with my browser.

Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp>

Closes #16338 from sarutak/SPARK-18837.

(cherry picked from commit f2ceb2a)
Signed-off-by: Sean Owen <sowen@cloudera.com>
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Dec 26, 2016
## What changes were proposed in this pull request?

This issue was reported by wangyum.

In the AllJobsPage, JobPage and StagePage, the description length was limited before like as follows.

![ui-2 0 0](https://cloud.githubusercontent.com/assets/4736016/21319673/8b225246-c651-11e6-9041-4fcdd04f4dec.gif)

But recently, the limitation seems to have been accidentally removed.

![ui-2 1 0](https://cloud.githubusercontent.com/assets/4736016/21319825/104779f6-c652-11e6-8bfa-dfd800396352.gif)

The cause is that some tables are no longer `sortable` class although they were, and `sortable` class does not only mark tables as sortable but also limited the width of their child `td` elements.
The reason why now some tables are not `sortable` class is because another sortable mechanism was introduced by apache#13620 and apache#13708 with pagination feature.

To fix this issue, I've introduced new class `table-cell-width-limited` which limits the description cell width and the description is like what it was.

<img width="1260" alt="2016-12-20 1 00 34" src="https://cloud.githubusercontent.com/assets/4736016/21320478/89141c7a-c654-11e6-8494-f8f91325980b.png">

## How was this patch tested?

Tested manually with my browser.

Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp>

Closes apache#16338 from sarutak/SPARK-18837.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?

This issue was reported by wangyum.

In the AllJobsPage, JobPage and StagePage, the description length was limited before like as follows.

![ui-2 0 0](https://cloud.githubusercontent.com/assets/4736016/21319673/8b225246-c651-11e6-9041-4fcdd04f4dec.gif)

But recently, the limitation seems to have been accidentally removed.

![ui-2 1 0](https://cloud.githubusercontent.com/assets/4736016/21319825/104779f6-c652-11e6-8bfa-dfd800396352.gif)

The cause is that some tables are no longer `sortable` class although they were, and `sortable` class does not only mark tables as sortable but also limited the width of their child `td` elements.
The reason why now some tables are not `sortable` class is because another sortable mechanism was introduced by apache#13620 and apache#13708 with pagination feature.

To fix this issue, I've introduced new class `table-cell-width-limited` which limits the description cell width and the description is like what it was.

<img width="1260" alt="2016-12-20 1 00 34" src="https://cloud.githubusercontent.com/assets/4736016/21320478/89141c7a-c654-11e6-8494-f8f91325980b.png">

## How was this patch tested?

Tested manually with my browser.

Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp>

Closes apache#16338 from sarutak/SPARK-18837.
dongjoon-hyun pushed a commit that referenced this pull request Nov 12, 2023
### What changes were proposed in this pull request?
SPARK-15591(#13708) introduced the `MissingStageTableRowData`, but it is no longer used after SPARK-20648(#19698), so this PR removes it.

### Why are the changes needed?
Clean up unused code.

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

### How was this patch tested?
Pass GitHub Actions

### Was this patch authored or co-authored using generative AI tooling?
No

Closes #43748 from LuciferYang/SPARK-45875.

Authored-by: yangjie01 <yangjie01@baidu.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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants