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-25502][CORE][WEBUI]Empty Page when page number exceeds the reatinedTask size. #22526
Conversation
6204cbe
to
af8ff45
Compare
cc @vanzin |
af8ff45
to
8309722
Compare
override def dataSize: Int = { | ||
val storedTasks = store.taskCount(stage.stageId, stage.attemptId).toInt | ||
val totalTasks = taskCount(stage) | ||
if (totalTasks > storedTasks) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just write math.min(storedTasks, totalTasks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. totalTasks will be always greater than or equal to storedTasks. We can simply return storedTasks. But for better understanding I have put it in the if else condition.
I have modified the code based on your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hm I would have thought it's simpler to return the single value that this always takes on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I have updated the PR by returning only storedTasks. Kindly look into the updated code.
8309722
to
8e7f87f
Compare
8e7f87f
to
4f37ca8
Compare
Test build #4349 has started for PR 22526 at commit |
@@ -685,7 +685,10 @@ private[ui] class TaskDataSource( | |||
|
|||
private var _tasksToShow: Seq[TaskData] = null | |||
|
|||
override def dataSize: Int = taskCount(stage) | |||
override def dataSize: Int = { | |||
val storedTasks = store.taskCount(stage.stageId, stage.attemptId).toInt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need the intermediate variable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks
4f37ca8
to
4444fa4
Compare
@@ -685,7 +685,7 @@ private[ui] class TaskDataSource( | |||
|
|||
private var _tasksToShow: Seq[TaskData] = null | |||
|
|||
override def dataSize: Int = taskCount(stage) | |||
override def dataSize: Int = store.taskCount(stage.stageId, stage.attemptId).toInt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: after this change, the function taskCount()
is only referenced by totalTasks
, we can inline that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Done.
9a33c34
to
ca8d4f2
Compare
Merging to master / 2.4 / 2.3. |
…atinedTask size. ## What changes were proposed in this pull request? Test steps : 1) bin/spark-shell --conf spark.ui.retainedTasks=200 ``` val rdd = sc.parallelize(1 to 1000, 1000) rdd.count ``` Stage tab in the UI will display 10 pages with 100 tasks per page. But number of retained tasks is only 200. So, from the 3rd page onwards will display nothing. We have to calculate total pages based on the number of tasks need display in the UI. **Before fix:** ![empty_4](https://user-images.githubusercontent.com/23054875/45918251-b1650580-bea1-11e8-90d3-7e0d491981a2.jpg) **After fix:** ![empty_3](https://user-images.githubusercontent.com/23054875/45918257-c2ae1200-bea1-11e8-960f-dfbdb4a90ae7.jpg) ## How was this patch tested? Manually tested Closes #22526 from shahidki31/SPARK-25502. Authored-by: Shahid <shahidki31@gmail.com> Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com> (cherry picked from commit 3ce2e00) Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
…atinedTask size. ## What changes were proposed in this pull request? Test steps : 1) bin/spark-shell --conf spark.ui.retainedTasks=200 ``` val rdd = sc.parallelize(1 to 1000, 1000) rdd.count ``` Stage tab in the UI will display 10 pages with 100 tasks per page. But number of retained tasks is only 200. So, from the 3rd page onwards will display nothing. We have to calculate total pages based on the number of tasks need display in the UI. **Before fix:** ![empty_4](https://user-images.githubusercontent.com/23054875/45918251-b1650580-bea1-11e8-90d3-7e0d491981a2.jpg) **After fix:** ![empty_3](https://user-images.githubusercontent.com/23054875/45918257-c2ae1200-bea1-11e8-960f-dfbdb4a90ae7.jpg) ## How was this patch tested? Manually tested Closes #22526 from shahidki31/SPARK-25502. Authored-by: Shahid <shahidki31@gmail.com> Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com> (cherry picked from commit 3ce2e00) Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
…atinedTask size. ## What changes were proposed in this pull request? Test steps : 1) bin/spark-shell --conf spark.ui.retainedTasks=200 ``` val rdd = sc.parallelize(1 to 1000, 1000) rdd.count ``` Stage tab in the UI will display 10 pages with 100 tasks per page. But number of retained tasks is only 200. So, from the 3rd page onwards will display nothing. We have to calculate total pages based on the number of tasks need display in the UI. **Before fix:** ![empty_4](https://user-images.githubusercontent.com/23054875/45918251-b1650580-bea1-11e8-90d3-7e0d491981a2.jpg) **After fix:** ![empty_3](https://user-images.githubusercontent.com/23054875/45918257-c2ae1200-bea1-11e8-960f-dfbdb4a90ae7.jpg) ## How was this patch tested? Manually tested Closes apache#22526 from shahidki31/SPARK-25502. Authored-by: Shahid <shahidki31@gmail.com> Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
What changes were proposed in this pull request?
Test steps :
Stage tab in the UI will display 10 pages with 100 tasks per page. But number of retained tasks is only 200. So, from the 3rd page onwards will display nothing.
We have to calculate total pages based on the number of tasks need display in the UI.
Before fix:
After fix:
How was this patch tested?
Manually tested