Skip to content

Single process client#165

Merged
zuiderkwast merged 4 commits intomainfrom
single-process-client
Apr 14, 2026
Merged

Single process client#165
zuiderkwast merged 4 commits intomainfrom
single-process-client

Conversation

@zuiderkwast
Copy link
Copy Markdown
Collaborator

@zuiderkwast zuiderkwast commented Apr 2, 2026

@zuiderkwast zuiderkwast requested a review from WilliamVoong April 2, 2026 14:29
Comment thread src/ered_client.erl
@zuiderkwast zuiderkwast requested a review from JeppeTh April 2, 2026 14:44
@zuiderkwast zuiderkwast requested a review from bjosv April 13, 2026 07:03
Comment thread src/ered_client.erl Outdated
Comment thread src/ered_client.erl
Comment thread src/ered_client.erl
Comment thread src/ered_client.erl Outdated
Comment thread src/ered_client.erl Outdated
-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()} |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

batch_size changed to pos_integer() here too?

Comment thread src/ered_client.erl Outdated
{connection_opts, [ered_connection:opt()]} |
{connection_opts, [connection_opt()]} |
%% Max number of commands allowed to wait in queue.
{max_waiting, non_neg_integer()} |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we change max_waiting changed to pos_integer() here too?
Same then for max_pending and queue_ok_level below

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

@zuiderkwast zuiderkwast force-pushed the single-process-client branch from 9dcd799 to ae09420 Compare April 14, 2026 13:46
Comment thread README.md
@zuiderkwast zuiderkwast changed the title Merge feature branch "Single process client" Single process client Apr 14, 2026
zuiderkwast and others added 4 commits April 14, 2026 17:23
* 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.
@zuiderkwast zuiderkwast force-pushed the single-process-client branch from 6e1864a to 68325b6 Compare April 14, 2026 15:23
@zuiderkwast zuiderkwast merged commit aed4535 into main Apr 14, 2026
22 checks passed
@zuiderkwast zuiderkwast deleted the single-process-client branch April 14, 2026 15:45
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