Skip to content

[SPARK-14437][Core]Use the address that NettyBlockTransferService listens to create BlockManagerId - #12240

Closed
zsxwing wants to merge 6 commits into
apache:masterfrom
zsxwing:SPARK-14437
Closed

[SPARK-14437][Core]Use the address that NettyBlockTransferService listens to create BlockManagerId#12240
zsxwing wants to merge 6 commits into
apache:masterfrom
zsxwing:SPARK-14437

Conversation

@zsxwing

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Here is why SPARK-14437 happens:
BlockManagerId is created using NettyBlockTransferService.hostName which comes from customHostname. And Executor will set customHostname to the hostname which is detected by the driver. However, the driver may not be able to detect the correct address in some complicated network (Netty's Channel.remoteAddress doesn't always return a connectable address). In such case, BlockManagerId will be created using a wrong hostname.

To fix this issue, this PR uses hostname provided by SparkEnv.create to create NettyBlockTransferService and set NettyBlockTransferService.hostname to this one directly. A bonus of this approach is NettyBlockTransferService won't bound to 0.0.0.0 which is much safer.

How was this patch tested?

Manually checked the bound address using local-cluster.

@zsxwing

Copy link
Copy Markdown
MemberAuthor

cc @vanzin

@SparkQA

Copy link
Copy Markdown

Test build #55236 has finished for PR 12240 at commit 9037529.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class NettyBlockTransferService(

* A BlockTransferService that uses Netty to fetch a set of blocks at at time.
*/
class NettyBlockTransferService(conf: SparkConf, securityManager: SecurityManager, numCores: Int)
private[spark] class NettyBlockTransferService(

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@rxin is it a mistake that NettyBlockTransferService was public?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Never mind. Just found it's in network package

@SparkQA

Copy link
Copy Markdown

Test build #55237 has finished for PR 12240 at commit 8c2d259.

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

@zsxwingzsxwing changed the title [SPARK-14437][Core]Use the addres that NettyBlockTransferService listens to create BlockManagerId[SPARK-14437][Core]Use the address that NettyBlockTransferService listens to create BlockManagerIdApr 7, 2016
@SparkQA

Copy link
Copy Markdown

Test build #55239 has finished for PR 12240 at commit 4a836dc.

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

@SparkQA

Copy link
Copy Markdown

Test build #55242 has finished for PR 12240 at commit 8a08eea.

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

@SparkQA

Copy link
Copy Markdown

Test build #55250 has finished for PR 12240 at commit 6e08543.

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

@SparkQA

Copy link
Copy Markdown

Test build #55317 has finished for PR 12240 at commit db9d497.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class DeserializeToObject(
    • case class SerializeFromObject(
    • case class DeserializeToObject(
    • case class SerializeFromObject(

@zsxwing

Copy link
Copy Markdown
MemberAuthor

retest this please

1 similar comment
@zsxwing

Copy link
Copy Markdown
MemberAuthor

retest this please

@SparkQA

Copy link
Copy Markdown

Test build #55364 has finished for PR 12240 at commit db9d497.

  • This patch fails some tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class DeserializeToObject(
    • case class SerializeFromObject(
    • case class DeserializeToObject(
    • case class SerializeFromObject(

@zsxwing

Copy link
Copy Markdown
MemberAuthor

retest this please

@SparkQA

Copy link
Copy Markdown

Test build #55376 has finished for PR 12240 at commit db9d497.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class DeserializeToObject(
    • case class SerializeFromObject(
    • case class DeserializeToObject(
    • case class SerializeFromObject(

@zsxwing

Copy link
Copy Markdown
MemberAuthor

retest this please

@SparkQA

Copy link
Copy Markdown

Test build #55379 has finished for PR 12240 at commit db9d497.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class DeserializeToObject(
    • case class SerializeFromObject(
    • case class DeserializeToObject(
    • case class SerializeFromObject(

@vanzin

Copy link
Copy Markdown
Contributor

LGTM.

@zsxwing

Copy link
Copy Markdown
MemberAuthor

Thanks, Merging to master.

@asfgitasfgit closed this in 4d7c359Apr 9, 2016
@zsxwing
zsxwing deleted the SPARK-14437 branch April 9, 2016 00:20
zzcclp pushed a commit to zzcclp/spark that referenced this pull request Apr 12, 2016
Sign up for freeto 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.

3 participants

@zsxwing@SparkQA@vanzin