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-25719][UI] : Search functionality in datatables in stages page should search over formatted data rather than the raw data #24419

Closed
wants to merge 5 commits into from

Conversation

pgandhi999
Copy link

@pgandhi999 pgandhi999 commented Apr 19, 2019

The Pull Request to add datatables to stage page SPARK-21809 got merged. The search functionality in those datatables being a great improvement for searching through a large number of tasks, also performs search over the raw data rather than the formatted data displayed in the tables. It would be great if the search can happen for the formatted data as well.

What changes were proposed in this pull request?

Added code to enable searching over displayed data in tables e.g. searching on "165.7 MiB" or "0.3 ms" will now return the search results. Also, earlier we were missing search for two columns in the task table "Shuffle Read Bytes" as well as "Shuffle Remote Reads", which I have added here.

How was this patch tested?

Manual Tests

…uld search over formatted data rather than the raw data

The Pull Request to add datatables to stage page SPARK-21809 got merged. The search functionality in those datatables being a great improvement for searching through a large number of tasks, also performs search over the raw data rather than the formatted data displayed in the tables. It would be great if the search can happen for the formatted data as well.

Added code to enable searching over displayed data in tables e.g. "165.7 MiB" or "0.3 ms"
@pgandhi999
Copy link
Author

ok to test

@pgandhi999
Copy link
Author

@abellina @vanzin @shahidki31

@pgandhi999
Copy link
Author

Have also added searching on Shuffle Read Bytes as well as Shuffle Remote Reads columns which was somehow missed out earlier.

def formatBytes(bytes: Long): String = {
if (bytes == 0) {
return "0.0 B"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, indentation.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Thank you.

|| containsValue(task.taskMetrics.get.shuffleReadMetrics.fetchWaitTime)
|| containsValue(UIUtils.formatDuration(
task.taskMetrics.get.shuffleReadMetrics.fetchWaitTime))
|| containsValue(UIUtils.formatBytes(
Copy link
Contributor

Choose a reason for hiding this comment

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

this is also adding new fields to search as well, no big deal, but should probably add to the description of the pr.

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 Apr 19, 2019

Test build #104762 has finished for PR 24419 at commit a1e72f7.

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

@pgandhi999
Copy link
Author

test this please

@SparkQA
Copy link

SparkQA commented Apr 19, 2019

Test build #104764 has finished for PR 24419 at commit a1e72f7.

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

@felixcheung felixcheung changed the title [SPARK-25719] : Search functionality in datatables in stages page sho… [SPARK-25719] : Search functionality in datatables in stages page should search over formatted data rather than the raw data Apr 20, 2019
@SparkQA
Copy link

SparkQA commented Apr 22, 2019

Test build #104813 has finished for PR 24419 at commit a3a74f7.

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

@@ -145,6 +145,16 @@ private[spark] object UIUtils extends Logging {
}
}

def formatBytes(bytes: Long): String = {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, there is Utils.bytesToString already, but it is using decimal units. In this case it makes sense to format in binary units, as you are trying to match what is shown in the UI for the search. You should probably add a comment above to say it's purpose. How about changing the name to formatBytesBinary?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense, have updated the code. Thank you.

@SparkQA
Copy link

SparkQA commented May 9, 2019

Test build #105294 has finished for PR 24419 at commit 8822c82.

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

@pgandhi999 pgandhi999 changed the title [SPARK-25719] : Search functionality in datatables in stages page should search over formatted data rather than the raw data [SPARK-25719][UI] : Search functionality in datatables in stages page should search over formatted data rather than the raw data May 10, 2019
@SparkQA
Copy link

SparkQA commented May 10, 2019

Test build #105315 has finished for PR 24419 at commit 35f78c2.

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

@SparkQA
Copy link

SparkQA commented May 13, 2019

Test build #105343 has finished for PR 24419 at commit 35f78c2.

  • This patch passes all tests.
  • 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.

👍 thanks @pgandhi999

@srowen
Copy link
Member

srowen commented May 14, 2019

Merged to master

@srowen srowen closed this in 695dbe2 May 14, 2019
@pgandhi999
Copy link
Author

Thank you @srowen and @abellina .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants