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.
If a client is executing a COM_CHANGE_USER command and the
reauthentication of the client fails, no error message would be logged
about the failure of the reauthentication process and only a routing
failure message would be logged.
The protocol could leak memory in rare cases where several commands were
queued at the same time. Readwritesplit also didn't free the memory it
acquired via qc_get_table_names.
The function implemented redundant functionality and replacement with
modutil_get_next_MySQL_packet was planned.
When faced with a packet header spread over multiple buffers, the packet
length calculation would read past the buffer end. This is fixed by taking
modutil_get_next_MySQL_packet into use.
Identical behavior to the old function is achieved by calling
gwbuf_make_contiguous for each packet to store them in a contiguous area
of memory. This should be either removed and only done when
RCAP_TYPE_CONTIGUOUS_INPUT is requested or be made an innate feature of
statement based routing.
The debug assertion introduced by commit 3d1c2b421a fails when a
COM_CHANGE_USER was executed. This was caused by the fact that the
authentication data was being interpreted as a command when it should've
been ignored.
Added a debug assertion into the reauthentication code to make sure the
current command remains the same.
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.
The default database was not extracted correctly as the length of the
user's name did not include the null terminator. Also the comparison for
database name length used the smaller than operator instead of the correct
larger than operator.
When the client reauthenticates via COM_CHANGE_USER the new SHA1 needs to
be stored as the backend connections rely on it being up-to-date.
This commit fixes the regression of the mxs548_short_session_change_user
test.
The re-authentication done in MaxScale caused multiple error packets to be
sent for the same COM_CHANGE_USER. In addition to this, the failure of
authentication did not terminate the client connection.
The change in behavior requires the test case to be changed as well.
If a connection is killed but the backend DCBs have not yet received their
thread IDs, the connections can be forcibly closed. This removes the
possibility of stale connections caused by an unfortunately timed KILL
query to a session that has partially connected to some servers.
To support a wider range of client connectors, MaxScale should respond
with an AuthSwitchRequest packet to all COM_CHANGE_USER commands. Only
MariaDB connectors understand the OK packet as the only response to a
COM_CHANGE_USER but all connectors understand the AuthSwitchRequest
packet.
The mysql_create_standard_error function accepted a packet number as a
parameter but did not use it as the actual packet number. As the value it
used happened to coincide with 50% of the use-cases, it went unnoticed.
The remaining 50% occurred when a KILL command was executed with an
unknown connection ID.
The COM_STMT_FETCH command will create a response. This was a
readwritesplit-specific interpretation of the command and it was wrong.
Also record the currently executed command event for session commands.
The current command needs to be updated before the queries are actually
routed. This allows the KILL command detection and processing to correctly
work.
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.
When backend SSL connections were created, the connection creation was
done twice. This was due to the lacking detection of an already
established SSL connection.
If a DCB is closed before a response to the handshake packet is received,
the DCB's session will point to the dummy session. In this case no error
should be written to the DCB.
This is a cherry-pick of commit f53e112bf49766f1cc55516c2d7ee571461d483f
from the 2.2 branch.
The tests are now built by default. This should make it easier for users
to verify that they have a working MaxScale.
Also made the building of test_parse_kill conditional like the rest of the
tests.
By always starting the session shutdown process by stopping the client
DCB, the manipulation of the session state can be removed from the backend
protocol modules and replaced with a fake hangup event.
Delivering this event via the core allows the actual dcb_close call on the
client DCB to be done only when the client DCB is being handled by a
worker.
Directly closing the client DCB in the backend protocol modules is not
correct anymore as the state of the session doesn't change when the client
DCB is closed. By propagating the shutdown of the session with a fake
hangup to the client DCB, the closing of the DCB is done only once.
Added debug assertions that make sure all DCBs are closed only
once. Removed redundant code in the backend protocol error handling code.
If a DCB is closed before a response to the handshake packet is received,
the DCB's session will point to the dummy session. In this case no error
should be written to the DCB.
Make all modules lowercase and make module loading case
insensitive. Further, make command invocation case insensitive,
as far as the module name is conserned.
If a shallow copy of the buffer is made, any modifications that are made
to the data after it has been queued will affect the queued query of the
LocalClient.
A copy-on-write mechanism would save the relatively expensive process of
copying the data but since the LocalClient is not often used, it is not
the most critical performance problem.
The service for a dummy session will be NULL. If authentication fails for
a dummy session, then no service level actions should be taken.
Only the binlogrouter can trigger authentication failure with a dummy
session as it creates connections before the service itself has started.
The readqueue should never be explicitly assigned and should only ever be
appended to. This guarantees that the packets are read and processed in
the correct order.
Also removed an unused function that deals with the readqueue
manipulation.