Fix crash on trx replay with session command

Readwritesplit would crash with the following transaction:

    BEGIN;
    SET @a = 1; -- This is where it would crash
    COMMIT;

When a session command was a part of the transaction, empty queries
(i.e. NULL GWBUFs) would be added to the transaction. If the transaction
were to be replayed, MaxScale would crash when these NULL queries were
executed.

Once the empty responses were fixed, the replaying of the transaction
would fail with a checksum mismatch. This was caused by the wrong order of
processing in RWSplitSession::clientReply. The response processing for
session commands was done after the response processing for replayed
transactions. This would trigger a checksum comparison too early for the
transaction in question.
This commit is contained in:
Markus Mäkelä 2018-06-22 22:56:12 +03:00
parent 9737962add
commit 93fdada534
No known key found for this signature in database
GPG Key ID: 72D48FCE664F7B19
2 changed files with 44 additions and 22 deletions

View File

@ -481,6 +481,19 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb)
if (m_config.transaction_replay && m_can_replay_trx &&
session_trx_is_active(m_client->session))
{
if (!backend->has_session_commands())
{
/**
* Session commands are tracked separately from the transaction.
* We must not put any response to a session command into
* the transaction as they are tracked separately.
*
* TODO: It might be wise to include the session commands to guarantee
* that the session state during the transaction replay remains
* consistent if the state change in the middle of the transaction
* is intentional.
*/
size_t size{m_trx.size() + m_current_query.length()};
// A transaction is open and it is eligible for replaying
if (size < m_config.trx_max_size)
@ -498,6 +511,7 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb)
m_can_replay_trx = false;
}
}
}
else
{
m_current_query.reset();
@ -531,7 +545,13 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb)
m_expected_responses, backend->name());
}
if (m_is_replay_active)
if (backend->has_session_commands())
{
/** Process the reply to an executed session command. This function can
* close the backend if it's a slave. */
process_sescmd_response(backend, &writebuf);
}
else if (m_is_replay_active)
{
ss_dassert(m_config.transaction_replay);
handle_trx_replay();
@ -561,13 +581,6 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb)
m_can_replay_trx = true;
}
if (backend->has_session_commands())
{
/** Process the reply to an executed session command. This function can
* close the backend if it's a slave. */
process_sescmd_response(backend, &writebuf);
}
if (backend->in_use() && backend->has_session_commands())
{
// Backend is still in use and has more session commands to execute

View File

@ -18,6 +18,7 @@
#include <maxscale/buffer.hh>
#include <maxscale/utils.hh>
#include <maxscale/modutil.hh>
// A transaction
class Trx
@ -38,6 +39,13 @@ public:
*/
void add_stmt(GWBUF* buf)
{
ss_info_dassert(buf, "Trx::add_stmt: Buffer must not be empty");
if (MXS_LOG_PRIORITY_IS_ENABLED(LOG_INFO))
{
MXS_INFO("Adding to trx: %s", mxs::extract_sql(buf, 512).c_str());
}
m_size += gwbuf_length(buf);
m_log.emplace_back(buf);
}
@ -123,6 +131,7 @@ public:
*/
void close()
{
MXS_INFO("Transaction is complete");
m_checksum.reset();
m_log.clear();
m_size = 0;