Skip to content

Conversation

@pgandhi999
Copy link

Support column sort, pagination and search for Stage Page using jQuery DataTable and REST API. Before this commit, the Stage page generated a hard-coded HTML table that could not support search. Supporting search and sort (over all applications rather than the 20 entries in the current page) in any case will greatly improve the user experience.
Created the stagespage-template.html for displaying application information in datables. Added REST api endpoint and javascript code to fetch data from the endpoint and display it on the data table.
Because of the above change, certain functionalities in the page had to be modified to support the addition of datatables. For example, the toggle checkbox 'Select All' previously would add the checked fields as columns in the Task table and as rows in the Summary Metrics table, but after the change, only columns are added in the Task Table as it got tricky to add rows dynamically in the datatables.

How was this patch tested?

I have attached the screenshots of the Stage Page UI before and after the fix.
Before:

30564304-35991e1c-9c8a-11e7-850f-2ac7a347f600

31360592-cbaa2bae-ad14-11e7-941d-95b4c7d14970

After:

31360591-c5650ee4-ad14-11e7-9665-5a08d8f21830

31360604-d266b6b0-ad14-11e7-94b5-dcc4bb5443f4

@gatorsmile
Copy link
Member

ok to test

@pgandhi999
Copy link
Author

@ajbozarth @vanzin @tgravescs Hello,
After our last discussion in PR #19270, I have made some changes to the datatables, to fetch the data and perform sort, search and pagination on the server side for the tasks table. I request you to review the changes.

Also one known issue with the PR is that currently, the task datatables are displaying all the columns(shuffle read metrics, write metrics, shuffle spill memory and disk etc.) regardless of any data being there or not. As of yet, I could not find a better way to hide those columns and display them only when there is data as the data is coming from the server end so performing client side validation was proving to be tricky. Will work on it later in the course of addressing reviews.

@SparkQA
Copy link

SparkQA commented Jul 1, 2018

Test build #92515 has finished for PR 21688 at commit d79eb80.

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

@SparkQA
Copy link

SparkQA commented Jul 1, 2018

Test build #92516 has finished for PR 21688 at commit 6a0622e.

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

@SparkQA
Copy link

SparkQA commented Jul 2, 2018

Test build #92517 has finished for PR 21688 at commit 132fce9.

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

@ajbozarth
Copy link
Member

I'll see if I get a chance to look at this this week, but with the holiday it may have to wait a week.

@pgandhi999
Copy link
Author

No problem, thank you @ajbozarth

@pgandhi999
Copy link
Author

Hi @ajbozarth did you get a chance to review it yet? Thank you.

@ajbozarth
Copy link
Member

Sorry, I have not. I've been really busy with other projects and haven't been in the spark code for a few months. I'll take a look when/if I have time, but it'd be faster for @vanzin or @tgravescs to review at this point.

@SparkQA
Copy link

SparkQA commented Sep 11, 2018

Test build #95902 has finished for PR 21688 at commit 573390d.

  • This patch fails from timeout after a configured wait of `400m`.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

@abellina abellina left a comment

Choose a reason for hiding this comment

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

@pgandhi999 Thanks for the PR. I know this is going to be very useful, as the tables you are improving are really slow to load and sort currently. This is a first pass, so take a look. I should do another pass in more detail, but let me know what you think about some of these suggestions below.

One general nit: For the Loading modal, I see the whole Stage page dims. Could we instead do this in the specific datatable that is being reloaded?

So instead of .blockUI I think we can select an element, and then .block it. I haven't tried this: http://malsup.com/jquery/block/#element

var stageId = words2[1].substr(3, stageIdLen - 3);
var newBaseURI = words.slice(0, ind + 2).join('/');
return newBaseURI + "/api/v1/applications/" + appId + "/stages/" + stageId;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is an else.. it's either proxy or history. Though I am fairly certain this function can (should) be rewritten as a regex. Maybe even one that can be configured in the future.

Copy link
Author

Choose a reason for hiding this comment

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

I agree with you on the part about if else, however, we do need the value of ind to determine the stage id and attempt in order to construct our REST endpoint url so, the check if(ind > 0) is still needed. Regarding the regex part, I did not completely get your point, do you have a specific example or something? Thanks.

Copy link
Author

Choose a reason for hiding this comment

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

Done

}

function getColumnNameForTaskMetricSummary(columnKey) {
if(columnKey == "executorRunTime") {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd replace with a switch statement.

Copy link
Author

Choose a reason for hiding this comment

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

Done

"<span class='expand-input-rate-arrow arrow-closed' id='arrowtoggle1'></span>" +
" Show Additional Metrics" +
"</a></div>" +
"<div class='container-fluid' id='toggle-metrics' hidden>" +
Copy link
Contributor

Choose a reason for hiding this comment

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

In the current UI, the "Show Additional Metrics" button will go ahead and toggle in "Summary Metrics":

Scheduler Delay
Task Deserialization Time
Result Serialization Time
Getting Result TIme
Peak Execution Memory

But in my local run of this change, the "Summary Metrics" table is static and not changed by the button.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that is because, each time, we show or hide a row, we have to construct the entire datatable again. Not sure if that is what we want, but we can discuss further.

Copy link
Author

Choose a reason for hiding this comment

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

Done

$('#active-tasks-table_filter input').unbind();
$('#active-tasks-table_filter input').bind('keyup', function(e) {
if(e.keyCode == 13) {
taskTableSelector.search( this.value ).draw();
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't expecting the search to wait for me to press enter. I realize this is to do the sorting server side. Did you find that datatables couldn't sort/filter a large task? Since the first load will have the full payload, I am wondering if we should just do front-end sorting/filtering after that

Copy link
Author

Choose a reason for hiding this comment

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

The tasks datatable has been implemented with server-side sorting, pagination and searching. So the data is pushed directly from the server, thus, implementing a front-end filtering functionality is not possible here. Regarding pressing Enter to get the results, I can change it to happen on keypress. Let me know what you think.

Copy link
Author

Choose a reason for hiding this comment

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

I have implemented the search functionality on any keyup rather than Enter in the PR. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pgandhi999 I meant to do this with a timer, so when you finish typing a 5 second timer for example starts (and gets reset on keyup). Only when the timer is up then it should make the fetch.

Copy link
Author

Choose a reason for hiding this comment

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

Done

$.getJSON(endPoint, function(response, status, jqXHR) {

// prepare data for task aggregated metrics table
indices = Object.keys(response[0].executorSummary);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, set var responseBody = response[0], and use responseBody throughout.

Copy link
Author

Choose a reason for hiding this comment

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

Done


var task_metrics_table = [];
var stageAttemptId = getStageAttemptId();
$.getJSON(stageEndPoint(appId) + "/"+stageAttemptId+"/taskSummary?quantiles=0,0.25,0.5,0.75,1.0", function(response1, status, jqXHR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

a lot of code happens inside of this callback. Could each of the tables be broken down into a function? With the callback here reduced to calls to a set of functions?

Copy link
Author

Choose a reason for hiding this comment

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

So this callback takes care of loading the entire task-summary table on client end, and all the tasks depend on the data received in the callback, but we can use functions instead though. Let me know what way works best.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think functions would be clearer IMHO, it is more of a style thing. That way you could send to each function only what it needs.

Copy link
Author

Choose a reason for hiding this comment

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

Done

ordered.skip(offset).max(length).asScala.map(_.toApi).toSeq
}

// Filters task list based on search parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

if the search can happen in UI via datatables, this function should go away.

<span class="additional-metric-title"><em>(De)select All</em></span>
</li>
<li>
<span data-toggle="tooltip"
Copy link
Contributor

Choose a reason for hiding this comment

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

did these tooltips go away?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry about that, added those tooltips again. Thanks.

val ret = ui.store.stageData(stageId, details = details)
var ret = ui.store.stageData(stageId, details = details)
if (ret.nonEmpty) {
for (i <- 0 to (ret.length - 1)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be rewritten as a range loop for (r <- ret)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also think that things like executorSummary.get.get(execId) could be stored somewhere in the stack, e.g. clean up a bit. It's hard to follow what you are setting.

Copy link
Author

Choose a reason for hiding this comment

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

Done

}

private[ui] object ApiHelper {
private[spark] object ApiHelper {
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this change needed?

Copy link
Author

Choose a reason for hiding this comment

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

In order to use the function indexName in StagesResource.scala.

@SparkQA
Copy link

SparkQA commented Sep 14, 2018

Test build #96081 has finished for PR 21688 at commit af83b6e.

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

@SparkQA
Copy link

SparkQA commented Sep 17, 2018

Test build #96146 has finished for PR 21688 at commit 07c1f19.

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

Copy link
Contributor

@tgravescs tgravescs left a comment

Choose a reason for hiding this comment

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

still reviewing in detail, tried it out and left some comments from testing.

It would be nice if we could find a way to leave out the columns not needed. Fo rinstance if we aren't outputting anything leave out the "outupt Size/records" columns

} );

// This function will only parse the URL under certain format
// e.g. https://domain:50509/history/application_1502220952225_59143/stages/stage/?id=0&attempt=0
Copy link
Contributor

Choose a reason for hiding this comment

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

might be nice to add example when used with proxy

Copy link
Author

Choose a reason for hiding this comment

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

Done

// e.g. https://domain:50509/history/application_1502220952225_59143/stages/stage/?id=0&attempt=0
function stageEndPoint(appId) {
var words = document.baseURI.split('/');
var words2 = document.baseURI.split('?');
Copy link
Contributor

Choose a reason for hiding this comment

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

rename words2 to something like queryString

Copy link
Author

Choose a reason for hiding this comment

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

Done

var ind = words.indexOf("proxy");
if (ind > 0) {
var appId = words[ind + 1];
var stageIdLen = words2[1].indexOf('&');
Copy link
Contributor

Choose a reason for hiding this comment

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

seems a bit brittle if the parameters were to change order, could you do a regexp for like id=[0-9]+[^&]*

Copy link
Author

Choose a reason for hiding this comment

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

Done

$.getJSON(stageEndPoint(appId) + "/"+stageAttemptId+"/taskSummary?quantiles="+quantiles, function(taskMetricsResponse, status, jqXHR) {
var taskMetricIndices = Object.keys(taskMetricsResponse);
taskMetricIndices.forEach(function (ix) {
var columnName = getColumnNameForTaskMetricSummary(ix);
Copy link
Contributor

Choose a reason for hiding this comment

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

the summary metrics Metric used to have tooltips on hover

Copy link
Author

Choose a reason for hiding this comment

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

It is somewhat tricky to have tooltips configured for all rows in that particular column, however, we have the same tooltips available in checkboxes and executor summary table columns.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pgandhi999 I think we mentioned switching on ix in the code below, instead of columnName.

Also could we change ix to be columnKey?

{
data: function (row, type) {
switch(row.metric) {
case 'Input Size / Records':
Copy link
Contributor

Choose a reason for hiding this comment

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

missing tooltip

Copy link
Author

Choose a reason for hiding this comment

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

Discussed in previous comment

<th>Output:</th>
<th>Shuffle Read: </th>
<th>Shuffle Write: </th>
<th>Shuffle Spill (Memory): </th>
Copy link
Contributor

Choose a reason for hiding this comment

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

tooltips missing from this and a few of the others

Copy link
Author

Choose a reason for hiding this comment

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

Done

</thead>
<tbody>
</tbody>
</table>
Copy link
Contributor

Choose a reason for hiding this comment

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

summary metrics table shows a bit odd when no tasks have finished, might not be a big deal but could see if we can improve. Basically it doesn't show any rows.

Copy link
Author

Choose a reason for hiding this comment

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

When I tested it, it did show me rows, so if you can share an example, that would be nice. Thank you.

{ "type": "file-size", "targets": 5 }
],
"paging": false,
"searching": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

what is searching for the aggregated metrics table based on? If you search for an integer like 32 (which would match an id) it finds other things but also shows rows with no 32 in it. I also assume the search is based on the unformatted numbers because I tried ot search for 1.2 GB and 1.2 which should have matched on the Input Size but it showed nothing

Copy link
Author

Choose a reason for hiding this comment

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

Yep we can discuss that.

var taskTableSelector = $(taskTable).DataTable(task_conf);
$('#active-tasks-table_filter input').unbind();
$('#active-tasks-table_filter input').bind('keyup', function(e) {
taskTableSelector.search( this.value ).draw();
Copy link
Contributor

Choose a reason for hiding this comment

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

search on the task table seems to do query upon typing, not sure that is what we want, would be better if use completed and hit enter since its doing server side query

Copy link
Author

Choose a reason for hiding this comment

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

Implemented search on keyup with timeout, thank you.

data : function (row, type) {
if ("taskMetrics" in row) {
if (row.taskMetrics.shuffleWriteMetrics.writeTime > 0) {
return type === 'display' ? formatDuration(parseInt(row.taskMetrics.shuffleWriteMetrics.writeTime) / 1000000) : row.taskMetrics.shuffleWriteMetrics.writeTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

write time shows up with 6 decimal points, we probably don't need that precision here:
36.213747 ms

Copy link
Author

Choose a reason for hiding this comment

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

Done

$.extend( $.fn.dataTable.ext.type.order, {
"file-size-pre": ConvertDurationString,

"file-size-asc": function ( a, b ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

file-size name here doesn't match converting duration. if we can change the name to be duration- instead of file-size might make more sense

Copy link
Author

Choose a reason for hiding this comment

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

Done

var task_metrics_table = [];
var stageAttemptId = getStageAttemptId();
var quantiles = "0,0.25,0.5,0.75,1.0";
$.getJSON(stageEndPoint(appId) + "/"+stageAttemptId+"/taskSummary?quantiles="+quantiles, function(taskMetricsResponse, status, jqXHR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap this line at 100 chars if we can

Copy link
Author

Choose a reason for hiding this comment

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

Done

@SparkQA
Copy link

SparkQA commented Nov 19, 2018

Test build #99011 has finished for PR 21688 at commit a696dcc.

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


def makeTimeline(tasks: Seq[TaskData], currentTime: Long): Seq[Node] = {
def makeTimeline(tasks: Seq[TaskData], currentTime: Long, page: Int, pageSize: Int,
totalPages: Int, stageId: Int, stageAttemptId: Int, totalTasks: Int): Seq[Node] = {
Copy link
Contributor

@tgravescs tgravescs Nov 20, 2018

Choose a reason for hiding this comment

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

fix format of arguments to be multi-line style like:

def runJob(
      sc: SparkContext,
      rdd: JavaRDD[Array[Byte]], 
      partitions: JArrayList[Int]): Array[Any] = {

Copy link
Author

Choose a reason for hiding this comment

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

Done

val parameterTaskPage = UIUtils.stripXSS(request.getParameter("task.page"))
val parameterTaskSortColumn = UIUtils.stripXSS(request.getParameter("task.sort"))
val parameterTaskSortDesc = UIUtils.stripXSS(request.getParameter("task.desc"))
val parameterTaskPageSize = UIUtils.stripXSS(request.getParameter("task.pageSize"))
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to clean this up as its not really used anymore, can you file a jira to change the timeline to use rest api or the data from the other tables..

Copy link
Author

Choose a reason for hiding this comment

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

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99076 has finished for PR 21688 at commit b3e2370.

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

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99077 has finished for PR 21688 at commit 672b12a.

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

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99084 has finished for PR 21688 at commit 8f6efbd.

  • This patch fails from timeout after a configured wait of 400m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tgravescs
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99127 has finished for PR 21688 at commit 8f6efbd.

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

@abellina
Copy link
Contributor

The failing suite is OrcQuerySuite . Looks unrelated.

@tgravescs
Copy link
Contributor

Test this please

1 similar comment
@tgravescs
Copy link
Contributor

Test this please

@SparkQA
Copy link

SparkQA commented Nov 26, 2018

Test build #4442 has finished for PR 21688 at commit 8f6efbd.

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

@tgravescs
Copy link
Contributor

+1 , going to merge to master

There are a few followup jiras on this.

  1. make the timeline visualization better: https://issues.apache.org/jira/browse/SPARK-26130
  2. improve search functionalit: https://issues.apache.org/jira/browse/SPARK-25719

@asfgit asfgit closed this in 76ef02e Nov 26, 2018
srowen pushed a commit that referenced this pull request Dec 30, 2018
…kList`

## What changes were proposed in this pull request?

In the method `taskList`(since #21688),  the executor log value is queried in KV store  for every task(method `constructTaskData`).
This PR propose to use a hashmap for reducing duplicated KV store lookups in the method.

![image](https://user-images.githubusercontent.com/1097932/49946230-841c7680-ff29-11e8-8b83-d8f7553bfe5e.png)

## How was this patch tested?

Manual check

Closes #23310 from gengliangwang/removeExecutorLog.

Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
…columns and searching

Support column sort, pagination and search for Stage Page using jQuery DataTable and REST API. Before this commit, the Stage page generated a hard-coded HTML table that could not support search. Supporting search and sort (over all applications rather than the 20 entries in the current page) in any case will greatly improve the user experience.
Created the stagespage-template.html for displaying application information in datables. Added REST api endpoint and javascript code to fetch data from the endpoint and display it on the data table.
Because of the above change, certain functionalities in the page had to be modified to support the addition of datatables. For example, the toggle checkbox 'Select All' previously would add the checked fields as columns in the Task table and as rows in the Summary Metrics table, but after the change, only columns are added in the Task Table as it got tricky to add rows dynamically in the datatables.

## How was this patch tested?
I have attached the screenshots of the Stage Page UI before and after the fix.
**Before:**

<img width="1419" alt="30564304-35991e1c-9c8a-11e7-850f-2ac7a347f600" src="https://user-images.githubusercontent.com/22228190/42137915-52054558-7d3a-11e8-8c85-433b2c94161d.png">

<img width="1435" alt="31360592-cbaa2bae-ad14-11e7-941d-95b4c7d14970" src="https://user-images.githubusercontent.com/22228190/42137928-79df500a-7d3a-11e8-9068-5630afe46ff3.png">

**After:**

<img width="1432" alt="31360591-c5650ee4-ad14-11e7-9665-5a08d8f21830" src="https://user-images.githubusercontent.com/22228190/42137936-a3fb9f42-7d3a-11e8-8502-22b3897cbf64.png">

<img width="1388" alt="31360604-d266b6b0-ad14-11e7-94b5-dcc4bb5443f4" src="https://user-images.githubusercontent.com/22228190/42137970-0fabc58c-7d3b-11e8-95ad-383b1bd1f106.png">

Closes apache#21688 from pgandhi999/SPARK-21809-2.3.

Authored-by: pgandhi <[email protected]>
Signed-off-by: Thomas Graves <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
…kList`

## What changes were proposed in this pull request?

In the method `taskList`(since apache#21688),  the executor log value is queried in KV store  for every task(method `constructTaskData`).
This PR propose to use a hashmap for reducing duplicated KV store lookups in the method.

![image](https://user-images.githubusercontent.com/1097932/49946230-841c7680-ff29-11e8-8b83-d8f7553bfe5e.png)

## How was this patch tested?

Manual check

Closes apache#23310 from gengliangwang/removeExecutorLog.

Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
HyukjinKwon pushed a commit that referenced this pull request Nov 26, 2020
### What changes were proposed in this pull request?

1. Remove the fixed width style of class `container-fluid-div`. So that the UI looks clean when the text is long.
2. Add one space between a checkbox and the text on the right side, which is consistent with the stage page.

### Why are the changes needed?

The width of class `container-fluid-div` is set as 200px after #21688 . This makes the checkbox in the executor page messy.
![image](https://user-images.githubusercontent.com/1097932/100242069-3bc5ab80-2ee9-11eb-8c7d-96c221398fee.png)

We should remove the width limit.

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

No
### How was this patch tested?

Manual test.
After the changes:
![image](https://user-images.githubusercontent.com/1097932/100257802-2f4a4e80-2efb-11eb-9eb0-92d6988ad14b.png)

Closes #30500 from gengliangwang/reviseStyle.

Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
LuciferYang added a commit that referenced this pull request Jan 29, 2025
…eSuite.scala`

### What changes were proposed in this pull request?
This pr removes a test file `StagePageSuite.scala` that is no longer needed.

### Why are the changes needed?
The current `StagePageSuite.scala` contains one test case and one private method. The reasons for cleaning them up are as follows:

1. The assertion in the test case "ApiHelper.COLUMN_TO_INDEX should match headers of the task table" was cleaned up in SPARK-44490 | #42085. Currently, this test case only creates a `StageData` object and an `AppStatusStore` object, with no other operations or assertions. Moreover, the creation of these objects is also covered in other test cases, such as `KVStoreProtobufSerializerSuite`, `AppStatusStoreSuite`, etc. Therefore, this is already an unnecessary test case and can be cleaned up.

2. The private method `renderStagePage` has not been used since SPARK-21809 | #21688, so it can be cleaned up.

After cleaning up the above content, `StagePageSuite.scala` becomes an empty file, so it can be completely deleted.

### 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 #49721 from LuciferYang/SPARK-51026.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
LuciferYang added a commit that referenced this pull request Jan 29, 2025
…eSuite.scala`

### What changes were proposed in this pull request?
This pr removes a test file `StagePageSuite.scala` that is no longer needed.

### Why are the changes needed?
The current `StagePageSuite.scala` contains one test case and one private method. The reasons for cleaning them up are as follows:

1. The assertion in the test case "ApiHelper.COLUMN_TO_INDEX should match headers of the task table" was cleaned up in SPARK-44490 | #42085. Currently, this test case only creates a `StageData` object and an `AppStatusStore` object, with no other operations or assertions. Moreover, the creation of these objects is also covered in other test cases, such as `KVStoreProtobufSerializerSuite`, `AppStatusStoreSuite`, etc. Therefore, this is already an unnecessary test case and can be cleaned up.

2. The private method `renderStagePage` has not been used since SPARK-21809 | #21688, so it can be cleaned up.

After cleaning up the above content, `StagePageSuite.scala` becomes an empty file, so it can be completely deleted.

### 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 #49721 from LuciferYang/SPARK-51026.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
(cherry picked from commit 3badfd2)
Signed-off-by: yangjie01 <[email protected]>
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 14, 2025
…eSuite.scala`

### What changes were proposed in this pull request?
This pr removes a test file `StagePageSuite.scala` that is no longer needed.

### Why are the changes needed?
The current `StagePageSuite.scala` contains one test case and one private method. The reasons for cleaning them up are as follows:

1. The assertion in the test case "ApiHelper.COLUMN_TO_INDEX should match headers of the task table" was cleaned up in SPARK-44490 | apache#42085. Currently, this test case only creates a `StageData` object and an `AppStatusStore` object, with no other operations or assertions. Moreover, the creation of these objects is also covered in other test cases, such as `KVStoreProtobufSerializerSuite`, `AppStatusStoreSuite`, etc. Therefore, this is already an unnecessary test case and can be cleaned up.

2. The private method `renderStagePage` has not been used since SPARK-21809 | apache#21688, so it can be cleaned up.

After cleaning up the above content, `StagePageSuite.scala` becomes an empty file, so it can be completely deleted.

### 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 apache#49721 from LuciferYang/SPARK-51026.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
(cherry picked from commit fb742ff)
Signed-off-by: yangjie01 <[email protected]>
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.

10 participants