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.
Backported the minimal set of changes required to build 2.1 with GCC
8. The format-truncation and format-overflow warnings are disabled instead
of fixed in 2.1 to remove duplication of effort that was already done in
2.2 (the commit doesn't cherry-pick cleanly).
The parameter extraction caused a recursive lock of the server
spinlock. To work around this, an unlocked version of server_get_parameter
is needed.
Ideally, a lock-free setup would be used but due to this being a bug fix,
it will have to be done later on.
Returning the length of the value instead of a boolean allows the user to
know when the parameter value exceeded the buffer size passed as the
parameter.
The individual servers were missing a statistic that would give an
estimated query count. As there is no simple way to count queries for all
modules, counting the number of routed protocol packets is a suitable
substitute.
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.
The resultset processing for MySQL requires some extra work as it lacks
the proper SERVER_MORE_RESULTS_EXIST flag in the last EOF packet. Instead,
the first EOF packet has the SERVER_PS_OUT_PARAMS flag which needs to be
interpreted as a SERVER_MORE_RESULTS_EXIST flag for the second EOF packet.
Also corrected the EOF packet handling to do the flag checks in the code
that deals with the EOF packets.
As the modutil_state parameter is now used for more than large packet
tracking, the correct solution is to store this state object in the
readwritesplit session instead of interpreting it to a boolean value.
Fixed string truncation warnings by reducing max parameter lengths by one
where applicable. The binlogrouter filename lengths are slightly different
so using memcpy to work around the warnings is an adequate "solution"
until the root of the problem is solved.
Removed unnecessary CMake policy settings from qc_sqlite. Adding a
self-dependency on the source file of an external project has no effect
and only caused warnings to be logged.
The documentation stated that all CPUs would be used when threads=auto was
used. In reality the behavior was the same as was with 2.0 (number of CPUs
minus one).
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.
Readwritesplit would not handle multiple overlapping COM_STMT_EXECUTE
commands properly if they opened cursors. This was due to the fact that
the result would not be marked as complete and COM_STMT_FETCH commands
were executed as if they did not return results.
The correct implementation is to consider a COM_STMT_EXECUTE that opens a
cursor complete only when the first EOF packet is read (that is, when the
resultset header is read). This allows subsequent COM_STMT_FETCH commands
to be handled separately.
The separate COM_STMT_FETCH handling must count the number of packets that
are being fetched. This allows correct tracking of the state of a
COM_STMT_FETCH by checking that the number of packets is correct or the
second EOF/ERR packet is read.
When the connection pool is inspected, both the client username and IP
must match. This causes the pool to be partitioned by username and IP,
prevening unintentional sharing of connections between different users.
The Backend::dcb() method gives the raw pointer to the internal DCB. This
pointer is used by at least readwritesplit to map raw DCB pointers to
backends. To prevent stale pointers from being returned, m_dcb needs to be
set to NULL after it has been closed.
From a practical perspective it makes no relevant difference
whether you have to add an entry to the config file and restart
maxscale or if you have to restart maxscale and provide a specific
command line, so better to provide just either possiblity.
More important would be to provide a way for turning this feature
on and off at runtime.
With the configuration entry
dump_last_statements=[never|on_close|on_error]
you can now specify when and if to dump the last statements
of of a session.
With the configuration entry
retain_last_statements=<unsigned>
or the debug flag '--debug=retain-last-statements=<unsigned>',
MaxScale will store the specified number of last statements
for each session. By calling
session_dump_statements(session);
MaxScale will dump the last statements as NOTICE messages.
For debugging purposes.
If a MaxScale-generated configuration defines an empty value, it is
ignored with the assumption that the next modification will cause the
problem to correct itself.
Disabling the session cache prevents errors from being generated as the
default OpenSSL configuration is to enable session caching but with an
uninitialized context ID. In addition to preventing the errors, it
prevents the possible security problems implicated by the definition a
"static" context ID.
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.
Add missing listener JSON diagnostics call. Check that the
diagnostics_json function exists before calling it.
As the protocol modules don't have diagnostics functions, they aren't
called.
Replace hard-coded strings with constant parameters. This makes it
slightly cleaner.
If two services referred to the same filter instance, it would
cause the filter to deleted twice at MaxScale shutdown with a
crash as the result.
Now when the services are deleted we just collect the unique
filter instances and then delete them after all services have
been deleted.
Earlier, if a service had multiple listeners you would have had
MaxScale> show dbusers MyService
User names: alice@% ...
User names: bob@% ...
That is, no indication of which listener is reporting what. With
this commit the result will be
User names (MyListener1): alice@% ...
User names (MyListener2): bob@% ...
Further, the diagnostics function of an authenticator is now expected
to write the list of users to the provided DCB, without performing any
other formatting. The formatting (printing "User names" and appending
a line-feed) is now handled by the handler for the MaxAdmin command
"show dbusers".
If local address has been specified, then all connections created
using mxs_mysql_real_connect() will use that same local address as
well.
A system test has not been created as our VMs do not have more than
one usable IP-address. Locally it has been verified to work as
expected.
