Bug #39629 Incorrect handling of LAST_INSERT_ID and FOUND_ROWS in rw-splitting.lua
Submitted: 24 Sep 2008 14:23 Modified: 16 Nov 2009 10:53
Reporter: Hristo Erinin Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Proxy: Scripts Severity:S2 (Serious)
Version:r511 OS:Any
Assigned to: Assigned Account CPU Architecture:Any
Tags: Contribution, found_rows, LAST_INSERT_ID, rw-splitting, rw-splitting.lua, sql_calc_found_rows
Triage: D3 (Medium)

[24 Sep 2008 14:23] Hristo Erinin
Description:
There are several bugs in rw-splitting.lua script that totally mess up processing of queries using LAST_INSERT_ID() and FOUND_ROWS. The problem with LAST_INSERT_ID is similar to the one described in http://bugs.mysql.com/bug.php?id=36505 (i.e. LAST_INSERT_ID() is TK_FUNCTION, not a literal) so queries containing this function go to slaves and hence return incorrect results.
The other problem arises from the lack of "affinity" of proxied connections - a query containing SQL_CALC_FOUND_ROWS hint can go to one slave, and when you actually execute a query with calc_found_rows() there is no way of knowing on which slave the previous query (containing the hint) has been executed. Instead the pooling mechanism just picks a connection from the pool. Thus it could return incorrect results, depending on the number of slaves and the connection pool.
I couldn't find a way to implement such an affinity, so in the included patch I've simply directed all SQL_CALC_FOUND_ROWS queries to the master.
Probably my fix will not work for multi-master setups, but my setup is with a single master and 5 slaves.

How to repeat:
Issue these commands directly against a properly replicating master:
> CREATE TABLE `test` (`a` int(1) DEFAULT NULL);
> insert into test values (1),(1),(1);
> select count(*) from test;
+----------+
| count(*) |
+----------+
|        3 |
+----------+
1 row in set (0.04 sec)

Then issue multiple times these queries to a mysql-proxy running the latest rw-splitting.lua scripts:
select SQL_CALC_FOUND_ROWS * from test limit 2;
select found_rows();
quit;

Depending on the size of the pool these queries have to be executed several times in order for the connection pool to fill up and mysql-proxy to actually start reusing the slaves.

Before my patch found_rows() returned randomly 0 (incorrect) and 3 (correct).

The same thing applies for LAST_INSERT_ID()... it returns random values depending on which slave it happens to run.

Suggested fix:
--- orig/rw-splitting.lua       2007-09-06 16:41:10.000000000 +0100
+++ new/rw-splitting.lua        2008-08-15 00:44:39.000000000 +0100
@@ -220,13 +220,14 @@
                                -- print("token: " .. token.token_name)
                                -- print("  val: " .. token.text)

-                               if not is_in_select_calc_found_rows and token.token_name == "TK_SQL_SQL_CALC_FOUND_ROWS" then
+                               if not is_in_select_calc_found_rows and (token.token_name == "TK_SQL_SQL_CALC_FOUND_ROWS" or (token.token_name == "TK_FUNCTION" and token.text:upper() == "FOUND_ROWS")) then
                                        is_in_select_calc_found_rows = true
+                               elseif not is_insert_id and token.token_name == "TK_FUNCTION" and token.text:upper() == "LAST_INSERT_ID" then
+                                       is_insert_id = true
                                elseif not is_insert_id and token.token_name == "TK_LITERAL" then
                                        local utext = token.text:upper()

-                                       if utext == "LAST_INSERT_ID" or
-                                          utext == "@@INSERT_ID" then
+                                       if utext == "@@INSERT_ID" then
                                                is_insert_id = true
                                        end
                                end
@@ -239,14 +240,15 @@

                        -- if we ask for the last-insert-id we have to ask it on the original
                        -- connection
-                       if not is_insert_id then
+                       if is_insert_id then
+                               print("   found a SELECT LAST_INSERT_ID(), going to master")
+                       elseif is_in_select_calc_found_rows then
+                               print("    need to calculate the rows, going to master")
+                       else -- neither of these two, pick an idle slave
                                local backend_ndx = lb.idle_ro()
-
                                if backend_ndx > 0 then
                                        proxy.connection.backend_ndx = backend_ndx
                                end
-                       else
-                               print("   found a SELECT LAST_INSERT_ID(), staying on the same backend")
                        end
                end
        end
[24 Sep 2008 14:24] Hristo Erinin
Patch to fix http://bugs.mysql.com/bug.php?id=39629

Attachment: rw-splitting-insert-found.diff (text/x-patch), 1.56 KiB.

[24 Sep 2008 16:06] Miguel Solorzano
Do you mean that the original pasted suggested fix isn't correct?. The actual patch is the attached file?. Thanks in advance.
[24 Sep 2008 21:10] Hristo Erinin
The pasted fix is the same as the attached file and is correct. It just got wrapped by the fixed width input field and is thus unusable. The attached file contains the diff agains r511 and should be directly applicable using patch(1).
[26 Sep 2008 8:28] Sveta Smirnova
Thank you for the report.

Verified as described. To repeat:

1. Start proxy as  ./src/mysql-proxy --proxy-backend-addresses=127.0.0.1:3351 --proxy-read-only-backend-addresses=127.0.0.1:3306  --proxy-read-only-backend-addresses=127.0.0.1:3350 --proxy-lua-script=./lib/rw-splitting.lua 

Table t1 exists only on 127.0.0.1:3351

2. Run following script in 3 terminals.

$cat bug39629.sh
#!/bin/bash

while (true); do php -r 'mysql_connect("127.0.0.1:4040", "root",""); mysql_select_db("test"); for($i=1; $i<=65; $i++) {mysql_query("select SQL_CALC_FOUND_ROWS * from t1 limit 2"); if (0 == mysql_error()) { $res = mysql_query("select found_rows()");var_dump(mysql_fetch_row($res));var_dump(mysql_error());}}'; done

3. Examine results files:

string(0) ""
array(1) {
  [0]=>
  string(1) "1"
}
string(0) ""
array(1) {
  [0]=>
  string(1) "1"
}
string(0) ""
array(1) {
  [0]=>
  string(1) "1"
}
string(0) ""
array(1) {
  [0]=>
  string(5) "98480"
}
string(0) ""
array(1) {
  [0]=>
  string(1) "1"
[8 Jun 2009 19:35] liz drachnik
Hello Hristo - 

Since you seem interested in contributing to MySQL, may I suggest that you sign the Sun|MySQL contributor agreement, i.e. the SCA.

The instructions are given here:
http://forge.mysql.com/wiki/Sun_Contributor_Agreement

I can review and turnaround your SCA request quite quickly. 

This will facilitate the handling of your contributions -- this one, and others in the future. 

Thank you ! 

Liz Drachnik - MySQL Program Manager.
[2 Oct 2009 23:00] Bugs System
No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".
[4 Nov 2009 0:00] Bugs System
No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".