[SPARK-3632] ConnectionManager can run out of receive threads with authentication on

[tgravescs]

If you turn authentication on and you are using a lot of executors. There is a chance that all the of the threads in the handleMessageExecutor could be waiting to send a message because they are blocked waiting on authentication to happen. This can cause a temporary deadlock until the connection times out.

To fix it, I got rid of the wait/notify and use a single outbox but only send security messages from it until authentication has completed.

[SparkQA]

QA tests have started for PR 2484 at commit 081b765.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2484 at commit 081b765.

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

[tgravescs]

@rxin I think you had looked at the connectionManager, any comments?

[rxin]

Hey sorry really busy this week. I will take a look at this next week. Added to my todo list.

[tgravescs]

Thanks @rxin

[rxin]

maybe add @tailrec to this?

[rxin]

This is kind of scary change since I don't fully understand the mechanics of auth. Let me contact you offline about this ..

[tgravescs]

Thanks @rxin. I updated based on your comments.

[SparkQA]

QA tests have started for PR 2484 at commit d6d4175.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2484 at commit d6d4175.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21039/

[rxin]

Actually this is probably more clear if you just write a while loop.

[rxin]

This change LGTM now I understand how it works.

[SparkQA]

QA tests have started for PR 2484 at commit b6bc80b.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2484 at commit b6bc80b.

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

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21216/

[SparkQA]

QA tests have started for PR 2484 at commit a0a961d.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2484 at commit a0a961d.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21217/

[rxin]

LGTM. Merging in master. Thanks.

[rxin]

Should we backport this into 1.1.1?

[tgravescs]

Yes. If it doesn't merge cleanly let me know.

[rxin]

Unfortunately it doesn't merge. How large of a cluster does we need to get this issue to happen?

[tgravescs]

I've seen it with 200 executors.