Skip to content

Conversation

@attilapiros
Copy link
Contributor

@attilapiros attilapiros commented Feb 12, 2018

What changes were proposed in this pull request?

Extending RDD storage page to show executor addresses in the block table.

How was this patch tested?

Manually:

screen shot 2018-02-13 at 10 30 59

rddPartition.memoryUsed,
rddPartition.diskUsed,
rddPartition.executors.mkString(" "))
rddPartition.executors.map(id => executorIdToAddress.get(id).getOrElse(id)).mkString(" "))
Copy link
Contributor

Choose a reason for hiding this comment

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

.map { id => ... }

Also in other places.

sortColumn,
desc)
desc,
executorSummaries.map(ex => (ex.id, ex.hostPort)).toMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a good idea to keep the map sorted by the executor id, somehow.

@vanzin
Copy link
Contributor

vanzin commented Feb 12, 2018

ok to test

@SparkQA
Copy link

SparkQA commented Feb 12, 2018

Test build #87344 has finished for PR 20589 at commit 3ccad53.

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

@SparkQA
Copy link

SparkQA commented Feb 12, 2018

Test build #87343 has finished for PR 20589 at commit 3ccad53.

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

@kiszk
Copy link
Member

kiszk commented Feb 13, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87360 has finished for PR 20589 at commit 3ccad53.

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

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87386 has finished for PR 20589 at commit ec160aa.

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

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87387 has finished for PR 20589 at commit eee987a.

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

blockSortColumn,
blockSortDesc)
blockSortDesc,
store.executorList(false))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this only list active executors?

Theoretically you shouldn't have any blocks stored on dead executors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we have a fallback to executorId we can do that.

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87395 has finished for PR 20589 at commit ef69450.

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

@attilapiros
Copy link
Contributor Author

retest this please

1 similar comment
@kiszk
Copy link
Member

kiszk commented Feb 13, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87413 has finished for PR 20589 at commit ef69450.

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

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87420 has finished for PR 20589 at commit cdc5168.

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

@attilapiros
Copy link
Contributor Author

jenkins retest this please

@SparkQA
Copy link

SparkQA commented Feb 14, 2018

Test build #87430 has finished for PR 20589 at commit cdc5168.

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

@attilapiros
Copy link
Contributor Author

jenkins retest this please

rddPartition.executors.mkString(" "))
rddPartition.executors
.sorted
.map { id => executorIdToAddress.get(id).getOrElse(id) }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we sort by address?

Copy link
Contributor Author

@attilapiros attilapiros Feb 14, 2018

Choose a reason for hiding this comment

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

ok, done

@jiangxb1987
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Feb 14, 2018

Test build #87441 has finished for PR 20589 at commit cdc5168.

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

@vanzin
Copy link
Contributor

vanzin commented Feb 14, 2018

Merging to master / 2.3.

@SparkQA
Copy link

SparkQA commented Feb 14, 2018

Test build #87447 has finished for PR 20589 at commit 75bb8c5.

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

asfgit pushed a commit that referenced this pull request Feb 14, 2018
…tead of the IDs

## What changes were proposed in this pull request?

Extending RDD storage page to show executor addresses in the block table.

## How was this patch tested?

Manually:

![screen shot 2018-02-13 at 10 30 59](https://user-images.githubusercontent.com/2017933/36142668-0b3578f8-10a9-11e8-95ea-2f57703ee4af.png)

Author: “attilapiros” <[email protected]>

Closes #20589 from attilapiros/SPARK-23394.

(cherry picked from commit 140f875)
Signed-off-by: Marcelo Vanzin <[email protected]>
@asfgit asfgit closed this in 140f875 Feb 14, 2018
@attilapiros attilapiros deleted the SPARK-23394 branch April 26, 2018 20:07
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.

5 participants