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-20630] [Web UI] Fixed column visibility in Executor Tab #17904

Closed
wants to merge 1 commit into from

Conversation

ajbozarth
Copy link
Member

What changes were proposed in this pull request?

#14617 added new columns to the executor table causing the visibility checks for the logs and threadDump columns to toggle the wrong columns since they used hard-coded column numbers.

I've updated the checks to use column names instead of numbers so future updates don't accidentally break this again.

Note: This will also need to be back ported into 2.2 since #14617 was merged there

How was this patch tested?

Manually tested

@SparkQA
Copy link

SparkQA commented May 9, 2017

Test build #76594 has finished for PR 17904 at commit 766bfb0.

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

@ajbozarth
Copy link
Member Author

Jenkins, retest this please

@felixcheung
Copy link
Member

Please rebase to pick up fix for R tests

@felixcheung
Copy link
Member

This is kinda weird - I don't know why it's running R tests

@SparkQA
Copy link

SparkQA commented May 9, 2017

Test build #76607 has finished for PR 17904 at commit 766bfb0.

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

@ajbozarth
Copy link
Member Author

Jenkins, retest this please

@ajbozarth
Copy link
Member Author

I'm not sure why it's failing those tests, plus my branch is up to date with master (minus one unrelated commit)

@SparkQA
Copy link

SparkQA commented May 9, 2017

Test build #76620 has started for PR 17904 at commit 766bfb0.

@jaceklaskowski
Copy link
Contributor

WFM. Thanks @ajbozarth!

$ git fetch origin pull/17904/head:17904
$ gco 17904
$ ./build/mvn -Phadoop-2.7,yarn,mesos,hive,hive-thriftserver -DskipTests clean install

The following worked fine (http://localhost:4040/executors/):

$ ./bin/spark-shell
$ ./bin/spark-shell --conf spark.ui.threadDumpsEnabled=true
$ ./bin/spark-shell --conf spark.ui.threadDumpsEnabled=false

Copy link
Contributor

@jaceklaskowski jaceklaskowski left a comment

Choose a reason for hiding this comment

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

Worked fine! Thanks.

@ajbozarth
Copy link
Member Author

Jenkins, retest this please

@ajbozarth
Copy link
Member Author

weird test failure and thanks for the review @jaceklaskowski

@SparkQA
Copy link

SparkQA commented May 9, 2017

Test build #76692 has finished for PR 17904 at commit 766bfb0.

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

@ajbozarth
Copy link
Member Author

@srowen passed test, this good to merge?

@srowen
Copy link
Member

srowen commented May 10, 2017

Merged to master/2.2

asfgit pushed a commit that referenced this pull request May 10, 2017
## What changes were proposed in this pull request?

#14617 added new columns to the executor table causing the visibility checks for the logs and threadDump columns to toggle the wrong columns since they used hard-coded column numbers.

I've updated the checks to use column names instead of numbers so future updates don't accidentally break this again.

Note: This will also need to be back ported into 2.2 since #14617 was merged there

## How was this patch tested?

Manually tested

Author: Alex Bozarth <ajbozart@us.ibm.com>

Closes #17904 from ajbozarth/spark20630.

(cherry picked from commit ca4625e)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@asfgit asfgit closed this in ca4625e May 10, 2017
@ajbozarth ajbozarth deleted the spark20630 branch May 10, 2017 18:45
liyichao pushed a commit to liyichao/spark that referenced this pull request May 24, 2017
## What changes were proposed in this pull request?

apache#14617 added new columns to the executor table causing the visibility checks for the logs and threadDump columns to toggle the wrong columns since they used hard-coded column numbers.

I've updated the checks to use column names instead of numbers so future updates don't accidentally break this again.

Note: This will also need to be back ported into 2.2 since apache#14617 was merged there

## How was this patch tested?

Manually tested

Author: Alex Bozarth <ajbozart@us.ibm.com>

Closes apache#17904 from ajbozarth/spark20630.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants