The client connections had the Nagle algorithm enabled which could cause
bad performance with smaller workloads. The common network configuration
code in utils.cc, currently used by the backend connections, sets it
properly.
The LocalClient micro-client required a reference to the session that was
valid at construction time. This is the reason why the previous
implementation used dcb_foreach to first gather the targets and then
execute queries on them. By replacing this reference with pointers to the
raw data it requires, we lift the requirement of the orignating session
being alive at construction time.
Now that the LocalClient no longer holds a reference to the session, the
killing of the connection does not have to be done on the same thread that
started the process. This prevents the deadlock that occurred when
concurrect dcb_foreach calls were made.
Replaced the unused dcb_foreach_parallel with a version of dcb_foreach
that allows iteration of DCBs local to this worker. The dcb_foreach_local
is the basis upon which all DCB access outside of administrative tasks
should be built on.
This change will introduce a regression in functionality: The client will
no longer receive an error if no connections match the KILL query
criteria. This is done to avoid having to synchronize the workers after
they have performed the killing of their own connections.
The dcb_foreach function is not safe to use from multiple threads at the
same time. This should be asserted by checking that the function is called
only from the main worker.
The addition of this assertion also implies that only administrative
operations should use the dcb_foreach function. To accommodate this
change, the KILL command iteration needs to be adjusted.
Due to the skewed accept distribution without SO_REUSEPORT, we use
round-robin assignment of workers for new client connections. This
provides better performance as work is more likely to be evenly
distributed across all threads.
Using a least-busy-worker algorithm would provide a more stable result but
this is not trivially simple to implement. For this reason, the
round-robin based approach was chosen for 2.2.
When a client connection is closed by MaxScale before the client initiates
a controlled closing of the connection, an error message is sent. This
error message now also explains why the connection was closed to make
problem resolution easier.
- If a client DCB should be moved to some other worker than
the current one (cli and maxinfo), and that fails, the
thread id must be reset to that of the calling thread as
otherwise asserts will be triggered.
- If the creation of the first DCB fails, then the dcb list
for that thread will be NULL and thus must be accessed
with some caution.
When DCBs are being hung in dcb_hangup_foreach, the hangup event can be
processed directly. This prevents excessive use of the worker message
queue pipe thus reducing the possibility of it being full.
The fact that a client dcb was immediately added to the epoll-
instance of the relevant worker (possible, since that is thread-
safe), but was added to the book-keeping via the message mechanism
(necessary, since that is not thread-safe), meant that if the
connection was closed before the message was delivered, the handling
of the message then caused an access error.
Now the fd is also added to the epoll-instance via the messaging
mechanism, so the problem can no longer occur. The only fds this
affects are connections made to maxadmin or maxinfo as they are
always handled by the main thread due to deadlock issues.
The same problem that caused maxadmin to lock up was also what caused
maxinfo to lock up. The concurrent access to the legacy administrative
functions caused deadlocks.
With the changes to the DCB handling, the service pointer of a client DCB
must always be assigned.
Also removed the unnecessary parentheses around the comparison.
If maxadmin connections are handled by different workers, then
there may be a deadlock if some maxadmin command requires
communication with all workers.
Namely, in that case a message will be sent to all other workers
but the current one, but that message will not be handled if that
other worker at that point sits in the debugcmd_lock spinlock
in debugcmd.c:execute_cmd().
We can prevent that deadlock from happening simply by ensuring
that all maxadmin connections are handled by one thread.
We need to copy some data from a AF_UNIX based listener dcb
to the accepted client dcb, to prevent assertion violation in
dcb_get_port(). Further, to be able to log the path in the case
of an authentication error we need to copy that as well.
When a double close is detected in a debug build, a debug assertion is
triggered. This will generate a core dump which should help investigate
the double close.
Since there is a concept called "listener" it is confusing that the
dcb "this" argument in some dcb functions is called 'listener' instead
of 'dcb' as it is called everywhere else.
The internal header directory conflicted with in-source builds causing a
build failure. This is fixed by renaming the internal header directory to
something other than maxscale.
The renaming pointed out a few problems in a couple of source files that
appeared to include internal headers when the headers were in fact public
headers.
Fixed maxctrl in-source builds by making the copying of the sources
optional.
Only used in conjunction with queued connections, which are not
enabled anyway. Once that comes on the table again, better to use
some standard data structures.
Apart from listeners, all DCBs will be assigned to the current
thread. This simplifies the addition of DCBs to worker threads.
Also performed a small cleanup of poll_add_dcb to make it more readable.
Now it is also possible to ensure that a DCB stays alive while
a task referring to it is posted from one worker to another.
That will be implemented in a subsequent commit.
The polling mechanism can now optionally be used for managing
the lifetime of an object placed into the poll set.
If a MXS_POLL_DATA has a non-null 'free', then the reference count
of the data will be increased before calling the handler and
decreased after. In that case, if the reference count reaches 0,
the free function will be called.
Note that the reference counts of *all* MXS_POLL_DATAs returned
by 'epoll_wait' will be increased before the events are delivered
to the handlers individually for each MXS_POLL_DATA, and then once
all events have been delivered, the reference count of each
MXS_POLL_DATA will be decreased.
This ensure that if there are interdependencies between different
MXS_POLL_DATAs returned by one call to 'epoll_wait', the case that
an MXS_POLL_DATA is deleted before its events have been delivered
can be avoided by using the reference count for lifetime management.
In subsequent commits, the reference count will be taken into use
in the lifetime management of DCBs.
When something fails inside dcb_connect we rewind the situation
properly, without calling any of the close functions intended for
shutting down a properly created DCB. That way they can be simplified
and once the reference counting is taken into use it is sufficient to
call dcb_dec_ref(dcb) instead of dcb_free_all_memory().
If a fake event is added to the current dcb, we arrange things so
that it is delivered immediately when the handling of the event(s)
during which the fake event was added, has been performed.
Otherwise the event is delivered via the event loop.
The adminusers test did not properly initialize all subsystems in
MaxScale. The polling and DCB tests weren't updated with the changes to
the DCB closing.
dcb_readq_append()
dcb_readq_prepend()
dcb_readq_set()
dcb_readq_has()
dcb_readq_release()
dcb_readq_get()
dcb_readq_length()
No code but for DCB code itself should directly manipulate the
internals of a DCB. Thesse functions will be taken into use in
protocol modules.
Before each event handler is called, it is checked whether the
dcb has been closed. If it has been, then the event handler is
not called.
The check has to be made before each event handler, because any
event handler can close the dcb.
If the current worker id is -1, we do not insist that the dcb
is closed by the owning thread. That will happen only for dcbs
that are created before the workers have been started and hence
it is also ok to close them after the workers have exited.
If a dcb being closed is the dcb for which events are currently being
processed, the dcb is not closed immediately but only after all events
have been delivered.
