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-6490][Core] Add spark.rpc.* and deprecate spark.akka.* #5595

Closed
wants to merge 2 commits into from
Closed

[SPARK-6490][Core] Add spark.rpc.* and deprecate spark.akka.* #5595

wants to merge 2 commits into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Apr 20, 2015

Deprecated spark.akka.num.retries, spark.akka.retry.wait, spark.akka.askTimeout, spark.akka.lookupTimeout, and added spark.rpc.num.retries, spark.rpc.retry.wait, spark.rpc.askTimeout, spark.rpc.lookupTimeout.

@zsxwing
Copy link
Member Author

zsxwing commented Apr 20, 2015

cc @rxin
cc @vanzin since you refactored the deprecated configs.

@SparkQA
Copy link

SparkQA commented Apr 20, 2015

Test build #30590 has finished for PR 5595 at commit 31dbe69.

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


/** Returns the configured number of milliseconds to wait on each retry */
def retryWaitMs(conf: SparkConf): Long = {
conf.getLong("spark.rpc.retry.wait", 3000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're here, could you use the new API:

conf.getTimeAsMs("spark.rpc.retry.wait", "3s")

Similar in other methods in this file.

@vanzin
Copy link
Contributor

vanzin commented Apr 20, 2015

I don't see these documented anywhere, so aside from needing an update to use the new conf API, this LGTM.


/** Returns the configured number of times to retry connecting */
def numRetries(conf: SparkConf): Int = {
conf.getInt("spark.rpc.num.retries", 3)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should do spark.rpc.numRetries instead (you might need to translate this to akka.num.retries if we pass this back to akka)

@rxin
Copy link
Contributor

rxin commented Apr 20, 2015

We should also update the documentation page, as @vanzin pointed out. That can be done as a follow-up pull request too.

@SparkQA
Copy link

SparkQA commented Apr 21, 2015

Test build #30626 has finished for PR 5595 at commit e0d80a9.

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

@rxin
Copy link
Contributor

rxin commented Apr 21, 2015

Merging in master. Thanks.

@rxin
Copy link
Contributor

rxin commented Apr 21, 2015

Can you create a PR to update documentation?

@zsxwing
Copy link
Member Author

zsxwing commented Apr 21, 2015

Can you create a PR to update documentation?

Updated docs in #5607.

should these be set to the default network timeout?

same for lookupTimeout

Just remind that the default network timeout isn't used now.

asfgit pushed a commit that referenced this pull request Apr 22, 2015
Added docs for rpc configurations and also fixed two places that should have been fixed in #5595.

Author: zsxwing <[email protected]>

Closes #5607 from zsxwing/SPARK-6490-docs and squashes the following commits:

25a6736 [zsxwing] Increase the default timeout to 120s
6e37c30 [zsxwing] Update docs
5577540 [zsxwing] Use spark.network.timeout as the default timeout if it presents
4f07174 [zsxwing] Fix unit tests
1c2cf26 [zsxwing] Add docs for rpc configurations
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
Deprecated `spark.akka.num.retries`, `spark.akka.retry.wait`, `spark.akka.askTimeout`,  `spark.akka.lookupTimeout`, and added `spark.rpc.num.retries`, `spark.rpc.retry.wait`, `spark.rpc.askTimeout`, `spark.rpc.lookupTimeout`.

Author: zsxwing <[email protected]>

Closes apache#5595 from zsxwing/SPARK-6490 and squashes the following commits:

e0d80a9 [zsxwing] Use getTimeAsMs and getTimeAsSeconds and other minor fixes
31dbe69 [zsxwing] Add spark.rpc.* and deprecate spark.akka.*
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
Added docs for rpc configurations and also fixed two places that should have been fixed in apache#5595.

Author: zsxwing <[email protected]>

Closes apache#5607 from zsxwing/SPARK-6490-docs and squashes the following commits:

25a6736 [zsxwing] Increase the default timeout to 120s
6e37c30 [zsxwing] Update docs
5577540 [zsxwing] Use spark.network.timeout as the default timeout if it presents
4f07174 [zsxwing] Fix unit tests
1c2cf26 [zsxwing] Add docs for rpc configurations
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.

4 participants