Skip to content

Non-blocking send#160

Merged
zuiderkwast merged 7 commits intoEricsson:single-process-clientfrom
zuiderkwast:non-blocking-send
Apr 1, 2026
Merged

Non-blocking send#160
zuiderkwast merged 7 commits intoEricsson:single-process-clientfrom
zuiderkwast:non-blocking-send

Conversation

@zuiderkwast
Copy link
Copy Markdown
Collaborator

@zuiderkwast zuiderkwast commented Mar 2, 2026

Use send_timeout 0. If send returns timeout, it has still queued all the data for sending later. When this happens, backoff and don't send more to the socket until the pending commands have got replies from the server. This requires that gen_tcp is used with the inet backend (the default) and not the socket backend.

With TLS, this approach works in older OTP versions, but it was broken for TLS in OTP 28.0 - 28.4, but fixed again in 28.4.1 (OTP-20018).

If we need to support these OTP versions, or if we want to suppor gen_tcp with the socket backend or the socket module directly, we can implement more variants later.

Use send_timeout 0. If send returns timeout, it has still queued all the data
for sending later. When this happens, backoff and don't send more to the
socket until the pending commands have got replies from the server.

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast marked this pull request as ready for review March 3, 2026 16:11
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast
Copy link
Copy Markdown
Collaborator Author

@zuiderkwast zuiderkwast requested review from bjosv and removed request for JeppeTh, WilliamVoong and drmull March 17, 2026 10:54
Copy link
Copy Markdown
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

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

LGTM, seems to be helpful

Copy link
Copy Markdown
Collaborator

@WilliamVoong WilliamVoong left a comment

Choose a reason for hiding this comment

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

Hi i think most of it looks good, just have a small comment in ered_client_tests.erl.

@zuiderkwast
Copy link
Copy Markdown
Collaborator Author

Some more context. If we use blocking send, the process can become unresponsive and we can get a deadlock. See the discussion here: #157 (comment)

zuiderkwast and others added 2 commits March 23, 2026 11:44
Co-authored-by: Hippo <44473866+WilliamVoong@users.noreply.github.com>
…blocking-send

# Conflicts:
#	src/ered_client.erl
Copy link
Copy Markdown
Collaborator

@WilliamVoong WilliamVoong left a comment

Choose a reason for hiding this comment

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

LGTM

Parametrize send_backoff_t with a Transport argument (gen_tcp or ssl)
and run both variants. The TLS variant is skipped on OTP 28.0 .. 28.4
(ssl 11.2.0 .. 11.5.2) where {send_timeout, 0} incorrectly closes the
ssl socket on timeout. This was fixed in ssl-11.5.3 (OTP-20018).
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast merged commit 16b1070 into Ericsson:single-process-client Apr 1, 2026
11 checks passed
zuiderkwast added a commit that referenced this pull request Apr 2, 2026
Use send_timeout 0. If send returns timeout, it has still queued all the
data for sending later. When this happens, backoff and don't send more
to the socket until the pending commands have got replies from the
server. This requires that gen_tcp is used with the inet backend (the
default) and not the socket backend.

With TLS, this approach works in older OTP versions, but it was broken
for TLS in OTP 28.0 - 28.4, but fixed again in 28.4.1 (OTP-20018).

If we need to support these OTP versions, or if we want to suppor
gen_tcp with the socket backend or the socket module directly, we can
implement more variants later.

---------

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Hippo <44473866+WilliamVoong@users.noreply.github.com>
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.

3 participants