[SPARK-12614][Core]Don't throw non fatal exception from ask

[zsxwing]

Right now RpcEndpointRef.ask may throw exception in some corner cases, such as calling ask after stopping RpcEnv. It's better to avoid throwing exception from RpcEndpointRef.ask. We can send the exception to the future for ask.

[zsxwing]

CC @vanzin @rxin

Most of changes of space changes. You can use https://github.com/apache/spark/pull/10568/files?w=1 to review.

[SparkQA]

Test build #48637 has finished for PR 10568 at commit b2c2ed1.

• This patch fails from timeout after a configured wait of 250m.
• This patch merges cleanly.
• This patch adds no public classes.

[zsxwing]

retest this please

[SparkQA]

Test build #48672 has finished for PR 10568 at commit b2c2ed1.

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

[vanzin]

Not sure I like it. This can mask / make it more difficult to debug real issues, such as unserializable messages.

My preferences would be, in order:

• avoid calling send / ask when they'd throw the noisy exceptions
• filter out just the noisy exceptions, not all of them

[zsxwing]

Since ask returns a Future, I prefer to send the exception to the Future.

For send, I just submitted a commit to avoid swallowing unserializable exception.

[zsxwing]

retest this please

[SparkQA]

Test build #48716 has finished for PR 10568 at commit 7d3095d.

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

[zsxwing]

retest this please

[SparkQA]

Test build #49024 has finished for PR 10568 at commit 7d3095d.

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

[zsxwing]

retest this please

[SparkQA]

Test build #49938 has finished for PR 10568 at commit 7d3095d.

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

[vanzin]

This is better, although I'm not exactly a fan of blanket-ignoring exceptions like in the case of a local send. Also, doesn't this overlap with #10881?

[zsxwing]

This is better, although I'm not exactly a fan of blanket-ignoring exceptions like in the case of a local send. Also, doesn't this overlap with #10881?

A bit overlap. However, #10881 is for eliminating the annoying stack trace while this one is for catching non fatal errors.

[vanzin]

Since #10881 is already handling the one case where we don't care about the exception, can you remove this try..catch and just let any other exceptions propagate up the stack?

[zsxwing]

Removed

[SparkQA]

Test build #50129 has finished for PR 10568 at commit 424682e.

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

[vanzin]

LGTM.

[zsxwing]

Merging to master, thanks!