From 7c3a354fd86bae8b6edc9c45017c6520545d5a3b Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Fri, 28 Mar 2014 00:16:18 +0200 Subject: [PATCH] Query classifier ignored implicit commits in cases of write commands. Fixed it. Added more tests for transaction support. Mostly different cases where some command triggers implicit commit in the middle of transaction. --- query_classifier/query_classifier.cc | 72 +++++++++---------- .../routing/readwritesplit/readwritesplit.c | 13 ++-- .../routing/readwritesplit/test/makefile | 2 +- .../routing/readwritesplit/test/rwsplit.sh | 64 +++++++++++++++++ .../test/test_implicit_commit1.sql | 8 +++ .../test/test_implicit_commit2.sql | 15 ++++ .../test/test_implicit_commit3.sql | 9 +++ .../test/test_implicit_commit4.sql | 9 +++ .../test/test_implicit_commit5.sql | 14 ++++ .../test/test_implicit_commit6.sql | 11 +++ .../test/test_implicit_commit7.sql | 10 +++ 11 files changed, 184 insertions(+), 43 deletions(-) create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit1.sql create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit2.sql create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit3.sql create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit4.sql create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit5.sql create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit6.sql create mode 100644 server/modules/routing/readwritesplit/test/test_implicit_commit7.sql diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 410f47d61..1976bd6b2 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -396,34 +396,7 @@ static skygw_query_type_t resolve_query_type( type = QUERY_TYPE_SESSION_WRITE; goto return_qtype; } - /** - * 1:ALTER TABLE, TRUNCATE, REPAIR, OPTIMIZE, ANALYZE, CHECK. - * 2:CREATE|ALTER|DROP|TRUNCATE|RENAME TABLE, LOAD, CREATE|DROP|ALTER DB, - * CREATE|DROP INDEX, CREATE|DROP VIEW, CREATE|DROP TRIGGER, - * CREATE|ALTER|DROP EVENT, UPDATE, INSERT, INSERT(SELECT), - * DELETE, REPLACE, REPLACE(SELECT), CREATE|RENAME|DROP USER, - * GRANT, REVOKE, OPTIMIZE, CREATE|ALTER|DROP FUNCTION|PROCEDURE, - * CREATE SPFUNCTION, INSTALL|UNINSTALL PLUGIN - */ - if (is_log_table_write_query(lex->sql_command) || - is_update_query(lex->sql_command)) - { - if (thd->variables.sql_log_bin == 0 && - force_data_modify_op_replication) - { - type |= QUERY_TYPE_SESSION_WRITE; - } else { - type |= QUERY_TYPE_WRITE; - } - - goto return_qtype; - } - - /** - * REVOKE ALL, ASSIGN_TO_KEYCACHE, - * PRELOAD_KEYS, FLUSH, RESET, CREATE|ALTER|DROP SERVER - * SET autocommit, various other SET commands. - */ + if (skygw_stmt_causes_implicit_commit(lex, CF_AUTO_COMMIT_TRANS)) { if (LOG_IS_ENABLED(LOGFILE_TRACE)) @@ -446,21 +419,47 @@ static skygw_query_type_t resolve_query_type( } } type |= QUERY_TYPE_COMMIT; - - if (lex->option_type == OPT_GLOBAL) + } + /** + * REVOKE ALL, ASSIGN_TO_KEYCACHE, + * PRELOAD_KEYS, FLUSH, RESET, CREATE|ALTER|DROP SERVER + */ + if (lex->option_type == OPT_GLOBAL) + { + type |= QUERY_TYPE_GLOBAL_WRITE; + goto return_qtype; + } + else if (lex->option_type == OPT_SESSION) + { + type |= QUERY_TYPE_SESSION_WRITE; + goto return_qtype; + } + /** + * 1:ALTER TABLE, TRUNCATE, REPAIR, OPTIMIZE, ANALYZE, CHECK. + * 2:CREATE|ALTER|DROP|TRUNCATE|RENAME TABLE, LOAD, CREATE|DROP|ALTER DB, + * CREATE|DROP INDEX, CREATE|DROP VIEW, CREATE|DROP TRIGGER, + * CREATE|ALTER|DROP EVENT, UPDATE, INSERT, INSERT(SELECT), + * DELETE, REPLACE, REPLACE(SELECT), CREATE|RENAME|DROP USER, + * GRANT, REVOKE, OPTIMIZE, CREATE|ALTER|DROP FUNCTION|PROCEDURE, + * CREATE SPFUNCTION, INSTALL|UNINSTALL PLUGIN + */ + if (is_log_table_write_query(lex->sql_command) || + is_update_query(lex->sql_command)) + { + if (thd->variables.sql_log_bin == 0 && + force_data_modify_op_replication) { - type |= QUERY_TYPE_GLOBAL_WRITE; - } - else if (lex->option_type == OPT_SESSION) - { - type |= QUERY_TYPE_SESSION_WRITE; + type |= QUERY_TYPE_SESSION_WRITE; + } else { + type |= QUERY_TYPE_WRITE; } + goto return_qtype; } /** Try to catch session modifications here */ switch (lex->sql_command) { - case SQLCOM_SET_OPTION: + case SQLCOM_SET_OPTION: /*< SET commands. */ if (lex->option_type == OPT_GLOBAL) { type |= QUERY_TYPE_GLOBAL_WRITE; @@ -672,6 +671,7 @@ static bool skygw_stmt_causes_implicit_commit(LEX* lex, uint mask) succp = lex->autocommit ? true : false; break; default: + succp = true; break; } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 15b1295af..5383c1eb6 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -670,6 +670,13 @@ static int routeQuery( "Packet type\t%s", STRPACKETTYPE(packet_type)))); + if (QUERY_IS_TYPE(qtype,QUERY_TYPE_COMMIT) && + transaction_active) + { + transaction_active = false; + } + + switch (qtype) { case QUERY_TYPE_WRITE: LOGIF(LT, (skygw_log_write( @@ -724,12 +731,6 @@ static int routeQuery( break; case QUERY_TYPE_SESSION_WRITE: - case (QUERY_TYPE_SESSION_WRITE|QUERY_TYPE_COMMIT): - if (QUERY_IS_TYPE(qtype,QUERY_TYPE_COMMIT) && - transaction_active) - { - transaction_active = false; - } /** * Execute in backends used by current router session. * Save session variable commands to router session property diff --git a/server/modules/routing/readwritesplit/test/makefile b/server/modules/routing/readwritesplit/test/makefile index 5c72df1aa..096e28fe7 100644 --- a/server/modules/routing/readwritesplit/test/makefile +++ b/server/modules/routing/readwritesplit/test/makefile @@ -30,7 +30,7 @@ runtests: @echo $(shell date) >> $(TESTLOG) @echo "Test MaxScale R/W Split" >> $(TESTLOG) @echo "-------------------------------" >> $(TESTLOG) - ./rwsplit.sh $(TESTLOG) $(THOST) $(TPORT_RW) $(TMASTER_ID) $(TUSER) $(TPWD) + ./rwsplit.sh $(TESTLOG) $(THOST) $(TPORT_RW) $(TMASTER_ID) $(TUSER) $(TPWD) @echo "" >> $(TESTLOG) diff --git a/server/modules/routing/readwritesplit/test/rwsplit.sh b/server/modules/routing/readwritesplit/test/rwsplit.sh index 15f41c50d..4755e4526 100755 --- a/server/modules/routing/readwritesplit/test/rwsplit.sh +++ b/server/modules/routing/readwritesplit/test/rwsplit.sh @@ -49,3 +49,67 @@ if [ "$a" != "$TRETVAL" ]; then else echo "$TINPUT PASSED">>$TLOG ; fi + +TINPUT=test_implicit_commit1.sql +TRETVAL=$TMASTER_ID + +a=`$RUNCMD < ./$TINPUT` +if [ "$a" == "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when it was not accetable">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi + +TINPUT=test_implicit_commit2.sql +TRETVAL=$TMASTER_ID +a=`$RUNCMD < ./$TINPUT` +if [ "$a" == "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when it was not accetable">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi + +TINPUT=test_implicit_commit3.sql +TRETVAL=$TMASTER_ID +a=`$RUNCMD < ./$TINPUT` +if [ "$a" == "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when it was not accetable">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi + +TINPUT=test_implicit_commit4.sql +TRETVAL=$TMASTER_ID +a=`$RUNCMD < ./$TINPUT` +if [ "$a" != "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when $TRETVAL was expected">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi + +TINPUT=test_implicit_commit5.sql +TRETVAL=$TMASTER_ID +a=`$RUNCMD < ./$TINPUT` +if [ "$a" == "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when it was not accetable">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi + +TINPUT=test_implicit_commit6.sql +TRETVAL=$TMASTER_ID +a=`$RUNCMD < ./$TINPUT` +if [ "$a" == "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when it was not accetable">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi + +TINPUT=test_implicit_commit7.sql +TRETVAL=$TMASTER_ID +a=`$RUNCMD < ./$TINPUT` +if [ "$a" == "$TRETVAL" ]; then + echo "$TINPUT FAILED, return value $a when it was not accetable">>$TLOG; +else + echo "$TINPUT PASSED">>$TLOG ; +fi diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit1.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit1.sql new file mode 100644 index 000000000..663c32efa --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit1.sql @@ -0,0 +1,8 @@ +DROP DATABASE If EXISTS FOO; +SET autocommit=0; +BEGIN; +CREATE DATABASE FOO; -- implicit commit +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from slave +DROP DATABASE If EXISTS FOO; +COMMIT; diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit2.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit2.sql new file mode 100644 index 000000000..80f0ebad1 --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit2.sql @@ -0,0 +1,15 @@ +USE test; +DROP TABLE IF EXISTS T1; +DROP EVENT IF EXISTS myevent; +SET autocommit=0; +BEGIN; +CREATE TABLE T1 (id integer); +CREATE EVENT myevent +ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR +DO +UPDATE t1 SET id = id + 1; +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from slave +DROP TABLE IF EXISTS T1; +DROP EVENT IF EXISTS myevent; +COMMIT; diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit3.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit3.sql new file mode 100644 index 000000000..4db36fdd5 --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit3.sql @@ -0,0 +1,9 @@ +USE test; +DROP TABLE IF EXISTS T1; +SET autocommit=0; +BEGIN; +CREATE TABLE T1 (id integer); -- implicit commit +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from slave +DROP TABLE IF EXISTS T1; +COMMIT; diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit4.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit4.sql new file mode 100644 index 000000000..f7fe048be --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit4.sql @@ -0,0 +1,9 @@ +USE test; +DROP TABLE IF EXISTS T1; +SET autocommit=0; +BEGIN; +CREATE TEMPORARY TABLE T1 (id integer); -- NO implicit commit +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from master +DROP TABLE IF EXISTS T1; +COMMIT; diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit5.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit5.sql new file mode 100644 index 000000000..3656f1af8 --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit5.sql @@ -0,0 +1,14 @@ +USE test; +DROP PROCEDURE IF EXISTS simpleproc; +SET autocommit=0; +BEGIN; +DELIMITER // +CREATE PROCEDURE simpleproc (OUT param1 INT) +BEGIN + SELECT COUNT(*) INTO param1 FROM t; +END // +DELIMITER ; +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from slave +DROP PROCEDURE IF EXISTS simpleproc; +COMMIT; diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit6.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit6.sql new file mode 100644 index 000000000..ba896966d --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit6.sql @@ -0,0 +1,11 @@ +USE test; +DROP FUNCTION IF EXISTS hello; +SET autocommit=0; +BEGIN; +CREATE FUNCTION hello (s CHAR(20)) +RETURNS CHAR(50) DETERMINISTIC +RETURN CONCAT('Hello, ',s,'!'); -- implicit COMMIT +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from slave +DROP FUNCTION IF EXISTS hello; +COMMIT; diff --git a/server/modules/routing/readwritesplit/test/test_implicit_commit7.sql b/server/modules/routing/readwritesplit/test/test_implicit_commit7.sql new file mode 100644 index 000000000..9b3c5bac7 --- /dev/null +++ b/server/modules/routing/readwritesplit/test/test_implicit_commit7.sql @@ -0,0 +1,10 @@ +USE test; +DROP TABLE IF EXISTS T1; +CREATE TABLE T1 (id integer); -- implicit commit +SET autocommit=0; +BEGIN; +CREATE INDEX foo_t1 on T1 (id); -- implicit commit +SELECT (@@server_id) INTO @a; +SELECT @a; --should read from slave +DROP TABLE IF EXISTS T1; +COMMIT;