Skip to content

Conversation

@mdumandag
Copy link
Contributor

Formerly, writes were checking the size of the write queue, and
if there are no items in it, were writing directly to the socket.
That is against the nature of the non-blocking API we provide.
Also, it restains us from optimizing the writes for non-blocking
usage (multiple client messages can be concatenated into single
buffer before writing to the socket to decrease the number of
send calls).

To solve that problem, connections are adding buffers into the
write queue. Reactor thread pops elements from that queue and
writes into the socket. (The optimization I mentioned above
can be implemented later). Of course, this approach will kill the
performance of blocking usage since the reactor thread will be
in sleep most of the time on asyncore.loop call. We need to wake
the reactor thread each time we want to write something if it is
not awake.

To do this, a pipe is added to the reactor's dispatchers map. Each
time we want to write something, we check whether or not it is awake
and if not, we write a single byte into the pipe. That wakes the
reactor thread and makes it process the message we added to its
write queue. The pipe works great on UNIX, but pipes are not
selectable or pollable on Windows. We fall back into sockets in case
the client is running on Windows.

On the reactor, we try to create the loop, check if the added waker
(pipe or socket) works or not. If it does, we use the wakable loop.
If not, we fall back to the loop that does busy waiting.

Formerly, writes were checking the size of the write queue, and
if there are no items in it, were writing directly to the socket.
That is against the nature of the non-blocking API we provide.
Also, it restains us from optimizing the writes for non-blocking
usage (multiple client messages can be concatenated into single
buffer before writing to the socket to decrease the number of
send calls).

To solve that problem, connections are adding buffers into the
write queue. Reactor thread pops elements from that queue and
writes into the socket. (The optimization I mentioned above
can be implemented later). Of course, this approach will kill the
performance of blocking usage since the reactor thread will be
in sleep most of the time on asyncore.loop call. We need to wake
the reactor thread each time we want to write something if it is
not awake.

To do this, a pipe is added to the reactor's dispatchers map. Each
time we want to write something, we check whether or not it is awake
and if not, we write a single byte into the pipe. That wakes the
reactor thread and makes it process the message we added to its
write queue. The pipe works great on UNIX, but pipes are not
selectable or pollable on Windows. We fall back into sockets in case
the client is running on Windows.

On the reactor, we try to create the loop, check if the added waker
(pipe or socket) works or not. If it does, we use the wakable loop.
If not, we fall back to the loop that does busy waiting.
It may be the case that client is disconnected while we are in the
loop, an no data is sent. In that case, we were adding the data
to the write queue again, and try to pop from it within the same loop.
That was causing an infinite loop. An early return is added to
solve this problem. When disconnected, dispatcher will be closed,
and handle_write won't be called again by the asyncore.
@puzpuzpuz puzpuzpuz self-requested a review November 17, 2020 07:41
Copy link

@puzpuzpuz puzpuzpuz left a comment

Choose a reason for hiding this comment

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

LGTM in general. Left some comments for further discussion, but there is nothing major

@mdumandag mdumandag merged commit 741e63b into hazelcast:master Nov 18, 2020
@mdumandag mdumandag deleted the non-blocking-writes branch November 18, 2020 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants