You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/18 14:03:07 UTC
[GitHub] [spark] kyoty opened a new pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
kyoty opened a new pull request #32223:
URL: https://github.com/apache/spark/pull/32223
### What changes were proposed in this pull request?
To make sure that pageSize shoud not be shared between different stage pages.
The screenshots of the problem are placed in the attachment of [JIRA](https://issues.apache.org/jira/browse/SPARK-35127)
in util.js, a config: `stateSave` will be set to true:
```javascript
function setDataTableDefaults() {
$.extend($.fn.dataTable.defaults, {
stateSave: true,
stateSaveParams: function(_, data) {
data.search.search = "";
},
lengthMenu: [[20, 40, 60, 100, -1], [20, 40, 60, 100, "All"]],
pageLength: 20
});
}
```
according to reference:https://datatables.net/reference/option/stateSave#Description
The browser will store state information such as pagination position, display length, filtering and sorting if `sateSave` is true.
active-tasks-table has a dynamic entry option:
```javascript
var taskTable = "#active-tasks-table";
var taskConf = {
"serverSide": true,
"paging": true,
"info": true,
"processing": true,
"lengthMenu": [[20, 40, 60, 100, totalTasksToShow], [20, 40, 60, 100, "All"]],
...
```
If the browser caches the `totalTasksToShow` of the previous page, **the new page would use this expired totalTasksToShow, which will cause the entry item to display empty.**
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
manual test, it is a small io problem, and the modification does not affect the function, but just an adjustment of js configuration
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822236140
@kyoty
> manual test, it is a small io problem, and the modification does not affect the function, but just an adjustment of js configuration
When test is done by manual, could you add code snippets to allow reviewers to check what and how you confirmed?
Also, could you add screenshots which shows before and after the change if the change affects appearance?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822156269
cc @sarutak @gengliangwang
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-824778269
Merged to `master`, `branch-3.1` and `branch-3.0`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822102275
@kyoty looks like here has the same problem .. can you rebase your branch to the latest master branch to sync?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822278569
Sorry @sarutak ,I added two GIFs in the main descption above to show how the problem can be produced and what happened after I modified it.
You can also visit the following url to see the details:
reproduce:
https://user-images.githubusercontent.com/52202080/115204351-f7060f80-a12a-11eb-8900-a009ad0c8870.gif
fix:
https://user-images.githubusercontent.com/52202080/115204886-91fee980-a12b-11eb-9ccb-d5900a99095d.gif
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on a change in pull request #32223: [WIP][SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on a change in pull request #32223:
URL: https://github.com/apache/spark/pull/32223#discussion_r616504336
##########
File path: core/src/main/resources/org/apache/spark/ui/static/stagepage.js
##########
@@ -747,6 +747,7 @@ $(document).ready(function () {
"info": true,
"processing": true,
"lengthMenu": [[20, 40, 60, 100, totalTasksToShow], [20, 40, 60, 100, "All"]],
Review comment:
@sarutak done this and munual test works well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822996638
`stateSave` doesn't affect the visibility of DAG in the stage page. It's controlled by `window.localStorage` directly.
But it affects which columns are shown in the datatable for tasks.
So, I don't think it's a good idea to set `stateSave` tor `false`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on a change in pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on a change in pull request #32223:
URL: https://github.com/apache/spark/pull/32223#discussion_r616378275
##########
File path: core/src/main/resources/org/apache/spark/ui/static/stagepage.js
##########
@@ -747,6 +747,7 @@ $(document).ready(function () {
"info": true,
"processing": true,
"lengthMenu": [[20, 40, 60, 100, totalTasksToShow], [20, 40, 60, 100, "All"]],
Review comment:
@sarutak Great advice! I am not familiar with `lengthMenu`. I just checked the reference https://datatables.net/reference/option/lengthMenu，set `totalTasksToShow ` to `-` is really the best solution
##########
File path: core/src/main/resources/org/apache/spark/ui/static/stagepage.js
##########
@@ -747,6 +747,7 @@ $(document).ready(function () {
"info": true,
"processing": true,
"lengthMenu": [[20, 40, 60, 100, totalTasksToShow], [20, 40, 60, 100, "All"]],
Review comment:
@sarutak Great advice! I am not familiar with `lengthMenu`. I just checked the reference https://datatables.net/reference/option/lengthMenu，set `totalTasksToShow ` to `-1` is really the best solution
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty edited a comment on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty edited a comment on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822278569
Sorry @sarutak ,I have added two GIFs in the main descption above to show how the problem can be produced and what happened after I modified it.
You can also visit the following url to see the details:
reproduce:
https://user-images.githubusercontent.com/52202080/115204351-f7060f80-a12a-11eb-8900-a009ad0c8870.gif
fix:
https://user-images.githubusercontent.com/52202080/115204886-91fee980-a12b-11eb-9ccb-d5900a99095d.gif
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822156189
Thanks @HyukjinKwon , I'll pay more attention to this problem..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822559743
@kyoty It seems that you remove the "Why are the changes needed?" section in the PR description.
Also, the state saving behavior exists in Spark. Spark save the state of whether showing DAG in job/stage page or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-826525420
@sarutak Thanks for your review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-826525420
@sarutak Thanks for your review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #32223:
URL: https://github.com/apache/spark/pull/32223#discussion_r616374758
##########
File path: core/src/main/resources/org/apache/spark/ui/static/stagepage.js
##########
@@ -747,6 +747,7 @@ $(document).ready(function () {
"info": true,
"processing": true,
"lengthMenu": [[20, 40, 60, 100, totalTasksToShow], [20, 40, 60, 100, "All"]],
Review comment:
Why not change `totalTasksToShow` to `-1` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kyoty commented on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
kyoty commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-823004730
@sarutak , thanks for your suggestion. How about custom the stateSave load function? like below:
```javascript
"stateLoadCallback": function(settings) {
var storageData = JSON.parse( localStorage.getItem(settings.sInstance) )};
if(storageData.length) {
var savedPageSize = storageData['length'];
// if savedPageSize doesn't in lengthMenu, we just simply set current pageSize = totalTasksToShow
if( !settings.lengthMenu.contains(savedPageSize )) {
savedPageSize = totalTasksToShow;
}
}
...............
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak closed pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
sarutak closed pull request #32223:
URL: https://github.com/apache/spark/pull/32223
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822102275
@kyoty looks like here has the same problem .. can you rebate your branch to the latest master branch to sync?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32223: [SPARK-35127]When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-821996801
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak edited a comment on pull request #32223: [SPARK-35127][UI] When we switch between different stage-detail pages, the entry item in the newly-opened page may be blank.
Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #32223:
URL: https://github.com/apache/spark/pull/32223#issuecomment-822996638
`stateSave` doesn't affect the visibility of DAG in the stage page. It's controlled by `window.localStorage` directly.
But it affects which columns are shown in the datatable for tasks.
So, I don't think it's a good idea to set `stateSave` to `false`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org