Skip to content
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

Reduce default TLS handshake timeout from 10 to 5 seconds #2947

Merged
merged 1 commit into from
May 31, 2024

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

10 seconds feels like too high value to hold a connection attempt by waiting for TLS handshake to complete. Because TLS handshake happens before ST writes a request, it's always marked as RetryableException and is retried up to 3 times.

Modifications:

  • Reduce DEFAULT_HANDSHAKE_TIMEOUT from 10 to 5 seconds;

Result:

Less wait time for TLS handshake process that hangs longer than expected.

Motivation:

10 seconds feels like too high value to hold a connection attempt by
waiting for TLS handshake to complete. Because TLS handshake happens
before ST writes a request, it's always marked as `RetryableException`
and is retried up to 3 times.

Modifications:

- Reduce `DEFAULT_HANDSHAKE_TIMEOUT` from 10 to 5 seconds;

Result:

Less wait time for TLS handshake process that hangs longer than
expected.
@idelpivnitskiy idelpivnitskiy requested review from daschl and tkountis May 29, 2024 19:39
@idelpivnitskiy idelpivnitskiy self-assigned this May 29, 2024
Copy link
Contributor

@tkountis tkountis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still feels high, but in lack of data to drive a better default lgtm

@idelpivnitskiy idelpivnitskiy merged commit b49e66b into apple:main May 31, 2024
15 checks passed
@idelpivnitskiy idelpivnitskiy deleted the handshake branch May 31, 2024 15:35
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.

4 participants