[SPARK-13810] [CORE] Add Port Configuration Suggestions on Bind Exceptions

[bjornjon]

What changes were proposed in this pull request?

Currently, when a java.net.BindException is thrown, it displays the following message:

java.net.BindException: Address already in use: Service '$serviceName' failed after 16 retries!

This change adds port configuration suggestions to the BindException, for example, for the UI, it now displays

java.net.BindException: Address already in use: Service 'SparkUI' failed after 16 retries! Consider explicitly setting the appropriate port for 'SparkUI' (for example spark.ui.port for SparkUI) to an available port or increasing spark.port.maxRetries.

How was this patch tested?

Manual tests

[srowen]

I like the principle, but passing around an error message as an arg to SparkEnv makes this tiny aspect leak too much into the API. If it's just to be able to report which config to change, I think I wouldn't do that. Generically advise changing a port config. This would be much simpler then, commensurate with the scope of the change.

[srowen]

LGTM. (By the way Bjorn fields a lot of support issues for Spark and this is apparently a frequent source of questions/confusion. Great if a simple message change can preempt a number of questions.)

[squito]

I can't see the earlier version of this diff, but I dont' understand why you'd need to introduce a SparkEnv. I see that currently, when a WebUI is created, you no longer know which conf was responsible for choosing the port. But that seems easy to fix -- couldn't the constructor just also take a portConf: Option[String]?

In any case, this is a strict improvement as-is -- if that adds too much complexity to get the correct conf name in there or something, its fine w/ me to add this as is.

[squito]

bjorn and I chatted about this a bit more offline, I understand why its a bit complicated for the non-UI port a bit more, but also feel this simple change is good enough to solve most of the concern, so LGTM

[vanzin]

ok to test

[SparkQA]

Test build #52948 has finished for PR 11644 at commit 8177e69.

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

[andrewor14]

need space here after for

[bjornjon]

There is a space in $serviceString, as before, so the text is exactly as shown in the description above.

[vanzin]

Man, that's tricky.

But in the case where serviceString is empty, this would look odd:

Consider explicitly setting the appropriate port for (for example spark.ui.port for SparkUI)

Perhaps serviceString should be changed so that it's "Service" if serviceName is empty. Or make sure that it's never empty.

[andrewor14]

looks good

[SparkQA]

Test build #52975 has finished for PR 11644 at commit 933e205.

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

[vanzin]

now you need a space after "the".

[SparkQA]

Test build #52976 has finished for PR 11644 at commit 0d8303e.

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

[SparkQA]

Test build #52980 has finished for PR 11644 at commit 3a0e3f1.

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

[srowen]

Merged to master and 1.6. I added 1.6 since it's just a tiny doc improvement, and probably adds disproportionate value.