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

Support for Socket Write and Read Timeout Settings in CRT #695

Open
2 tasks
kareali opened this issue Dec 30, 2024 · 2 comments
Open
2 tasks

Support for Socket Write and Read Timeout Settings in CRT #695

kareali opened this issue Dec 30, 2024 · 2 comments
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@kareali
Copy link

kareali commented Dec 30, 2024

Describe the feature

The current implementation of the CRT's SocketOptions does not provide support for configuring read and write timeouts for non-blocking socket operations. While traditional blocking sockets can leverage options like SO_RCVTIMEO (receive timeout) and SO_SNDTIMEO (send timeout) to set timeouts for read and write operations, CRT operates in a non-blocking mode, requiring custom mechanisms to achieve similar timeout behavior.

Use Case

We are migrating our Java SDK clients to the CRT-based HTTP client. The existing SDK supports configurable socket-level timeouts that ensure timely error handling and retries if:

  1. A write operation cannot be completed within the write timeout (e.g., sending a request).
  2. A read operation cannot be completed within the read timeout (e.g., receiving a response after a request is sent).
    To maintain compatibility and ensure robust error handling in non-blocking CRT, we need equivalent timeout settings that close the connection when timeouts are exceeded.

Proposed Solution

  1. Extend the SocketOptions Interface:
    Add the following properties to SocketOptions to support timeout settings:
  • readTimeoutMs: The maximum time (in milliseconds) allowed to complete a read operation.
  • writeTimeoutMs: The maximum time (in milliseconds) allowed to complete a write operation.
  1. Introduce Timeout Handlers:

Implement dedicated read timeout handlers and write timeout handlers that monitor the progress of non-blocking operations.

These handlers should enforce timeouts:

Read Timeout Handler: Monitors the time elapsed since a read operation is triggered. If no data is received within the readTimeoutMs, raise a timeout exception.
Write Timeout Handler: Monitors the time elapsed since the last write operation. If data cannot be sent within the writeTimeoutMs, raise a timeout exception.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@kareali kareali added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 30, 2024
@bretambrose
Copy link
Contributor

bretambrose commented Dec 30, 2024

The CRT team can comment better on the current configuration options for throughput control, but I wanted to chime in that I don't think those socket options are relevant to the CRT. They appear to be relevant to blocking calls (on both Linux and Windows), which the CRT does not make. Any read/write timeout options, if they ever are added, are likely to require hand-written logic within a particular protocol's implementation (or an optional monitoring system attached to the protocol implementation).

@kareali kareali changed the title Support for Configurable SO_RCVTIMEO and SO_SNDTIMEO Socket Timeout Options in CRT Support for Socket Write and Read Timeout Settings in CRT Dec 31, 2024
@kareali
Copy link
Author

kareali commented Dec 31, 2024

Thank you for the clarification! You're absolutely right that SO_RCVTIMEO and SO_SNDTIMEO are specifically for blocking calls, which CRT does not use. My intention wasn't to imply that these exact parameters are required but rather to highlight the need for equivalent socket configurations or mechanisms that provide the same functionality.

I've updated the issue to include more details about the desired behavior and the rationale behind it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants