toddstoffel
From our Slack discussion:

there's a couple of ways you could do it:

- send a COM_QUERY to the target server with a SELECT MASTER_GTID_WAIT(...) inside of it and wait for that to complete before routing the COM_STMT_EXECUTE or COM_STMT_PREPARE

- modify the COM_STMT_PREPARE to take an extra parameter so that the whole thing can be done with one command (this is the way the current causal_reads works with COM_QUERY)

the first option is pretty straightforward and only requires some additional states in the router

the latter would be the fastest but it's way more complex

First option roughly 2 weeks, second option 4 weeks. The second option, although faster, would be pretty complex and better avoided.

There's a third option that is a combination of the two: store the SQL for every open prepared statement and emulate the SQL modification by injecting a COM_STMT_PREPARE before it, modifying the ID of the COM_STMT_EXECUTE to refer to that (similar to how Connector-C does it) and appending a COM_STMT_CLOSE to it. This removes the need to modify the actual payload of the COM_STMT_EXECUTE and it would keep the latency about as low as the normal use of causal_reads.

The "downside" of this is that this is only doable with the use of a BEGIN NOT ATOMIC ... END block which means only MariaDB 10.2 and newer can be supported. Since causal_reads already partially relies on this, it's not a real limitation.

The SQL that would be used is:

BEGIN NOT ATOMIC 

  IF(MASTER_GTID_WAIT('0-1-123', 10) = 0) THEN 

    <copy SQL statement here>;

  ELSE 

    SIGNAL SQLSTATE '45000' SET MYSQL_ERRNO=30001, MESSAGE_TEXT='GTID wait timed out';

  END IF;

END;

The user-provided SQL must be trimmed of any trailing semicolons to prevent syntax errors from occurring.

From an effort point of view, this would take the same amount of work as the naive COM_QUERY implementation with some extra work that would have to be done on the protocol layer: the current implementation buffers input until the COM_STMT_PREPARE completes as the prepared statement mapping was moved into the backend protocol. This should not be a complex thing to implement.

Option 3 makes the most sense to me as well. The only question I have is about this segment:

The user-provided SQL must be trimmed of any trailing semicolons to prevent syntax errors from occurring.

Is that something Maxscale will do or is it expected to be cleaned up before it enters maxscale?

The SQL cleanup has to be at least attempted by MaxScale. As in most cases the user-provided SQL either contains no trailing semicolons or it only has one, we can quite easily walk the SQL string backwards and just trim off whitespace until we see a semicolon. This of course won't work correctly when an end-of-the-line style comment (# or --) is used and the SQL before it has a semicolon before the comment starts but I'd expect these to be less common.

After looking into the alternative implementation that uses the BEGIN NOT ATOMIC block to perform the synchronization in one step, I found a problem in it that would make it troublesome to implement neatly. Whenever the SQL statement uses a conditional of some sorts, it returns an extra OK packet in the result as if a stored procedure was being executed. As the client who executed the query doesn't expect the SQL to return an extra OK packet, readwritesplit is forced to manipulate the result so that this trailing OK packet is removed and the MORE_RESULTS flag of the last result is cleared. Additional problems arise for any commands that are qualified for causal reads but do not generate a resultset: in this case there is no extra OK packet as the protocol collapses multiple OK packets into one and readwritesplit would have to account for this as well.

After figuring out that this approach wasn't as simple as it initially seemed to be, I proceeded to look for alternative ways to do this. The next solution was to send a COM_QUERY packet before the COM_STMT_EXECUTE with the following SQL:

IF (MASTER_GTID_WAIT('0-1-123', 10) <> 0) THEN

  KILL (SELECT CONNECTION_ID());

END IF

The SQL will attempt to synchronize the connection with a known GTID position. If the synchronization is successful, the server will respond with an OK packet which readwritesplit will discard. If the synchronization fails, the KILL closes the connection, thus preventing the COM_STMT_EXECUTE that follows the COM_QUERY from being executed. Latency-wise this should be roughly equal to both the BEGIN NOT ATOMIC version as well as the multi-statement version that is currently used for normal SQL statements. In the success case this should be slightly more efficient than the BEGIN NOT ATOMIC version would as it does not have to create the prepared statement, execute it and then close it. The downside of this approach is the heavy cost of failure as the whole connection has to be recreated if the failed server is to be used again.

Another interesting feature of this approach is that the code required to implement this in 2.6 is very small: the response ignoring code already exists and the re-routing of the failed query to the master can be achieved by injecting a routing hint into the backup copy of the original COM_STMT_EXECUTE. This means that only the SQL generation code has to be added and that's roughly 40 lines of code.

The final implementation used the one described earlier: a COM_QUERY is sent before the COM_STMT_EXECUTE which terminates the connection if it times out. If it doesn't, the OK packet returned by the COM_QUERY is discarded.