Skip to content

Conversation

@zuxqoj
Copy link
Contributor

@zuxqoj zuxqoj commented Apr 27, 2015

No description provided.

@srowen
Copy link
Member

srowen commented Apr 27, 2015

Does this need to be documented too?

@zuxqoj
Copy link
Contributor Author

zuxqoj commented Apr 27, 2015

I think we should document it.
I will update running-on-yarn.md also

@zuxqoj
Copy link
Contributor Author

zuxqoj commented Apr 27, 2015

added "spark.yarn.am.port" in running-on-yarn.md

@srowen
Copy link
Member

srowen commented Apr 27, 2015

@zsxwing @vanzin what do you think?

@SparkQA
Copy link

SparkQA commented Apr 27, 2015

Test build #718 has finished for PR 5719 at commit 456a592.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

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 we should be more specific in the description of this. an application master could have many ports open, what is this one for. I think either put in name or atleast put in description.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also kind of confusing is that in other places we use the spark.yarn.am.* namespace to refer only to the yarn-client application master. If this applies to both yarn-client and yarn-cluster, the naming is kind of inconsistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sryza what do you suggest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm missing something this is only used in yarn client mode. This the communication channel (rpc) between AM on cluster and driver on the client side. It looks like the rest of spark uses things just like spark.executor.port, spark.driver.port, etc.. So this kind of follows that convention. Personally I would like it more descriptive but since that is kind of convention I'm ok with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WangTaoTheTonic I am not 100% sure but I think it does communicate with driver also, in runAMEndpoint method it is creating RpcEndpoint to communicate with driver. Can someone please confirm, I will update description accordingly.

@vanzin
Copy link
Contributor

vanzin commented Apr 27, 2015

So, isn't a single port here kind of limiting? If you're unlucky and two AMs start on the same node, one of them will fail. Wouldn't it be better to have a port range here?

@zuxqoj
Copy link
Contributor Author

zuxqoj commented Apr 27, 2015

@vanzin user is expected to specify different port number for different application.

@vanzin
Copy link
Contributor

vanzin commented Apr 27, 2015

That is not very user-friendly at all.

@zsxwing
Copy link
Member

zsxwing commented Apr 27, 2015

@vanzin, Rpc.create will call Utils.startServiceOnPort to find another port for port conflicts.

@vanzin
Copy link
Contributor

vanzin commented Apr 27, 2015

Rpc.create will call Utils.startServiceOnPort to find another port for port conflicts.

I see. In that case the documentation added in this change is wrong, since the actual port will be anything between x and x + spark.port.maxRetries.

@tgravescs
Copy link
Contributor

There is also a separate jira to add support for port ranges. SPARK-4449

@tgravescs
Copy link
Contributor

ok to test

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this channel to communicate with driver either?

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 not the channel used to communicate with executors.
This channel is used in yarn client mode to communicate between the spark driver running on a gateway and the application master (which is doing container requests) running on yarn.

It also looks like I was wrong my previous comment. It actually is also used in yarn cluster mode. Its used for the dynamic executor feature, where it is handling the kill from the scheduler backend.

@SparkQA
Copy link

SparkQA commented Apr 29, 2015

Test build #31258 has finished for PR 5719 at commit 9de5330.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@tgravescs
Copy link
Contributor

this is odd, I don't see our comments showing up out here. @zuxqoj can you look at the Files changed tab and update based on the comments

…ystem

updated config description as suggested by @tgravescs
@srowen
Copy link
Member

srowen commented May 2, 2015

LGTM pending tests

@SparkQA
Copy link

SparkQA commented May 2, 2015

Test build #31661 has finished for PR 5719 at commit 5117258.

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

Copy link
Member

Choose a reason for hiding this comment

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

Nits: yarn -> YARN, spark -> Spark, 'application master' -> 'Application Master'. Is this really the most accurate description of the AM role? it's where you find the Spark UI, right? the rest of this info isn't so useful to callers. It's just the port where the AM listens.

@SparkQA
Copy link

SparkQA commented May 3, 2015

Test build #31688 has finished for PR 5719 at commit 5574ff7.

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

@srowen
Copy link
Member

srowen commented May 3, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented May 3, 2015

Test build #31690 has finished for PR 5719 at commit 5574ff7.

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

asfgit pushed a commit that referenced this pull request May 5, 2015
…system

Author: shekhar.bansal <[email protected]>

Closes #5719 from zuxqoj/master and squashes the following commits:

5574ff7 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
5117258 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
9de5330 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
456a592 [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
803e93e [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system

(cherry picked from commit fc8feaa)
Signed-off-by: Sean Owen <[email protected]>
@asfgit asfgit closed this in fc8feaa May 5, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
…system

Author: shekhar.bansal <[email protected]>

Closes apache#5719 from zuxqoj/master and squashes the following commits:

5574ff7 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
5117258 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
9de5330 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
456a592 [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
803e93e [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
…system

Author: shekhar.bansal <[email protected]>

Closes apache#5719 from zuxqoj/master and squashes the following commits:

5574ff7 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
5117258 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
9de5330 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
456a592 [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
803e93e [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
…system

Author: shekhar.bansal <[email protected]>

Closes apache#5719 from zuxqoj/master and squashes the following commits:

5574ff7 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
5117258 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
9de5330 [shekhar.bansal] [SPARK-6653][yarn] New config to specify port for sparkYarnAM actor system
456a592 [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
803e93e [shekhar.bansal] [SPARK-6653][yarn] New configuration property to specify port for sparkYarnAM actor system
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.

8 participants