Single process client#165
Merged
zuiderkwast merged 4 commits intomainfrom Apr 14, 2026
Merged
Conversation
zuiderkwast
commented
Apr 2, 2026
bjosv
reviewed
Apr 13, 2026
bjosv
reviewed
Apr 13, 2026
bjosv
reviewed
Apr 13, 2026
| -type connection_opt() :: | ||
| %% If commands are queued up in the process message queue this is the max | ||
| %% amount of messages that will be received and sent in one call | ||
| {batch_size, non_neg_integer()} | |
Collaborator
There was a problem hiding this comment.
batch_size changed to pos_integer() here too?
| {connection_opts, [ered_connection:opt()]} | | ||
| {connection_opts, [connection_opt()]} | | ||
| %% Max number of commands allowed to wait in queue. | ||
| {max_waiting, non_neg_integer()} | |
Collaborator
There was a problem hiding this comment.
Should we change max_waiting changed to pos_integer() here too?
Same then for max_pending and queue_ok_level below
Collaborator
Author
There was a problem hiding this comment.
Good catch. I guess queue_ok_level can be zero though. It means the queue is not considered OK until it's emptied. Should be possible to use.
9dcd799 to
ae09420
Compare
bjosv
reviewed
Apr 14, 2026
bjosv
approved these changes
Apr 14, 2026
* Get rid of the two processes in ered_connection and duplicated logic. * Use active mode. * Response timeout handling is existing but incomplete. * Batching is not fixed. Only sends one command at a time. * Sending may block. The client process is unresponsive when it happens. Batching and non-blocking send are to be fixed in follow-up PRs. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
When the pending queue is full, don't send more commands until the pending queue can fit another batch of commands, as configured with the existing config `batch_size`. The purpose is to improve performance by sending fewer and larger TCP packets. The config for `batch_size` has existed before but the batching functionality has not worked properly. Bonus: Use assert macros instead of direct pattern matching in eunit tests to verify output/input, to get better error messages on failure. --------- Co-authored-by: William Voong <william.voong@ericsson.com> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
Store erlang:monotonic_time(millisecond) in each pending_req when commands are sent to the server. Use the oldest pending request's timestamp to compute the remaining response timeout, so that time already spent waiting is accounted for.
6e1864a to
68325b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.