[SPARK-33842][Core][Shuffle] Add spark.shuffle.io.sessionTimeout to check an established connection for being idled or dead

[yaooqinn]

What changes were proposed in this pull request?

We have spark.shuffle.io.connectionTimeout for timeout checking when shuffle clients establish connections with shuffle servers. We also use it to check the established connection is idled or dead too. If we want a connection that can keep alive for a long time, we may increase the blocking time to create a shuffle client if the server is busy at that time.

In practice, we always use connection timeout and session timeout to control these two different phases.
This PR adds spark.shuffle.io.sessionTimeout to check an established connection for being idled or dead

Why are the changes needed?

Does this PR introduce any user-facing change?

yes, w/ new conf spark.shuffle.io.sessionTimeout ` to check an established connection for being idled or dead

How was this patch tested?

modified tests

[SparkQA]

Test build #133029 has finished for PR 30844 at commit 9f9f2f6.

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

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37628/

[SparkQA]

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37628/

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37632/

[SparkQA]

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37632/

[SparkQA]

Test build #133033 has finished for PR 30844 at commit e51904f.

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

[mridulm]

+CC @otterc

[yaooqinn]

cc @cloud-fan @maropu @HyukjinKwon too thanks

[otterc]

@yaooqinn
In #30312, we have introduced a connectionCreationTimeout and I think the motivation for that change and this one is the same. So, we should decide which implementation, related with this config, we would want go ahead with.
cc @mridulm @Ngone51 @Victsm @tgravescs

[yaooqinn]

Oh, sorry @otterc, I didn't notice #30312.
IMHO, connectionCreationTimeout seems to be not a common usage. But sessionTimeout and connectionTimeout are always used as a pair in RPC environments.
#30312 seems will get merged soon. I am fine w/ that too.

[otterc]

IMHO, connectionCreationTimeout seems to be not a common usage. But sessionTimeout and connectionTimeout are always used as a pair in RPC environments.

Just stating the reason for why we went with creating connectionCreationTimeout instead of idle/sessionTimeout.
We wanted to reduce the timeout for creating connections. Ideally this should be much lower and for our clusters we have set this to 30s. However, the default value for connectionTimeout is high which makes sense because it is also used as idleTimeout and since shuffle services can be overloaded, they may take longer to respond. The problem with creating a separate config for idle/sessionTimeout and not for connectionCreation is that some users (frameworks) may have also over-written the value of connectionTimeout to a much higher value. So, even if want to enforce all these jobs to use smaller timeouts for creating connections, they will still be using a larger timeouts for that.