Unlike readwritesplit, schemarouter will process all responses from
backends as if they are expected. There are cases where errors are
generated that aren't sent as a response to a query. These queries must be
ignored and not routed to the client. Copying the code as-is from
readwritesplit isn't the cleanest solution but it avoids refactoring code
in a patch release.
The custom error number (2003) used by the backend protocol code was not
an actual error number that the server would send. The error code in
question was for an error that only the C connector returns:
CR_CONN_HOST_ERROR. Using ER_CONNECTION_KILLED as the error number better
conveys the fact that the connection was killed due to a reason not
related to any ongoing query.
By using a known error number that is correctly handled, we also avoid
writing errors to the client in the middle of a resultset or as the
initial response to a result. This explains why the problem described in
MXS-3267 happened in the first place: an unrelated connection was lost in
the middle of a resultset and the error was interpreted as the end of a
resultset. As a result of there being more data to be read, the unexpected
result state messages were logged.
This could happen if a session command triggers a master reconnection and
the connection fails while the history replay is ongoing. The code assumed
that history replay would only happen when a query was in the query queue.
This correctly triggers the session command response processing to accept
results from other servers than the current master backend if the session
can continue. If the session cannot continue, it will be stopped
immediately.
The code used a null GWBUF with gwbuf_append which causes a crash. The
return value of the function that used it was also not correctly handled
and would be mistaken for a different error.
This allows the set of servers used by the service to also participate in
the cache value resolution. This will prevent the most obvious of problems
but any abstractions of the servers will prevent this from working.
Session commands did not trigger a reconnection process which caused
sessions to be closed in cases where recovery was possible.
Added a test case that verifies the patch fixes the problem.
If the session command could not be routed, the log message should contain
the actual command that was routed. This makes failure analysis much
easier.
If a limit on the replication lag is configured, servers with unmeasured
replication lag should not be used. The code in question did use them even
when a limit was set as the value used for undefined lag was -1 which
always measured lower than the limit.
The code relied on last_read for the idle time calculation which caused
the pings that were written to not reset the idle time. This increased the
chance of multiple COM_PING packets being sent to a backend before a reply
was received.
The use of the server state is not transactional across multiple uses of
the function. This means that any assertions on the target state can fail
if the monitor updates the state between target selection and the
assertion.
The slave backend would be closed twice if it would both respond with a
different result and be closed due to a hangup before the master
responded.
Added a test case that reproduced the problem.
