Skip to content

[SPARK-10432] spark.port.maxRetries documentation is unclear #8585

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

Closed
wants to merge 1 commit into from

Conversation

tgravescs
Copy link
Contributor

No description provided.

@SparkQA
Copy link

SparkQA commented Sep 3, 2015

Test build #41971 has finished for PR 8585 at commit 1070031.

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

@srowen
Copy link
Member

srowen commented Sep 3, 2015

LGTM

@andrewor14
Copy link
Contributor

Yup, looks good.

@holdenk
Copy link
Contributor

holdenk commented Sep 3, 2015

Maybe worth mentioning what happens when the port is given a non-specific value as well?

@tgravescs
Copy link
Contributor Author

I was on the fence about that as it somewhat seemed redundant with the first line, but if people thinks it will help I can add a line like:

Maximum number of retries when binding to a port before giving up.

  • When the port is 0 it retries 0 the specified maximum number of retries.
    When a port is non 0, each subsequent retry will
    increment the port used in the previous attempt by 1 before retrying. This
    essentially allows it to try a range of ports from the start port specified
    to port + maxRetries.

@srowen @andrewor14 thoughts?

@andrewor14
Copy link
Contributor

Yeah, sounds a little redundant. I think it's OK given that it's pretty common for 0 to mean a random port. Since we explicitly say (n + 1) only applies to non-0 ports I think it's fine.

@andrewor14
Copy link
Contributor

OK, I'm going to merge this into master.

@asfgit asfgit closed this in 49aff7b Sep 3, 2015
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