Merge feature branch "Single process client"#165
Open
zuiderkwast wants to merge 3 commits intomainfrom
Open
Conversation
Collaborator
zuiderkwast
commented
Apr 2, 2026
zuiderkwast
commented
- Merge ered_client and ered_connection (reduces the number of processes from 3 to 1 per connection)
- Send commands in batches
- Non-blocking send
* 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>
zuiderkwast
commented
Apr 2, 2026
Comment on lines
+814
to
+818
| response_timeout(State) when not ?q_is_empty(State#st.pending) -> | ||
| %% FIXME: Store req timeout in each pending item | ||
| State#st.opts#opts.timeout; | ||
| response_timeout(_State) -> | ||
| infinity. |
Collaborator
Author
There was a problem hiding this comment.
Many functions such as handle_cast return this timeout in the element in the type, such as {noreply, State, response_timeout(State)}.
This is quite simplistic, since it will effectively reset the timeout every time there is any interaction with the process, including sending more to the server.
Storing a response timeout timestamp per pending request would be the most exact handling, but maybe it's overkill?
Another approach is to have a single timestamp and reset it only whenever we get a response from the server. Then, this timeout effectively limits how long we're waiting between each response.
This depends very much on your requirements. @WilliamVoong please find out. 😄
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
2 participants
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.