diff options
| author | unknown <malff/marcsql@weblab.(none)> | 2007-01-02 14:18:13 -0700 |
|---|---|---|
| committer | unknown <malff/marcsql@weblab.(none)> | 2007-01-02 14:18:13 -0700 |
| commit | d7577ecb7d455d9a326a7543c8ef1a81e59150d1 (patch) | |
| tree | 1a43bc5c6b21902a943e0a8759f9f36c9c407c1c | |
| parent | f20cb8d0379264eef566c6b91fe2aa03017789ba (diff) | |
| parent | 552d4cede57926ecfba91624e9ef2a72cf1bd81f (diff) | |
| download | mariadb-git-d7577ecb7d455d9a326a7543c8ef1a81e59150d1.tar.gz | |
Merge weblab.(none):/home/marcsql/TREE/mysql-5.1-base
into weblab.(none):/home/marcsql/TREE/mysql-5.1-merge
mysql-test/r/sp.result:
Auto merged
server-tools/instance-manager/commands.cc:
Auto merged
server-tools/instance-manager/commands.h:
Auto merged
server-tools/instance-manager/guardian.cc:
Auto merged
server-tools/instance-manager/guardian.h:
Auto merged
server-tools/instance-manager/instance.cc:
Auto merged
server-tools/instance-manager/instance.h:
Auto merged
server-tools/instance-manager/instance_map.cc:
Auto merged
server-tools/instance-manager/instance_map.h:
Auto merged
server-tools/instance-manager/instance_options.h:
Auto merged
server-tools/instance-manager/listener.cc:
Auto merged
server-tools/instance-manager/manager.cc:
Auto merged
server-tools/instance-manager/manager.h:
Auto merged
server-tools/instance-manager/user_map.cc:
Auto merged
sql/event_data_objects.cc:
Auto merged
sql/event_queue.cc:
Auto merged
sql/handler.cc:
Auto merged
sql/lock.cc:
Auto merged
sql/set_var.cc:
Auto merged
sql/set_var.h:
Auto merged
sql/sp_head.cc:
Auto merged
sql/sp_head.h:
Auto merged
sql/sql_class.cc:
Auto merged
sql/sql_class.h:
Auto merged
sql/sql_lex.cc:
Auto merged
sql/sql_lex.h:
Auto merged
sql/sql_prepare.cc:
Auto merged
sql/sql_yacc.yy:
Auto merged
storage/csv/ha_tina.cc:
Auto merged
tests/mysql_client_test.c:
Auto merged
44 files changed, 3263 insertions, 1169 deletions
diff --git a/mysql-test/r/csv.result b/mysql-test/r/csv.result index 32ca47e20d2..12cf900046f 100644 --- a/mysql-test/r/csv.result +++ b/mysql-test/r/csv.result @@ -5223,3 +5223,11 @@ check table bug22080_3; Table Op Msg_type Msg_text test.bug22080_3 check error Corrupt drop tables bug22080_1,bug22080_2,bug22080_3; +CREATE TABLE `bug21328` ( +`col1` int(11) DEFAULT NULL, +`col2` int(11) DEFAULT NULL, +`col3` int(11) DEFAULT NULL +) ENGINE=CSV; +insert into bug21328 values (1,NULL,NULL); +alter table bug21328 engine=myisam; +drop table bug21328; diff --git a/mysql-test/r/log_tables.result b/mysql-test/r/log_tables.result index 3e8b7d32e88..8264f252287 100644 --- a/mysql-test/r/log_tables.result +++ b/mysql-test/r/log_tables.result @@ -111,9 +111,6 @@ slow_log CREATE TABLE `slow_log` ( ) ENGINE=CSV DEFAULT CHARSET=utf8 COMMENT='Slow log' alter table mysql.general_log engine=myisam; alter table mysql.slow_log engine=myisam; -Warnings: -Warning 1366 Incorrect integer value: '' for column 'last_insert_id' at row 0 -Warning 1366 Incorrect integer value: '' for column 'insert_id' at row 0 show create table mysql.general_log; Table Create Table general_log CREATE TABLE `general_log` ( diff --git a/mysql-test/r/read_only.result b/mysql-test/r/read_only.result index 69d25fbef6f..f270f1ed5ad 100644 --- a/mysql-test/r/read_only.result +++ b/mysql-test/r/read_only.result @@ -39,11 +39,61 @@ delete t1 from t1,t3 where t1.a=t3.a; drop table t1; insert into t1 values(1); ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +set global read_only=0; +lock table t1 write; +lock table t2 write; +set global read_only=1; +ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction +unlock tables ; +set global read_only=1; +select @@global.read_only; +@@global.read_only +0 +unlock tables ; +select @@global.read_only; +@@global.read_only +1 +set global read_only=0; +lock table t1 read; +lock table t2 read; +set global read_only=1; +ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction +unlock tables ; +set global read_only=1; +select @@global.read_only; +@@global.read_only +0 +unlock tables ; +select @@global.read_only; +@@global.read_only +1 +set global read_only=0; +BEGIN; +BEGIN; +set global read_only=1; +ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction +ROLLBACK; +set global read_only=1; +select @@global.read_only; +@@global.read_only +1 +ROLLBACK; +set global read_only=0; +flush tables with read lock; +set global read_only=1; +unlock tables; +set global read_only=0; +flush tables with read lock; +set global read_only=1; +select @@global.read_only; +@@global.read_only +1 +unlock tables; drop temporary table ttt; ERROR 42S02: Unknown table 'ttt' drop temporary table if exists ttt; Warnings: Note 1051 Unknown table 'ttt' +set global read_only=0; drop table t1,t2; drop user test@localhost; -set global read_only=0; diff --git a/mysql-test/r/read_only_innodb.result b/mysql-test/r/read_only_innodb.result new file mode 100644 index 00000000000..d028e3cc207 --- /dev/null +++ b/mysql-test/r/read_only_innodb.result @@ -0,0 +1,18 @@ +DROP TABLE IF EXISTS table_11733 ; +grant CREATE, SELECT, DROP on *.* to test@localhost; +set global read_only=0; +create table table_11733 (a int) engine=InnoDb; +BEGIN; +insert into table_11733 values(11733); +set global read_only=1; +select @@global.read_only; +@@global.read_only +1 +select * from table_11733 ; +a +11733 +COMMIT; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +set global read_only=0; +drop table table_11733 ; +drop user test@localhost; diff --git a/mysql-test/r/rpl_read_only.result b/mysql-test/r/rpl_read_only.result new file mode 100644 index 00000000000..0b06a3d414a --- /dev/null +++ b/mysql-test/r/rpl_read_only.result @@ -0,0 +1,113 @@ +stop slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +reset master; +reset slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +start slave; +create table t1(a int) engine=InnoDB; +create table t2(a int) engine=MyISAM; +insert into t1 values(1001); +insert into t2 values(2001); +set global read_only=1; +select @@read_only; +@@read_only +1 +select * from t1; +a +1001 +select * from t2; +a +2001 +select @@read_only; +@@read_only +0 +select * from t1; +a +1001 +select * from t2; +a +2001 +set global read_only=0; +BEGIN; +insert into t1 values(1002); +insert into t2 values(2002); +BEGIN; +insert into t1 values(1003); +insert into t2 values(2003); +set global read_only=1; +COMMIT; +COMMIT; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +set global read_only=0; +insert into t1 values(1004); +insert into t2 values(2004); +select * from t1; +a +1001 +1002 +1004 +select * from t2; +a +2001 +2002 +2003 +2004 +select * from t1; +a +1001 +1002 +1004 +select * from t2; +a +2001 +2002 +2003 +2004 +set global read_only=1; +select @@read_only; +@@read_only +1 +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +show create table t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1005); +insert into t2 values(2005); +select * from t1; +a +1001 +1002 +1004 +1005 +select * from t2; +a +2001 +2002 +2003 +2004 +2005 +select * from t1; +a +1001 +1002 +1004 +1005 +select * from t2; +a +2001 +2002 +2003 +2004 +2005 +insert into t1 values(1006); +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +insert into t2 values(2006); +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +drop table t1; +drop table t2; diff --git a/mysql-test/r/sp-code.result b/mysql-test/r/sp-code.result index 4ae38861d29..0b0ad802b54 100644 --- a/mysql-test/r/sp-code.result +++ b/mysql-test/r/sp-code.result @@ -199,6 +199,421 @@ Pos Instruction 44 jump 14 45 stmt 9 "drop temporary table sudoku_work, sud..." drop procedure sudoku_solve; +DROP PROCEDURE IF EXISTS proc_19194_simple; +DROP PROCEDURE IF EXISTS proc_19194_searched; +DROP PROCEDURE IF EXISTS proc_19194_nested_1; +DROP PROCEDURE IF EXISTS proc_19194_nested_2; +DROP PROCEDURE IF EXISTS proc_19194_nested_3; +DROP PROCEDURE IF EXISTS proc_19194_nested_4; +CREATE PROCEDURE proc_19194_simple(i int) +BEGIN +DECLARE str CHAR(10); +CASE i +WHEN 1 THEN SET str="1"; +WHEN 2 THEN SET str="2"; +WHEN 3 THEN SET str="3"; +ELSE SET str="unknown"; +END CASE; +SELECT str; +END| +CREATE PROCEDURE proc_19194_searched(i int) +BEGIN +DECLARE str CHAR(10); +CASE +WHEN i=1 THEN SET str="1"; +WHEN i=2 THEN SET str="2"; +WHEN i=3 THEN SET str="3"; +ELSE SET str="unknown"; +END CASE; +SELECT str; +END| +CREATE PROCEDURE proc_19194_nested_1(i int, j int) +BEGIN +DECLARE str_i CHAR(10); +DECLARE str_j CHAR(10); +CASE i +WHEN 10 THEN SET str_i="10"; +WHEN 20 THEN +BEGIN +set str_i="20"; +CASE +WHEN j=1 THEN SET str_j="1"; +WHEN j=2 THEN SET str_j="2"; +WHEN j=3 THEN SET str_j="3"; +ELSE SET str_j="unknown"; +END CASE; +select "i was 20"; +END; +WHEN 30 THEN SET str_i="30"; +WHEN 40 THEN SET str_i="40"; +ELSE SET str_i="unknown"; +END CASE; +SELECT str_i, str_j; +END| +CREATE PROCEDURE proc_19194_nested_2(i int, j int) +BEGIN +DECLARE str_i CHAR(10); +DECLARE str_j CHAR(10); +CASE +WHEN i=10 THEN SET str_i="10"; +WHEN i=20 THEN +BEGIN +set str_i="20"; +CASE j +WHEN 1 THEN SET str_j="1"; +WHEN 2 THEN SET str_j="2"; +WHEN 3 THEN SET str_j="3"; +ELSE SET str_j="unknown"; +END CASE; +select "i was 20"; +END; +WHEN i=30 THEN SET str_i="30"; +WHEN i=40 THEN SET str_i="40"; +ELSE SET str_i="unknown"; +END CASE; +SELECT str_i, str_j; +END| +CREATE PROCEDURE proc_19194_nested_3(i int, j int) +BEGIN +DECLARE str_i CHAR(10); +DECLARE str_j CHAR(10); +CASE i +WHEN 10 THEN SET str_i="10"; +WHEN 20 THEN +BEGIN +set str_i="20"; +CASE j +WHEN 1 THEN SET str_j="1"; +WHEN 2 THEN SET str_j="2"; +WHEN 3 THEN SET str_j="3"; +ELSE SET str_j="unknown"; +END CASE; +select "i was 20"; +END; +WHEN 30 THEN SET str_i="30"; +WHEN 40 THEN SET str_i="40"; +ELSE SET str_i="unknown"; +END CASE; +SELECT str_i, str_j; +END| +CREATE PROCEDURE proc_19194_nested_4(i int, j int) +BEGIN +DECLARE str_i CHAR(10); +DECLARE str_j CHAR(10); +CASE +WHEN i=10 THEN SET str_i="10"; +WHEN i=20 THEN +BEGIN +set str_i="20"; +CASE +WHEN j=1 THEN SET str_j="1"; +WHEN j=2 THEN SET str_j="2"; +WHEN j=3 THEN SET str_j="3"; +ELSE SET str_j="unknown"; +END CASE; +select "i was 20"; +END; +WHEN i=30 THEN SET str_i="30"; +WHEN i=40 THEN SET str_i="40"; +ELSE SET str_i="unknown"; +END CASE; +SELECT str_i, str_j; +END| +SHOW PROCEDURE CODE proc_19194_simple; +Pos Instruction +0 set str@1 NULL +1 set_case_expr (12) 0 i@0 +2 jump_if_not 5(12) (case_expr@0 = 1) +3 set str@1 _latin1'1' +4 jump 12 +5 jump_if_not 8(12) (case_expr@0 = 2) +6 set str@1 _latin1'2' +7 jump 12 +8 jump_if_not 11(12) (case_expr@0 = 3) +9 set str@1 _latin1'3' +10 jump 12 +11 set str@1 _latin1'unknown' +12 stmt 0 "SELECT str" +SHOW PROCEDURE CODE proc_19194_searched; +Pos Instruction +0 set str@1 NULL +1 jump_if_not 4(11) (i@0 = 1) +2 set str@1 _latin1'1' +3 jump 11 +4 jump_if_not 7(11) (i@0 = 2) +5 set str@1 _latin1'2' +6 jump 11 +7 jump_if_not 10(11) (i@0 = 3) +8 set str@1 _latin1'3' +9 jump 11 +10 set str@1 _latin1'unknown' +11 stmt 0 "SELECT str" +SHOW PROCEDURE CODE proc_19194_nested_1; +Pos Instruction +0 set str_i@2 NULL +1 set str_j@3 NULL +2 set_case_expr (27) 0 i@0 +3 jump_if_not 6(27) (case_expr@0 = 10) +4 set str_i@2 _latin1'10' +5 jump 27 +6 jump_if_not 20(27) (case_expr@0 = 20) +7 set str_i@2 _latin1'20' +8 jump_if_not 11(18) (j@1 = 1) +9 set str_j@3 _latin1'1' +10 jump 18 +11 jump_if_not 14(18) (j@1 = 2) +12 set str_j@3 _latin1'2' +13 jump 18 +14 jump_if_not 17(18) (j@1 = 3) +15 set str_j@3 _latin1'3' +16 jump 18 +17 set str_j@3 _latin1'unknown' +18 stmt 0 "select "i was 20"" +19 jump 27 +20 jump_if_not 23(27) (case_expr@0 = 30) +21 set str_i@2 _latin1'30' +22 jump 27 +23 jump_if_not 26(27) (case_expr@0 = 40) +24 set str_i@2 _latin1'40' +25 jump 27 +26 set str_i@2 _latin1'unknown' +27 stmt 0 "SELECT str_i, str_j" +SHOW PROCEDURE CODE proc_19194_nested_2; +Pos Instruction +0 set str_i@2 NULL +1 set str_j@3 NULL +2 jump_if_not 5(27) (i@0 = 10) +3 set str_i@2 _latin1'10' +4 jump 27 +5 jump_if_not 20(27) (i@0 = 20) +6 set str_i@2 _latin1'20' +7 set_case_expr (18) 0 j@1 +8 jump_if_not 11(18) (case_expr@0 = 1) +9 set str_j@3 _latin1'1' +10 jump 18 +11 jump_if_not 14(18) (case_expr@0 = 2) +12 set str_j@3 _latin1'2' +13 jump 18 +14 jump_if_not 17(18) (case_expr@0 = 3) +15 set str_j@3 _latin1'3' +16 jump 18 +17 set str_j@3 _latin1'unknown' +18 stmt 0 "select "i was 20"" +19 jump 27 +20 jump_if_not 23(27) (i@0 = 30) +21 set str_i@2 _latin1'30' +22 jump 27 +23 jump_if_not 26(27) (i@0 = 40) +24 set str_i@2 _latin1'40' +25 jump 27 +26 set str_i@2 _latin1'unknown' +27 stmt 0 "SELECT str_i, str_j" +SHOW PROCEDURE CODE proc_19194_nested_3; +Pos Instruction +0 set str_i@2 NULL +1 set str_j@3 NULL +2 set_case_expr (28) 0 i@0 +3 jump_if_not 6(28) (case_expr@0 = 10) +4 set str_i@2 _latin1'10' +5 jump 28 +6 jump_if_not 21(28) (case_expr@0 = 20) +7 set str_i@2 _latin1'20' +8 set_case_expr (19) 1 j@1 +9 jump_if_not 12(19) (case_expr@1 = 1) +10 set str_j@3 _latin1'1' +11 jump 19 +12 jump_if_not 15(19) (case_expr@1 = 2) +13 set str_j@3 _latin1'2' +14 jump 19 +15 jump_if_not 18(19) (case_expr@1 = 3) +16 set str_j@3 _latin1'3' +17 jump 19 +18 set str_j@3 _latin1'unknown' +19 stmt 0 "select "i was 20"" +20 jump 28 +21 jump_if_not 24(28) (case_expr@0 = 30) +22 set str_i@2 _latin1'30' +23 jump 28 +24 jump_if_not 27(28) (case_expr@0 = 40) +25 set str_i@2 _latin1'40' +26 jump 28 +27 set str_i@2 _latin1'unknown' +28 stmt 0 "SELECT str_i, str_j" +SHOW PROCEDURE CODE proc_19194_nested_4; +Pos Instruction +0 set str_i@2 NULL +1 set str_j@3 NULL +2 jump_if_not 5(26) (i@0 = 10) +3 set str_i@2 _latin1'10' +4 jump 26 +5 jump_if_not 19(26) (i@0 = 20) +6 set str_i@2 _latin1'20' +7 jump_if_not 10(17) (j@1 = 1) +8 set str_j@3 _latin1'1' +9 jump 17 +10 jump_if_not 13(17) (j@1 = 2) +11 set str_j@3 _latin1'2' +12 jump 17 +13 jump_if_not 16(17) (j@1 = 3) +14 set str_j@3 _latin1'3' +15 jump 17 +16 set str_j@3 _latin1'unknown' +17 stmt 0 "select "i was 20"" +18 jump 26 +19 jump_if_not 22(26) (i@0 = 30) +20 set str_i@2 _latin1'30' +21 jump 26 +22 jump_if_not 25(26) (i@0 = 40) +23 set str_i@2 _latin1'40' +24 jump 26 +25 set str_i@2 _latin1'unknown' +26 stmt 0 "SELECT str_i, str_j" +CALL proc_19194_nested_1(10, 1); +str_i str_j +10 NULL +CALL proc_19194_nested_1(25, 1); +str_i str_j +unknown NULL +CALL proc_19194_nested_1(20, 1); +i was 20 +i was 20 +str_i str_j +20 1 +CALL proc_19194_nested_1(20, 2); +i was 20 +i was 20 +str_i str_j +20 2 +CALL proc_19194_nested_1(20, 3); +i was 20 +i was 20 +str_i str_j +20 3 +CALL proc_19194_nested_1(20, 4); +i was 20 +i was 20 +str_i str_j +20 unknown +CALL proc_19194_nested_1(30, 1); +str_i str_j +30 NULL +CALL proc_19194_nested_1(40, 1); +str_i str_j +40 NULL +CALL proc_19194_nested_1(0, 0); +str_i str_j +unknown NULL +CALL proc_19194_nested_2(10, 1); +str_i str_j +10 NULL +CALL proc_19194_nested_2(25, 1); +str_i str_j +unknown NULL +CALL proc_19194_nested_2(20, 1); +i was 20 +i was 20 +str_i str_j +20 1 +CALL proc_19194_nested_2(20, 2); +i was 20 +i was 20 +str_i str_j +20 2 +CALL proc_19194_nested_2(20, 3); +i was 20 +i was 20 +str_i str_j +20 3 +CALL proc_19194_nested_2(20, 4); +i was 20 +i was 20 +str_i str_j +20 unknown +CALL proc_19194_nested_2(30, 1); +str_i str_j +30 NULL +CALL proc_19194_nested_2(40, 1); +str_i str_j +40 NULL +CALL proc_19194_nested_2(0, 0); +str_i str_j +unknown NULL +CALL proc_19194_nested_3(10, 1); +str_i str_j +10 NULL +CALL proc_19194_nested_3(25, 1); +str_i str_j +unknown NULL +CALL proc_19194_nested_3(20, 1); +i was 20 +i was 20 +str_i str_j +20 1 +CALL proc_19194_nested_3(20, 2); +i was 20 +i was 20 +str_i str_j +20 2 +CALL proc_19194_nested_3(20, 3); +i was 20 +i was 20 +str_i str_j +20 3 +CALL proc_19194_nested_3(20, 4); +i was 20 +i was 20 +str_i str_j +20 unknown +CALL proc_19194_nested_3(30, 1); +str_i str_j +30 NULL +CALL proc_19194_nested_3(40, 1); +str_i str_j +40 NULL +CALL proc_19194_nested_3(0, 0); +str_i str_j +unknown NULL +CALL proc_19194_nested_4(10, 1); +str_i str_j +10 NULL +CALL proc_19194_nested_4(25, 1); +str_i str_j +unknown NULL +CALL proc_19194_nested_4(20, 1); +i was 20 +i was 20 +str_i str_j +20 1 +CALL proc_19194_nested_4(20, 2); +i was 20 +i was 20 +str_i str_j +20 2 +CALL proc_19194_nested_4(20, 3); +i was 20 +i was 20 +str_i str_j +20 3 +CALL proc_19194_nested_4(20, 4); +i was 20 +i was 20 +str_i str_j +20 unknown +CALL proc_19194_nested_4(30, 1); +str_i str_j +30 NULL +CALL proc_19194_nested_4(40, 1); +str_i str_j +40 NULL +CALL proc_19194_nested_4(0, 0); +str_i str_j +unknown NULL +DROP PROCEDURE proc_19194_simple; +DROP PROCEDURE proc_19194_searched; +DROP PROCEDURE proc_19194_nested_1; +DROP PROCEDURE proc_19194_nested_2; +DROP PROCEDURE proc_19194_nested_3; +DROP PROCEDURE proc_19194_nested_4; DROP PROCEDURE IF EXISTS p1; CREATE PROCEDURE p1() CREATE INDEX idx ON t1 (c1); SHOW PROCEDURE CODE p1; diff --git a/mysql-test/r/sp_stress_case.result b/mysql-test/r/sp_stress_case.result new file mode 100644 index 00000000000..8ec68363c8d --- /dev/null +++ b/mysql-test/r/sp_stress_case.result @@ -0,0 +1,120 @@ +DROP PROCEDURE IF EXISTS proc_19194_codegen; +DROP PROCEDURE IF EXISTS bug_19194_simple; +DROP PROCEDURE IF EXISTS bug_19194_searched; +CREATE PROCEDURE proc_19194_codegen( +IN proc_name VARCHAR(50), +IN count INTEGER, +IN simple INTEGER, +OUT body MEDIUMTEXT) +BEGIN +DECLARE code MEDIUMTEXT; +DECLARE i INT DEFAULT 1; +SET code = concat("CREATE PROCEDURE ", proc_name, "(i INT)\n"); +SET code = concat(code, "BEGIN\n"); +SET code = concat(code, " DECLARE str CHAR(10);\n"); +IF (simple) +THEN +SET code = concat(code, " CASE i\n"); +ELSE +SET code = concat(code, " CASE\n"); +END IF; +WHILE (i <= count) +DO +IF (simple) +THEN +SET code = concat(code, " WHEN ", i, " THEN SET str=\"", i, "\";\n"); +ELSE +SET code = concat(code, " WHEN i=", i, " THEN SET str=\"", i, "\";\n"); +END IF; +SET i = i + 1; +END WHILE; +SET code = concat(code, " ELSE SET str=\"unknown\";\n"); +SET code = concat(code, " END CASE;\n"); +SET code = concat(code, " SELECT str;\n"); +SET code = concat(code, "END\n"); +SET body = code; +END| +set @body=""; +call proc_19194_codegen("test_simple", 10, 1, @body); +select @body; +@body +CREATE PROCEDURE test_simple(i INT) +BEGIN + DECLARE str CHAR(10); + CASE i + WHEN 1 THEN SET str="1"; + WHEN 2 THEN SET str="2"; + WHEN 3 THEN SET str="3"; + WHEN 4 THEN SET str="4"; + WHEN 5 THEN SET str="5"; + WHEN 6 THEN SET str="6"; + WHEN 7 THEN SET str="7"; + WHEN 8 THEN SET str="8"; + WHEN 9 THEN SET str="9"; + WHEN 10 THEN SET str="10"; + ELSE SET str="unknown"; + END CASE; + SELECT str; +END + +call proc_19194_codegen("test_searched", 10, 0, @body); +select @body; +@body +CREATE PROCEDURE test_searched(i INT) +BEGIN + DECLARE str CHAR(10); + CASE + WHEN i=1 THEN SET str="1"; + WHEN i=2 THEN SET str="2"; + WHEN i=3 THEN SET str="3"; + WHEN i=4 THEN SET str="4"; + WHEN i=5 THEN SET str="5"; + WHEN i=6 THEN SET str="6"; + WHEN i=7 THEN SET str="7"; + WHEN i=8 THEN SET str="8"; + WHEN i=9 THEN SET str="9"; + WHEN i=10 THEN SET str="10"; + ELSE SET str="unknown"; + END CASE; + SELECT str; +END + +CALL bug_19194_simple(1); +str +1 +CALL bug_19194_simple(2); +str +2 +CALL bug_19194_simple(1000); +str +1000 +CALL bug_19194_simple(4998); +str +4998 +CALL bug_19194_simple(4999); +str +4999 +CALL bug_19194_simple(9999); +str +unknown +CALL bug_19194_searched(1); +str +1 +CALL bug_19194_searched(2); +str +2 +CALL bug_19194_searched(1000); +str +1000 +CALL bug_19194_searched(4998); +str +4998 +CALL bug_19194_searched(4999); +str +4999 +CALL bug_19194_searched(9999); +str +unknown +DROP PROCEDURE proc_19194_codegen; +DROP PROCEDURE bug_19194_simple; +DROP PROCEDURE bug_19194_searched; diff --git a/mysql-test/t/csv.test b/mysql-test/t/csv.test index 60d38394fc0..48c174355f4 100644 --- a/mysql-test/t/csv.test +++ b/mysql-test/t/csv.test @@ -1605,3 +1605,17 @@ check table bug22080_2; check table bug22080_3; drop tables bug22080_1,bug22080_2,bug22080_3; + +# +# Bug #21328 mysqld issues warnings on ALTER CSV table to MyISAM +# + +CREATE TABLE `bug21328` ( + `col1` int(11) DEFAULT NULL, + `col2` int(11) DEFAULT NULL, + `col3` int(11) DEFAULT NULL +) ENGINE=CSV; + +insert into bug21328 values (1,NULL,NULL); +alter table bug21328 engine=myisam; +drop table bug21328; diff --git a/mysql-test/t/log.sh b/mysql-test/t/log.sh index 20b265087cc..29cf8d3e1a3 100755 --- a/mysql-test/t/log.sh +++ b/mysql-test/t/log.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh ########################################################################### diff --git a/mysql-test/t/read_only.test b/mysql-test/t/read_only.test index 8e14b310f4c..709238c3d76 100644 --- a/mysql-test/t/read_only.test +++ b/mysql-test/t/read_only.test @@ -102,6 +102,111 @@ drop table t1; insert into t1 values(1); # +# BUG#11733: COMMITs should not happen if read-only is set +# + +# LOCK TABLE ... WRITE / READ_ONLY +# - is an error in the same connection +# - is ok in a different connection + +connection default; +set global read_only=0; +lock table t1 write; + +connection con1; +lock table t2 write; + +connection default; +--error ER_LOCK_OR_ACTIVE_TRANSACTION +set global read_only=1; +unlock tables ; +# The following call blocks until con1 releases the write lock. +# Blocking is expected. +send set global read_only=1; + +connection con1; +--sleep 1 +select @@global.read_only; +unlock tables ; +--sleep 1 +select @@global.read_only; + +connection default; +reap; + +# LOCK TABLE ... READ / READ_ONLY +# - is an error in the same connection +# - is ok in a different connection + +connection default; +set global read_only=0; +lock table t1 read; + +connection con1; +lock table t2 read; + +connection default; +--error ER_LOCK_OR_ACTIVE_TRANSACTION +set global read_only=1; +unlock tables ; +# The following call blocks until con1 releases the read lock. +# Blocking is a limitation, and could be improved. +send set global read_only=1; + +connection con1; +--sleep 1 +select @@global.read_only; +unlock tables ; +--sleep 1 +select @@global.read_only; + +connection default; +reap; + +# pending transaction / READ_ONLY +# - is an error in the same connection +# - is ok in a different connection + +connection default; +set global read_only=0; +BEGIN; + +connection con1; +BEGIN; + +connection default; +--error ER_LOCK_OR_ACTIVE_TRANSACTION +set global read_only=1; +ROLLBACK; +set global read_only=1; + +connection con1; +select @@global.read_only; +ROLLBACK; + +# Verify that FLUSH TABLES WITH READ LOCK do not block READ_ONLY +# - in the same SUPER connection +# - in another SUPER connection + +connection default; +set global read_only=0; +flush tables with read lock; +set global read_only=1; +unlock tables; + +connect (root2,localhost,root,,test); + +connection default; +set global read_only=0; +flush tables with read lock; + +connection root2; +set global read_only=1; + +connection default; +select @@global.read_only; +unlock tables; + # BUG #22077 "DROP TEMPORARY TABLE fails with wrong error if read_only is set" # # check if DROP TEMPORARY on a non-existing temporary table returns the right @@ -113,8 +218,10 @@ drop temporary table ttt; # check if DROP TEMPORARY TABLE IF EXISTS produces a warning with read_only set drop temporary table if exists ttt; +# +# Cleanup +# connection default; +set global read_only=0; drop table t1,t2; drop user test@localhost; - -set global read_only=0; diff --git a/mysql-test/t/read_only_innodb.test b/mysql-test/t/read_only_innodb.test new file mode 100644 index 00000000000..76d9748aa60 --- /dev/null +++ b/mysql-test/t/read_only_innodb.test @@ -0,0 +1,43 @@ +# should work with embedded server after mysqltest is fixed +-- source include/not_embedded.inc +-- source include/have_innodb.inc + +# +# BUG#11733: COMMITs should not happen if read-only is set +# + +--disable_warnings +DROP TABLE IF EXISTS table_11733 ; +--enable_warnings + +# READ_ONLY does nothing to SUPER users +# so we use a non-SUPER one: + +grant CREATE, SELECT, DROP on *.* to test@localhost; + +connect (con1,localhost,test,,test); + +connection default; +set global read_only=0; + +# Any transactional engine will do +create table table_11733 (a int) engine=InnoDb; + +connection con1; +BEGIN; +insert into table_11733 values(11733); + +connection default; +set global read_only=1; + +connection con1; +select @@global.read_only; +select * from table_11733 ; +-- error ER_OPTION_PREVENTS_STATEMENT +COMMIT; + +connection default; +set global read_only=0; +drop table table_11733 ; +drop user test@localhost; + diff --git a/mysql-test/t/rpl_read_only-slave.opt b/mysql-test/t/rpl_read_only-slave.opt new file mode 100644 index 00000000000..627becdbfb5 --- /dev/null +++ b/mysql-test/t/rpl_read_only-slave.opt @@ -0,0 +1 @@ +--innodb diff --git a/mysql-test/t/rpl_read_only.test b/mysql-test/t/rpl_read_only.test new file mode 100644 index 00000000000..659c3d10044 --- /dev/null +++ b/mysql-test/t/rpl_read_only.test @@ -0,0 +1,105 @@ +# Test case for BUG #11733 +-- source include/master-slave.inc +-- source include/have_innodb.inc + +# Setting the master readonly : +# - the variable @@readonly is not replicated on the slave + +connect (master2,127.0.0.1,test,,test,$MASTER_MYPORT,); +connect (slave2,127.0.0.1,test,,test,$SLAVE_MYPORT,); + +connection master1; + +create table t1(a int) engine=InnoDB; +create table t2(a int) engine=MyISAM; +insert into t1 values(1001); +insert into t2 values(2001); + +connection master; +set global read_only=1; + +connection master1; +select @@read_only; +select * from t1; +select * from t2; + +sync_slave_with_master; +select @@read_only; +select * from t1; +select * from t2; + +# - replication of transactions +connection master; +set global read_only=0; + +connection master1; +BEGIN; +insert into t1 values(1002); +insert into t2 values(2002); + +connection master2; +BEGIN; +insert into t1 values(1003); +insert into t2 values(2003); + +connection master; +set global read_only=1; + +connection master1; +## works even with read_only=1, because master1 is root +COMMIT; + +connection master2; +--error ER_OPTION_PREVENTS_STATEMENT +COMMIT; + +connection master; +set global read_only=0; + +connection master1; +insert into t1 values(1004); +insert into t2 values(2004); + +select * from t1; +select * from t2; + +sync_slave_with_master; +select * from t1; +select * from t2; + +# Setting the slave readonly : replication will pass +# +connection slave1; +set global read_only=1; + +connection slave; +select @@read_only; +# Make sure the replicated table is also transactional +show create table t1; +# Make sure the replicated table is not transactional +show create table t2; + +connection master; +insert into t1 values(1005); +insert into t2 values(2005); +select * from t1; +select * from t2; + +sync_slave_with_master; +connection slave; +select * from t1; +select * from t2; + +# Non root user can not write on the slave +connection slave2; +--error ER_OPTION_PREVENTS_STATEMENT +insert into t1 values(1006); +--error ER_OPTION_PREVENTS_STATEMENT +insert into t2 values(2006); + +## Cleanup +connection master; +drop table t1; +drop table t2; +sync_slave_with_master; + diff --git a/mysql-test/t/sp-code.test b/mysql-test/t/sp-code.test index 72efa831059..97bc29fcad2 100644 --- a/mysql-test/t/sp-code.test +++ b/mysql-test/t/sp-code.test @@ -191,6 +191,241 @@ show procedure code sudoku_solve; drop procedure sudoku_solve; +# +# Bug#19194 (Right recursion in parser for CASE causes excessive stack +# usage, limitation) +# This bug also exposed a flaw in the generated code with nested case +# statements +# + +--disable_warnings +DROP PROCEDURE IF EXISTS proc_19194_simple; +DROP PROCEDURE IF EXISTS proc_19194_searched; +DROP PROCEDURE IF EXISTS proc_19194_nested_1; +DROP PROCEDURE IF EXISTS proc_19194_nested_2; +DROP PROCEDURE IF EXISTS proc_19194_nested_3; +DROP PROCEDURE IF EXISTS proc_19194_nested_4; +--enable_warnings + +delimiter |; + +CREATE PROCEDURE proc_19194_simple(i int) +BEGIN + DECLARE str CHAR(10); + + CASE i + WHEN 1 THEN SET str="1"; + WHEN 2 THEN SET str="2"; + WHEN 3 THEN SET str="3"; + ELSE SET str="unknown"; + END CASE; + + SELECT str; +END| + +CREATE PROCEDURE proc_19194_searched(i int) +BEGIN + DECLARE str CHAR(10); + + CASE + WHEN i=1 THEN SET str="1"; + WHEN i=2 THEN SET str="2"; + WHEN i=3 THEN SET str="3"; + ELSE SET str="unknown"; + END CASE; + + SELECT str; +END| + +# Outer SIMPLE case, inner SEARCHED case +CREATE PROCEDURE proc_19194_nested_1(i int, j int) +BEGIN + DECLARE str_i CHAR(10); + DECLARE str_j CHAR(10); + + CASE i + WHEN 10 THEN SET str_i="10"; + WHEN 20 THEN + BEGIN + set str_i="20"; + CASE + WHEN j=1 THEN SET str_j="1"; + WHEN j=2 THEN SET str_j="2"; + WHEN j=3 THEN SET str_j="3"; + ELSE SET str_j="unknown"; + END CASE; + select "i was 20"; + END; + WHEN 30 THEN SET str_i="30"; + WHEN 40 THEN SET str_i="40"; + ELSE SET str_i="unknown"; + END CASE; + + SELECT str_i, str_j; +END| + +# Outer SEARCHED case, inner SIMPLE case +CREATE PROCEDURE proc_19194_nested_2(i int, j int) +BEGIN + DECLARE str_i CHAR(10); + DECLARE str_j CHAR(10); + + CASE + WHEN i=10 THEN SET str_i="10"; + WHEN i=20 THEN + BEGIN + set str_i="20"; + CASE j + WHEN 1 THEN SET str_j="1"; + WHEN 2 THEN SET str_j="2"; + WHEN 3 THEN SET str_j="3"; + ELSE SET str_j="unknown"; + END CASE; + select "i was 20"; + END; + WHEN i=30 THEN SET str_i="30"; + WHEN i=40 THEN SET str_i="40"; + ELSE SET str_i="unknown"; + END CASE; + + SELECT str_i, str_j; +END| + +# Outer SIMPLE case, inner SIMPLE case +CREATE PROCEDURE proc_19194_nested_3(i int, j int) +BEGIN + DECLARE str_i CHAR(10); + DECLARE str_j CHAR(10); + + CASE i + WHEN 10 THEN SET str_i="10"; + WHEN 20 THEN + BEGIN + set str_i="20"; + CASE j + WHEN 1 THEN SET str_j="1"; + WHEN 2 THEN SET str_j="2"; + WHEN 3 THEN SET str_j="3"; + ELSE SET str_j="unknown"; + END CASE; + select "i was 20"; + END; + WHEN 30 THEN SET str_i="30"; + WHEN 40 THEN SET str_i="40"; + ELSE SET str_i="unknown"; + END CASE; + + SELECT str_i, str_j; +END| + +# Outer SEARCHED case, inner SEARCHED case +CREATE PROCEDURE proc_19194_nested_4(i int, j int) +BEGIN + DECLARE str_i CHAR(10); + DECLARE str_j CHAR(10); + + CASE + WHEN i=10 THEN SET str_i="10"; + WHEN i=20 THEN + BEGIN + set str_i="20"; + CASE + WHEN j=1 THEN SET str_j="1"; + WHEN j=2 THEN SET str_j="2"; + WHEN j=3 THEN SET str_j="3"; + ELSE SET str_j="unknown"; + END CASE; + select "i was 20"; + END; + WHEN i=30 THEN SET str_i="30"; + WHEN i=40 THEN SET str_i="40"; + ELSE SET str_i="unknown"; + END CASE; + + SELECT str_i, str_j; +END| + +delimiter ;| + +SHOW PROCEDURE CODE proc_19194_simple; +SHOW PROCEDURE CODE proc_19194_searched; +SHOW PROCEDURE CODE proc_19194_nested_1; +SHOW PROCEDURE CODE proc_19194_nested_2; +SHOW PROCEDURE CODE proc_19194_nested_3; +SHOW PROCEDURE CODE proc_19194_nested_4; + +CALL proc_19194_nested_1(10, 1); + +# +# Before 19194, the generated code was: +# 20 jump_if_not 23(27) 30 +# 21 set str_i@2 _latin1'30' +# As opposed to the expected: +# 20 jump_if_not 23(27) (case_expr@0 = 30) +# 21 set str_i@2 _latin1'30' +# +# and as a result, this call returned "30", +# because the expression 30 is always true, +# masking the case 40, case 0 and the else. +# +CALL proc_19194_nested_1(25, 1); + +CALL proc_19194_nested_1(20, 1); +CALL proc_19194_nested_1(20, 2); +CALL proc_19194_nested_1(20, 3); +CALL proc_19194_nested_1(20, 4); +CALL proc_19194_nested_1(30, 1); +CALL proc_19194_nested_1(40, 1); +CALL proc_19194_nested_1(0, 0); + +CALL proc_19194_nested_2(10, 1); + +# +# Before 19194, the generated code was: +# 20 jump_if_not 23(27) (case_expr@0 = (i@0 = 30)) +# 21 set str_i@2 _latin1'30' +# As opposed to the expected: +# 20 jump_if_not 23(27) (i@0 = 30) +# 21 set str_i@2 _latin1'30' +# and as a result, this call crashed the server, because there is no +# such variable as "case_expr@0". +# +CALL proc_19194_nested_2(25, 1); + +CALL proc_19194_nested_2(20, 1); +CALL proc_19194_nested_2(20, 2); +CALL proc_19194_nested_2(20, 3); +CALL proc_19194_nested_2(20, 4); +CALL proc_19194_nested_2(30, 1); +CALL proc_19194_nested_2(40, 1); +CALL proc_19194_nested_2(0, 0); + +CALL proc_19194_nested_3(10, 1); +CALL proc_19194_nested_3(25, 1); +CALL proc_19194_nested_3(20, 1); +CALL proc_19194_nested_3(20, 2); +CALL proc_19194_nested_3(20, 3); +CALL proc_19194_nested_3(20, 4); +CALL proc_19194_nested_3(30, 1); +CALL proc_19194_nested_3(40, 1); +CALL proc_19194_nested_3(0, 0); + +CALL proc_19194_nested_4(10, 1); +CALL proc_19194_nested_4(25, 1); +CALL proc_19194_nested_4(20, 1); +CALL proc_19194_nested_4(20, 2); +CALL proc_19194_nested_4(20, 3); +CALL proc_19194_nested_4(20, 4); +CALL proc_19194_nested_4(30, 1); +CALL proc_19194_nested_4(40, 1); +CALL proc_19194_nested_4(0, 0); + +DROP PROCEDURE proc_19194_simple; +DROP PROCEDURE proc_19194_searched; +DROP PROCEDURE proc_19194_nested_1; +DROP PROCEDURE proc_19194_nested_2; +DROP PROCEDURE proc_19194_nested_3; +DROP PROCEDURE proc_19194_nested_4; # # Bug#19207: Final parenthesis omitted for CREATE INDEX in Stored diff --git a/mysql-test/t/sp_stress_case.test b/mysql-test/t/sp_stress_case.test new file mode 100644 index 00000000000..1b5bd8991a9 --- /dev/null +++ b/mysql-test/t/sp_stress_case.test @@ -0,0 +1,89 @@ +# +# Bug#19194 (Right recursion in parser for CASE causes excessive stack +# usage, limitation) +# + +--disable_warnings +DROP PROCEDURE IF EXISTS proc_19194_codegen; +DROP PROCEDURE IF EXISTS bug_19194_simple; +DROP PROCEDURE IF EXISTS bug_19194_searched; +--enable_warnings + +delimiter |; + +CREATE PROCEDURE proc_19194_codegen( + IN proc_name VARCHAR(50), + IN count INTEGER, + IN simple INTEGER, + OUT body MEDIUMTEXT) +BEGIN + DECLARE code MEDIUMTEXT; + DECLARE i INT DEFAULT 1; + + SET code = concat("CREATE PROCEDURE ", proc_name, "(i INT)\n"); + SET code = concat(code, "BEGIN\n"); + SET code = concat(code, " DECLARE str CHAR(10);\n"); + + IF (simple) + THEN + SET code = concat(code, " CASE i\n"); + ELSE + SET code = concat(code, " CASE\n"); + END IF; + + WHILE (i <= count) + DO + IF (simple) + THEN + SET code = concat(code, " WHEN ", i, " THEN SET str=\"", i, "\";\n"); + ELSE + SET code = concat(code, " WHEN i=", i, " THEN SET str=\"", i, "\";\n"); + END IF; + + SET i = i + 1; + END WHILE; + + SET code = concat(code, " ELSE SET str=\"unknown\";\n"); + SET code = concat(code, " END CASE;\n"); + SET code = concat(code, " SELECT str;\n"); + + SET code = concat(code, "END\n"); + + SET body = code; +END| + +delimiter ;| + +set @body=""; +call proc_19194_codegen("test_simple", 10, 1, @body); +select @body; +call proc_19194_codegen("test_searched", 10, 0, @body); +select @body; + +--disable_query_log +call proc_19194_codegen("bug_19194_simple", 5000, 1, @body); +let $proc_body = `select @body`; +eval $proc_body; +call proc_19194_codegen("bug_19194_searched", 5000, 1, @body); +let $proc_body = `select @body`; +eval $proc_body; +--enable_query_log + +CALL bug_19194_simple(1); +CALL bug_19194_simple(2); +CALL bug_19194_simple(1000); +CALL bug_19194_simple(4998); +CALL bug_19194_simple(4999); +CALL bug_19194_simple(9999); + +CALL bug_19194_searched(1); +CALL bug_19194_searched(2); +CALL bug_19194_searched(1000); +CALL bug_19194_searched(4998); +CALL bug_19194_searched(4999); +CALL bug_19194_searched(9999); + +DROP PROCEDURE proc_19194_codegen; +DROP PROCEDURE bug_19194_simple; +DROP PROCEDURE bug_19194_searched; + diff --git a/server-tools/instance-manager/commands.cc b/server-tools/instance-manager/commands.cc index f8bd63033d7..1be64ec4969 100644 --- a/server-tools/instance-manager/commands.cc +++ b/server-tools/instance-manager/commands.cc @@ -28,6 +28,7 @@ #include "guardian.h" #include "instance_map.h" #include "log.h" +#include "manager.h" #include "messages.h" #include "mysqld_error.h" #include "mysql_manager_error.h" @@ -35,8 +36,11 @@ #include "priv.h" #include "protocol.h" +/************************************************************************** + {{{ Static functions. +**************************************************************************/ -/* +/** modify_defaults_to_im_error -- a map of error codes of mysys::modify_defaults_file() into Instance Manager error codes. */ @@ -45,38 +49,25 @@ static const int modify_defaults_to_im_error[]= { 0, ER_OUT_OF_RESOURCES, ER_ACCESS_OPTION_FILE }; -/* - Add a string to a buffer. +/** + Parse version number from the version string. SYNOPSIS - put_to_buff() - buff buffer to add the string - str string to add - position offset in the buff to add a string + parse_version_number() + version_str + version + version_size DESCRIPTION + TODO - Function to add a string to the buffer. It is different from - store_to_protocol_packet, which is used in the protocol.cc. - The last one also stores the length of the string in a special way. - This is required for MySQL client/server protocol support only. + TODO: Move this function to Instance_options and parse version number + only once. - RETURN - 0 - ok - 1 - error occured + NOTE: This function is used only in SHOW INSTANCE STATUS statement at the + moment. */ -static inline int put_to_buff(Buffer *buff, const char *str, uint *position) -{ - uint len= strlen(str); - if (buff->append(*position, str, len)) - return 1; - - *position+= len; - return 0; -} - - static int parse_version_number(const char *version_str, char *version, uint version_size) { @@ -101,6 +92,9 @@ static int parse_version_number(const char *version_str, char *version, return 0; } +/************************************************************************** + }}} +**************************************************************************/ /************************************************************************** Implementation of Instance_name. @@ -121,7 +115,7 @@ Instance_name::Instance_name(const LEX_STRING *name) Implementation of Show_instances. **************************************************************************/ -/* +/** Implementation of SHOW INSTANCES statement. Possible error codes: @@ -171,7 +165,6 @@ int Show_instances::write_data(st_net *net) Instance *instance; Instance_map::Iterator iterator(instance_map); - instance_map->guardian->lock(); instance_map->lock(); while ((instance= iterator.next())) @@ -179,20 +172,25 @@ int Show_instances::write_data(st_net *net) Buffer send_buf; /* buffer for packets */ uint pos= 0; + instance->lock(); + const char *instance_name= instance->options.instance_name.str; - const char *state_name= instance_map->get_instance_state_name(instance); + const char *state_name= instance->get_state_name(); if (store_to_protocol_packet(&send_buf, instance_name, &pos) || store_to_protocol_packet(&send_buf, state_name, &pos) || my_net_write(net, send_buf.buffer, pos)) { err_status= TRUE; - break; } + + instance->unlock(); + + if (err_status) + break; } instance_map->unlock(); - instance_map->guardian->unlock(); return err_status ? ER_OUT_OF_RESOURCES : 0; } @@ -202,7 +200,7 @@ int Show_instances::write_data(st_net *net) Implementation of Flush_instances. **************************************************************************/ -/* +/** Implementation of FLUSH INSTANCES statement. Possible error codes: @@ -212,36 +210,19 @@ int Show_instances::write_data(st_net *net) int Flush_instances::execute(st_net *net, ulong connection_id) { - instance_map->guardian->lock(); - instance_map->lock(); - - if (instance_map->is_there_active_instance()) - { - instance_map->unlock(); - instance_map->guardian->unlock(); - return ER_THERE_IS_ACTIVE_INSTACE; - } - - if (instance_map->flush_instances()) - { - instance_map->unlock(); - instance_map->guardian->unlock(); + if (Manager::flush_instances()) return ER_OUT_OF_RESOURCES; - } - - instance_map->unlock(); - instance_map->guardian->unlock(); return net_send_ok(net, connection_id, NULL) ? ER_OUT_OF_RESOURCES : 0; } /************************************************************************** - Implementation of Abstract_instance_cmd. + Implementation of Instance_cmd. **************************************************************************/ -Abstract_instance_cmd::Abstract_instance_cmd(const LEX_STRING *instance_name_arg) - :instance_name(instance_name_arg) +Instance_cmd::Instance_cmd(const LEX_STRING *instance_name_arg): + instance_name(instance_name_arg) { /* MT-NOTE: we can not make a search for Instance object here, @@ -250,26 +231,39 @@ Abstract_instance_cmd::Abstract_instance_cmd(const LEX_STRING *instance_name_arg } +/************************************************************************** + Implementation of Abstract_instance_cmd. +**************************************************************************/ + +Abstract_instance_cmd::Abstract_instance_cmd( + const LEX_STRING *instance_name_arg) + :Instance_cmd(instance_name_arg) +{ +} + + int Abstract_instance_cmd::execute(st_net *net, ulong connection_id) { int err_code; + Instance *instance; instance_map->lock(); - { - Instance *instance= instance_map->find(get_instance_name()); - - if (!instance) - { - instance_map->unlock(); - return ER_BAD_INSTANCE_NAME; - } + instance= instance_map->find(get_instance_name()); - err_code= execute_impl(net, instance); + if (!instance) + { + instance_map->unlock(); + return ER_BAD_INSTANCE_NAME; } + instance->lock(); instance_map->unlock(); + err_code= execute_impl(net, instance); + + instance->unlock(); + if (!err_code) err_code= send_ok_response(net, connection_id); @@ -287,7 +281,7 @@ Show_instance_status::Show_instance_status(const LEX_STRING *instance_name_arg) } -/* +/** Implementation of SHOW INSTANCE STATUS statement. Possible error codes: @@ -362,19 +356,14 @@ int Show_instance_status::write_data(st_net *net, Instance *instance) char version_num_buf[MAX_VERSION_LENGTH]; uint pos= 0; - const char *state_name; + const char *state_name= instance->get_state_name(); const char *version_tag= "unknown"; const char *version_num= "unknown"; - const char *mysqld_compatible_status; - - instance_map->guardian->lock(); - state_name= instance_map->get_instance_state_name(instance); - mysqld_compatible_status= instance->is_mysqld_compatible() ? "yes" : "no"; - instance_map->guardian->unlock(); + const char *mysqld_compatible_status= + instance->is_mysqld_compatible() ? "yes" : "no"; if (instance->options.mysqld_version) { - if (parse_version_number(instance->options.mysqld_version, version_num_buf, sizeof(version_num_buf))) return ER_OUT_OF_RESOURCES; @@ -408,7 +397,7 @@ Show_instance_options::Show_instance_options( } -/* +/** Implementation of SHOW INSTANCE OPTIONS statement. Possible error codes: @@ -504,23 +493,33 @@ Start_instance::Start_instance(const LEX_STRING *instance_name_arg) } -/* +/** Implementation of START INSTANCE statement. Possible error codes: ER_BAD_INSTANCE_NAME The instance with the given name does not exist - ER_OUT_OF_RESOURCES Not enough resources to complete the operation + ER_INSTANCE_MISCONFIGURED The instance configuration is invalid + ER_INSTANCE_ALREADY_STARTED The instance is already started + ER_CANNOT_START_INSTANCE The instance could not have been started + + TODO: as soon as this method operates only with Instance, we probably + should introduce a new method (execute_stop_instance()) in Instance and + just call it from here. */ int Start_instance::execute_impl(st_net * /* net */, Instance *instance) { - int err_code; + if (!instance->is_configured()) + return ER_INSTANCE_MISCONFIGURED; - if ((err_code= instance->start())) - return err_code; + if (instance->is_active()) + return ER_INSTANCE_ALREADY_STARTED; + + if (instance->start_mysqld()) + return ER_CANNOT_START_INSTANCE; - if (!(instance->options.nonguarded)) - instance_map->guardian->guard(instance); + instance->reset_stat(); + instance->set_state(Instance::NOT_STARTED); return 0; } @@ -545,25 +544,26 @@ Stop_instance::Stop_instance(const LEX_STRING *instance_name_arg) } -/* +/** Implementation of STOP INSTANCE statement. Possible error codes: ER_BAD_INSTANCE_NAME The instance with the given name does not exist ER_OUT_OF_RESOURCES Not enough resources to complete the operation + + TODO: as soon as this method operates only with Instance, we probably + should introduce a new method (execute_stop_instance()) in Instance and + just call it from here. */ int Stop_instance::execute_impl(st_net * /* net */, Instance *instance) { - int err_code; + if (!instance->is_active()) + return ER_INSTANCE_IS_NOT_STARTED; - if (!(instance->options.nonguarded)) - instance_map->guardian->stop_guard(instance); + instance->set_state(Instance::STOPPED); - if ((err_code= instance->stop())) - return err_code; - - return 0; + return instance->stop_mysqld() ? ER_STOP_INSTANCE : 0; } @@ -581,12 +581,12 @@ int Stop_instance::send_ok_response(st_net *net, ulong connection_id) **************************************************************************/ Create_instance::Create_instance(const LEX_STRING *instance_name_arg) - :instance_name(instance_name_arg) + :Instance_cmd(instance_name_arg) { } -/* +/** This operation initializes Create_instance object. SYNOPSIS @@ -603,7 +603,7 @@ bool Create_instance::init(const char **text) } -/* +/** This operation parses CREATE INSTANCE options. SYNOPSIS @@ -723,7 +723,7 @@ bool Create_instance::parse_args(const char **text) } -/* +/** Implementation of CREATE INSTANCE statement. Possible error codes: @@ -735,6 +735,7 @@ bool Create_instance::parse_args(const char **text) int Create_instance::execute(st_net *net, ulong connection_id) { int err_code; + Instance *instance; /* Check that the name is valid and there is no instance with such name. */ @@ -760,17 +761,26 @@ int Create_instance::execute(st_net *net, ulong connection_id) return err_code; } + instance= instance_map->find(get_instance_name()); + DBUG_ASSERT(instance); + if ((err_code= create_instance_in_file(get_instance_name(), &options))) { - Instance *instance= instance_map->find(get_instance_name()); - - if (instance) - instance_map->remove_instance(instance); /* instance is deleted here. */ + instance_map->remove_instance(instance); /* instance is deleted here. */ instance_map->unlock(); return err_code; } + /* + CREATE INSTANCE must not lead to start instance, even if it guarded. + + TODO: The problem however is that if Instance Manager restarts after + creating instance, the instance will be restarted (see also BUG#19718). + */ + + instance->set_state(Instance::STOPPED); + /* That's all. */ instance_map->unlock(); @@ -789,12 +799,12 @@ int Create_instance::execute(st_net *net, ulong connection_id) **************************************************************************/ Drop_instance::Drop_instance(const LEX_STRING *instance_name_arg) - :Abstract_instance_cmd(instance_name_arg) + :Instance_cmd(instance_name_arg) { } -/* +/** Implementation of DROP INSTANCE statement. Possible error codes: @@ -803,14 +813,38 @@ Drop_instance::Drop_instance(const LEX_STRING *instance_name_arg) ER_OUT_OF_RESOURCES Not enough resources to complete the operation */ -int Drop_instance::execute_impl(st_net * /* net */, Instance *instance) +int Drop_instance::execute(st_net *net, ulong connection_id) { int err_code; + Instance *instance; + + /* Lock Guardian, then Instance_map. */ + + instance_map->lock(); + + /* Find an instance. */ + + instance= instance_map->find(get_instance_name()); + + if (!instance) + { + instance_map->unlock(); + return ER_BAD_INSTANCE_NAME; + } + + instance->lock(); /* Check that the instance is offline. */ - if (instance_map->guardian->is_active(instance)) + if (instance->is_active()) + { + instance->unlock(); + instance_map->unlock(); + return ER_DROP_ACTIVE_INSTANCE; + } + + /* Try to remove instance from the file. */ err_code= modify_defaults_file(Options::Main::config_file, NULL, NULL, get_instance_name()->str, MY_REMOVE_SECTION); @@ -823,27 +857,30 @@ int Drop_instance::execute_impl(st_net * /* net */, Instance *instance) (const char *) get_instance_name()->str, (const char *) Options::Main::config_file, (int) err_code); - } - if (err_code) + instance->unlock(); + instance_map->unlock(); + return modify_defaults_to_im_error[err_code]; + } - /* Remove instance from the instance map hash and Guardian's list. */ + /* Unlock the instance before destroy. */ - if (!instance->options.nonguarded) - instance_map->guardian->stop_guard(instance); + instance->unlock(); - if ((err_code= instance->stop())) - return err_code; + /* + Remove instance from the instance map + (the instance will be also destroyed here). + */ instance_map->remove_instance(instance); - return 0; -} + /* Unlock the instance map. */ + instance_map->unlock(); + + /* That's all: send ok. */ -int Drop_instance::send_ok_response(st_net *net, ulong connection_id) -{ if (net_send_ok(net, connection_id, "Instance dropped")) return ER_OUT_OF_RESOURCES; @@ -866,7 +903,7 @@ Show_instance_log::Show_instance_log(const LEX_STRING *instance_name_arg, } -/* +/** Implementation of SHOW INSTANCE LOG statement. Possible error codes: @@ -1011,7 +1048,7 @@ Show_instance_log_files::Show_instance_log_files } -/* +/** Implementation of SHOW INSTANCE LOG FILES statement. Possible error codes: @@ -1132,7 +1169,7 @@ int Show_instance_log_files::write_data(st_net *net, Instance *instance) Implementation of Abstract_option_cmd. **************************************************************************/ -/* +/** Instance_options_list -- a data class representing a list of options for some instance. */ @@ -1250,7 +1287,7 @@ bool Abstract_option_cmd::init(const char **text) } -/* +/** Correct the option file. The "skip" option is used to remove the found option. @@ -1289,8 +1326,8 @@ int Abstract_option_cmd::correct_file(Instance *instance, Named_value *option, } -/* - Implementation of SET statement. +/** + Lock Instance Map and call execute_impl(). Possible error codes: ER_BAD_INSTANCE_NAME The instance with the given name does not exist @@ -1340,6 +1377,11 @@ Abstract_option_cmd::get_instance_options_list(const LEX_STRING *instance_name) } +/** + Skeleton implementation of option-management command. + + MT-NOTE: Instance Map is locked before calling this operation. +*/ int Abstract_option_cmd::execute_impl(st_net *net, ulong connection_id) { int err_code= 0; @@ -1351,12 +1393,18 @@ int Abstract_option_cmd::execute_impl(st_net *net, ulong connection_id) Instance_options_list *lst= (Instance_options_list *) hash_element(&instance_options_map, i); + bool instance_is_active; + lst->instance= instance_map->find(lst->get_instance_name()); if (!lst->instance) return ER_BAD_INSTANCE_NAME; - if (instance_map->guardian->is_active(lst->instance)) + lst->instance->lock(); + instance_is_active= lst->instance->is_active(); + lst->instance->unlock(); + + if (instance_is_active) return ER_INSTANCE_IS_ACTIVE; } @@ -1367,6 +1415,8 @@ int Abstract_option_cmd::execute_impl(st_net *net, ulong connection_id) Instance_options_list *lst= (Instance_options_list *) hash_element(&instance_options_map, i); + lst->instance->lock(); + for (int j= 0; j < lst->options.get_size(); ++j) { Named_value option= lst->options.get_element(j); @@ -1376,6 +1426,8 @@ int Abstract_option_cmd::execute_impl(st_net *net, ulong connection_id) break; } + lst->instance->unlock(); + if (err_code) break; } @@ -1391,7 +1443,7 @@ int Abstract_option_cmd::execute_impl(st_net *net, ulong connection_id) Implementation of Set_option. **************************************************************************/ -/* +/** This operation parses SET options. SYNOPSIS @@ -1565,7 +1617,7 @@ int Set_option::process_option(Instance *instance, Named_value *option) Implementation of Unset_option. **************************************************************************/ -/* +/** This operation parses UNSET options. SYNOPSIS @@ -1661,7 +1713,7 @@ bool Unset_option::parse_args(const char **text) } -/* +/** Implementation of UNSET statement. Possible error codes: diff --git a/server-tools/instance-manager/commands.h b/server-tools/instance-manager/commands.h index faba1e0a18b..5c2b2f9bbb7 100644 --- a/server-tools/instance-manager/commands.h +++ b/server-tools/instance-manager/commands.h @@ -29,7 +29,7 @@ #endif -/* +/** Print all instances of this instance manager. Grammar: SHOW INSTANCES */ @@ -49,7 +49,7 @@ private: }; -/* +/** Reread configuration file and refresh internal cache. Grammar: FLUSH INSTANCES */ @@ -65,11 +65,50 @@ public: }; -/* +/** + Base class for Instance-specific commands + (commands that operate on one instance). + + Instance_cmd extends Command class by: + - an attribute for storing instance name; + - code to initialize instance name in constructor; + - an accessor to get instance name. +*/ + +class Instance_cmd : public Command +{ +public: + Instance_cmd(const LEX_STRING *instance_name_arg); + +protected: + inline const LEX_STRING *get_instance_name() const + { + return instance_name.get_str(); + } + +private: + Instance_name instance_name; +}; + + +/** Abstract class for Instance-specific commands. + + Abstract_instance_cmd extends Instance_cmd by providing a common + framework for writing command-implementations. Basically, the class + implements Command::execute() pure virtual function in the following + way: + - Lock Instance_map; + - Get an instance by name. Return an error, if there is no such + instance; + - Lock the instance; + - Unlock Instance_map; + - Call execute_impl(), which should be implemented in derived class; + - Unlock the instance; + - Send response to the client and return error status. */ -class Abstract_instance_cmd: public Command +class Abstract_instance_cmd: public Instance_cmd { public: Abstract_instance_cmd(const LEX_STRING *instance_name_arg); @@ -78,29 +117,24 @@ public: virtual int execute(st_net *net, ulong connection_id); protected: - /* MT-NOTE: this operation is called under acquired Instance_map's lock. */ + /** + This operation is intended to contain command-specific implementation. + + MT-NOTE: this operation is called under acquired Instance's lock. + */ virtual int execute_impl(st_net *net, Instance *instance) = 0; - /* + /** This operation is invoked on successful return of execute_impl() and is intended to send closing data. - MT-NOTE: this operation is called under released Instance_map's lock. + MT-NOTE: this operation is called under released Instance's lock. */ virtual int send_ok_response(st_net *net, ulong connection_id) = 0; - -protected: - inline const LEX_STRING *get_instance_name() const - { - return instance_name.get_str(); - } - -private: - Instance_name instance_name; }; -/* +/** Print status of an instance. Grammar: SHOW INSTANCE STATUS <instance_name> */ @@ -120,7 +154,7 @@ private: }; -/* +/** Print options of chosen instance. Grammar: SHOW INSTANCE OPTIONS <instance_name> */ @@ -140,7 +174,7 @@ private: }; -/* +/** Start an instance. Grammar: START INSTANCE <instance_name> */ @@ -156,7 +190,7 @@ protected: }; -/* +/** Stop an instance. Grammar: STOP INSTANCE <instance_name> */ @@ -172,12 +206,12 @@ protected: }; -/* +/** Create an instance. Grammar: CREATE INSTANCE <instance_name> [<options>] */ -class Create_instance: public Command +class Create_instance: public Instance_cmd { public: Create_instance(const LEX_STRING *instance_name_arg); @@ -188,22 +222,15 @@ public: protected: virtual int execute(st_net *net, ulong connection_id); - inline const LEX_STRING *get_instance_name() const - { - return instance_name.get_str(); - } - private: bool parse_args(const char **text); private: - Instance_name instance_name; - Named_value_arr options; }; -/* +/** Drop an instance. Grammar: DROP INSTANCE <instance_name> @@ -212,18 +239,17 @@ private: is removed from the instance map. */ -class Drop_instance: public Abstract_instance_cmd +class Drop_instance: public Instance_cmd { public: Drop_instance(const LEX_STRING *instance_name_arg); protected: - virtual int execute_impl(st_net *net, Instance *instance); - virtual int send_ok_response(st_net *net, ulong connection_id); + virtual int execute(st_net *net, ulong connection_id); }; -/* +/** Print requested part of the log. Grammar: SHOW <instance_name> LOG {ERROR | SLOW | GENERAL} size[, offset_from_end] @@ -251,7 +277,7 @@ private: }; -/* +/** Shows the list of the log files, used by an instance. Grammar: SHOW <instance_name> LOG FILES */ @@ -271,7 +297,7 @@ private: }; -/* +/** Abstract class for option-management commands. */ @@ -311,7 +337,7 @@ private: }; -/* +/** Set an option for the instance. Grammar: SET instance_name.option[=option_value][, ...] */ @@ -328,7 +354,7 @@ protected: }; -/* +/** Remove option of the instance. Grammar: UNSET instance_name.option[, ...] */ @@ -345,7 +371,7 @@ protected: }; -/* +/** Syntax error command. This command is issued if parser reported a syntax error. We need it to diff --git a/server-tools/instance-manager/guardian.cc b/server-tools/instance-manager/guardian.cc index 289e80d8b74..bad43b1f92e 100644 --- a/server-tools/instance-manager/guardian.cc +++ b/server-tools/instance-manager/guardian.cc @@ -27,102 +27,126 @@ #include "instance_map.h" #include "log.h" #include "mysql_manager_error.h" +#include "options.h" -const char * -Guardian::get_instance_state_name(enum_instance_state state) -{ - switch (state) { - case NOT_STARTED: - return "offline"; - - case STARTING: - return "starting"; - - case STARTED: - return "online"; - case JUST_CRASHED: - return "failed"; +/************************************************************************* + {{{ Constructor & destructor. +*************************************************************************/ - case CRASHED: - return "crashed"; - - case CRASHED_AND_ABANDONED: - return "abandoned"; - - case STOPPING: - return "stopping"; - } +/** + Guardian constructor. - return NULL; /* just to ignore compiler warning. */ -} + SYNOPSIS + Guardian() + thread_registry_arg + instance_map_arg -/* {{{ Constructor & destructor. */ + DESCRIPTION + Nominal contructor intended for assigning references and initialize + trivial objects. Real initialization is made by init() method. +*/ Guardian::Guardian(Thread_registry *thread_registry_arg, - Instance_map *instance_map_arg, - uint monitoring_interval_arg) - :stopped(FALSE), - monitoring_interval(monitoring_interval_arg), + Instance_map *instance_map_arg) + :shutdown_requested(FALSE), + stopped(FALSE), thread_registry(thread_registry_arg), - instance_map(instance_map_arg), - guarded_instances(0), - shutdown_requested(FALSE) + instance_map(instance_map_arg) { pthread_mutex_init(&LOCK_guardian, 0); pthread_cond_init(&COND_guardian, 0); - init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0); } Guardian::~Guardian() { - /* delay guardian destruction to the moment when no one needs it */ - pthread_mutex_lock(&LOCK_guardian); - free_root(&alloc, MYF(0)); - pthread_mutex_unlock(&LOCK_guardian); + /* + NOTE: it's necessary to synchronize here, because Guiardian thread can be + still alive an hold the mutex (because it is detached and we have no + control over it). + */ + + lock(); + unlock(); + pthread_mutex_destroy(&LOCK_guardian); pthread_cond_destroy(&COND_guardian); } -/* }}} */ +/************************************************************************* + }}} +*************************************************************************/ +/** + Send request to stop Guardian. + + SYNOPSIS + request_shutdown() +*/ + void Guardian::request_shutdown() { - pthread_mutex_lock(&LOCK_guardian); - /* STOP Instances or just clean up Guardian repository */ stop_instances(); + + lock(); shutdown_requested= TRUE; - pthread_mutex_unlock(&LOCK_guardian); + unlock(); + + ping(); } -void Guardian::process_instance(Instance *instance, - GUARD_NODE *current_node, - LIST **guarded_instances, - LIST *node) +/** + Process an instance. + + SYNOPSIS + process_instance() + instance a pointer to the instance for processing + + MT-NOTE: + - the given instance must be locked before calling this operation; + - Guardian must be locked before calling this operation. +*/ + +void Guardian::process_instance(Instance *instance) { - uint waitchild= (uint) Instance::DEFAULT_SHUTDOWN_DELAY; - /* The amount of times, Guardian attempts to restart an instance */ int restart_retry= 100; time_t current_time= time(NULL); - if (current_node->state == STOPPING) + if (instance->get_state() == Instance::STOPPING) { - waitchild= instance->options.get_shutdown_delay(); + /* This brach is executed during shutdown. */ - /* this returns TRUE if and only if an instance was stopped for sure */ + /* This returns TRUE if and only if an instance was stopped for sure. */ if (instance->is_crashed()) - *guarded_instances= list_delete(*guarded_instances, node); - else if ( (uint) (current_time - current_node->last_checked) > waitchild) { + log_info("Guardian: '%s' stopped.", + (const char *) instance->get_name()->str); + + instance->set_state(Instance::STOPPED); + } + else if ((uint) (current_time - instance->last_checked) >= + instance->options.get_shutdown_delay()) + { + log_info("Guardian: '%s' hasn't stopped within %d secs.", + (const char *) instance->get_name()->str, + (int) instance->options.get_shutdown_delay()); + instance->kill_mysqld(SIGKILL); - /* - Later we do node= node->next. This is ok, as we are only removing - the node from the list. The pointer to the next one is still valid. - */ - *guarded_instances= list_delete(*guarded_instances, node); + + log_info("Guardian: pretend that '%s' is killed.", + (const char *) instance->get_name()->str); + + instance->set_state(Instance::STOPPED); + } + else + { + log_info("Guardian: waiting for '%s' to stop (%d secs left).", + (const char *) instance->get_name()->str, + (int) (instance->options.get_shutdown_delay() - + current_time + instance->last_checked)); } return; @@ -133,83 +157,90 @@ void Guardian::process_instance(Instance *instance, /* The instance can be contacted on it's port */ /* If STARTING also check that pidfile has been created */ - if (current_node->state == STARTING && - current_node->instance->options.load_pid() == 0) + if (instance->get_state() == Instance::STARTING && + instance->options.load_pid() == 0) { /* Pid file not created yet, don't go to STARTED state yet */ } - else if (current_node->state != STARTED) + else if (instance->get_state() != Instance::STARTED) { /* clear status fields */ log_info("Guardian: '%s' is running, set state to STARTED.", (const char *) instance->options.instance_name.str); - current_node->restart_counter= 0; - current_node->crash_moment= 0; - current_node->state= STARTED; + instance->reset_stat(); + instance->set_state(Instance::STARTED); } } else { - switch (current_node->state) { - case NOT_STARTED: + switch (instance->get_state()) { + case Instance::NOT_STARTED: log_info("Guardian: starting '%s'...", (const char *) instance->options.instance_name.str); - /* NOTE, set state to STARTING _before_ start() is called */ - current_node->state= STARTING; - instance->start(); - current_node->last_checked= current_time; - break; - case STARTED: /* fallthrough */ - case STARTING: /* let the instance start or crash */ - if (instance->is_crashed()) - { - current_node->crash_moment= current_time; - current_node->last_checked= current_time; - current_node->state= JUST_CRASHED; - /* fallthrough -- restart an instance immediately */ - } - else - break; - case JUST_CRASHED: - if (current_time - current_node->crash_moment <= 2) + /* NOTE: set state to STARTING _before_ start() is called. */ + instance->set_state(Instance::STARTING); + instance->last_checked= current_time; + + instance->start_mysqld(); + + return; + + case Instance::STARTED: /* fallthrough */ + case Instance::STARTING: /* let the instance start or crash */ + if (!instance->is_crashed()) + return; + + instance->crash_moment= current_time; + instance->last_checked= current_time; + instance->set_state(Instance::JUST_CRASHED); + /* fallthrough -- restart an instance immediately */ + + case Instance::JUST_CRASHED: + if (current_time - instance->crash_moment <= 2) { if (instance->is_crashed()) { - instance->start(); + instance->start_mysqld(); log_info("Guardian: starting '%s'...", (const char *) instance->options.instance_name.str); } } else - current_node->state= CRASHED; - break; - case CRASHED: /* just regular restarts */ - if (current_time - current_node->last_checked > - monitoring_interval) + instance->set_state(Instance::CRASHED); + + return; + + case Instance::CRASHED: /* just regular restarts */ + if (current_time - instance->last_checked <= + Options::Main::monitoring_interval) + return; + + if (instance->restart_counter < restart_retry) { - if ((current_node->restart_counter < restart_retry)) - { - if (instance->is_crashed()) - { - instance->start(); - current_node->last_checked= current_time; - current_node->restart_counter++; - log_info("Guardian: restarting '%s'...", - (const char *) instance->options.instance_name.str); - } - } - else + if (instance->is_crashed()) { - log_info("Guardian: can not start '%s'. " - "Abandoning attempts to (re)start it", + instance->start_mysqld(); + instance->last_checked= current_time; + + log_info("Guardian: restarting '%s'...", (const char *) instance->options.instance_name.str); - current_node->state= CRASHED_AND_ABANDONED; } } - break; - case CRASHED_AND_ABANDONED: - break; /* do nothing */ + else + { + log_info("Guardian: can not start '%s'. " + "Abandoning attempts to (re)start it", + (const char *) instance->options.instance_name.str); + + instance->set_state(Instance::CRASHED_AND_ABANDONED); + } + + return; + + case Instance::CRASHED_AND_ABANDONED: + return; /* do nothing */ + default: DBUG_ASSERT(0); } @@ -217,56 +248,78 @@ void Guardian::process_instance(Instance *instance, } -/* +/** Main function of Guardian thread. SYNOPSIS run() DESCRIPTION - Check for all guarded instances and restart them if needed. If everything - is fine go and sleep for some time. + Check for all guarded instances and restart them if needed. */ void Guardian::run() { - Instance *instance; - LIST *node; struct timespec timeout; log_info("Guardian: started."); thread_registry->register_thread(&thread_info); - pthread_mutex_lock(&LOCK_guardian); + /* Loop, until all instances were shut down at the end. */ - /* loop, until all instances were shut down at the end */ - while (!(shutdown_requested && (guarded_instances == NULL))) + while (true) { - node= guarded_instances; + Instance_map::Iterator instances_it(instance_map); + Instance *instance; + bool all_instances_stopped= TRUE; + + instance_map->lock(); - while (node != NULL) + while ((instance= instances_it.next())) { - GUARD_NODE *current_node= (GUARD_NODE *) node->data; - instance= ((GUARD_NODE *) node->data)->instance; - process_instance(instance, current_node, &guarded_instances, node); + instance->lock(); - node= node->next; + if (!instance->is_guarded() || + instance->get_state() == Instance::STOPPED) + { + instance->unlock(); + continue; + } + + process_instance(instance); + + if (instance->get_state() != Instance::STOPPED) + all_instances_stopped= FALSE; + + instance->unlock(); } - timeout.tv_sec= time(NULL) + monitoring_interval; + + instance_map->unlock(); + + lock(); + + if (shutdown_requested && all_instances_stopped) + { + log_info("Guardian: all guarded mysqlds stopped."); + + stopped= TRUE; + unlock(); + break; + } + + timeout.tv_sec= time(NULL) + Options::Main::monitoring_interval; timeout.tv_nsec= 0; - /* check the loop predicate before sleeping */ - if (!(shutdown_requested && (!(guarded_instances)))) - thread_registry->cond_timedwait(&thread_info, &COND_guardian, - &LOCK_guardian, &timeout); + thread_registry->cond_timedwait(&thread_info, &COND_guardian, + &LOCK_guardian, &timeout); + unlock(); } log_info("Guardian: stopped."); - stopped= TRUE; - pthread_mutex_unlock(&LOCK_guardian); - /* now, when the Guardian is stopped we can stop the IM */ + /* Now, when the Guardian is stopped we can stop the IM. */ + thread_registry->unregister_thread(&thread_info); thread_registry->request_shutdown(); @@ -274,129 +327,65 @@ void Guardian::run() } -int Guardian::is_stopped() +/** + Return the value of stopped flag. +*/ + +bool Guardian::is_stopped() { int var; - pthread_mutex_lock(&LOCK_guardian); + + lock(); var= stopped; - pthread_mutex_unlock(&LOCK_guardian); + unlock(); + return var; } -/* - Initialize the list of guarded instances: loop through the Instance_map and - add all of the instances, which don't have 'nonguarded' option specified. - - SYNOPSIS - Guardian::init() +/** + Wake up Guardian thread. - NOTE: The operation should be invoked with the following locks acquired: - - Guardian; - - Instance_map; - - RETURN - 0 - ok - 1 - error occurred + MT-NOTE: though usually the mutex associated with condition variable should + be acquired before signalling the variable, here this is not needed. + Signalling under locked mutex is used to avoid lost signals. In the current + logic however locking mutex does not guarantee that the signal will not be + lost. */ -int Guardian::init() +void Guardian::ping() { - Instance *instance; - Instance_map::Iterator iterator(instance_map); - - /* clear the list of guarded instances */ - free_root(&alloc, MYF(0)); - init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0); - guarded_instances= NULL; - - while ((instance= iterator.next())) - { - if (instance->options.nonguarded) - continue; - - if (guard(instance, TRUE)) /* do not lock guardian */ - return 1; - } - - return 0; + pthread_cond_signal(&COND_guardian); } -/* - Add instance to the Guardian list +/** + Prepare list of instances. SYNOPSIS - guard() - instance the instance to be guarded - nolock whether we prefer do not lock Guardian here, - but use external locking instead - - DESCRIPTION - - The instance is added to the guarded instances list. Usually guard() is - called after we start an instance. + init() - RETURN - 0 - ok - 1 - error occurred + MT-NOTE: Instance Map must be locked before calling the operation. */ -int Guardian::guard(Instance *instance, bool nolock) +void Guardian::init() { - LIST *node; - GUARD_NODE *content; - - node= (LIST *) alloc_root(&alloc, sizeof(LIST)); - content= (GUARD_NODE *) alloc_root(&alloc, sizeof(GUARD_NODE)); - - if ((!(node)) || (!(content))) - return 1; - /* we store the pointers to instances from the instance_map's MEM_ROOT */ - content->instance= instance; - content->restart_counter= 0; - content->crash_moment= 0; - content->state= NOT_STARTED; - node->data= (void*) content; - - if (nolock) - guarded_instances= list_add(guarded_instances, node); - else - { - pthread_mutex_lock(&LOCK_guardian); - guarded_instances= list_add(guarded_instances, node); - pthread_mutex_unlock(&LOCK_guardian); - } - - return 0; -} - - -/* - TODO: perhaps it would make sense to create a pool of the LIST nodeents - and give them upon request. Now we are loosing a bit of memory when - guarded instance was stopped and then restarted (since we cannot free just - a piece of the MEM_ROOT). -*/ - -int Guardian::stop_guard(Instance *instance) -{ - LIST *node; - - pthread_mutex_lock(&LOCK_guardian); - - node= find_instance_node(instance); + Instance *instance; + Instance_map::Iterator iterator(instance_map); - if (node != NULL) - guarded_instances= list_delete(guarded_instances, node); + while ((instance= iterator.next())) + { + instance->lock(); - pthread_mutex_unlock(&LOCK_guardian); + instance->reset_stat(); + instance->set_state(Instance::NOT_STARTED); - /* if there is nothing to delete it is also fine */ - return 0; + instance->unlock(); + } } -/* + +/** An internal method which is called at shutdown to unregister instances and attempt to stop them if requested. @@ -409,86 +398,71 @@ int Guardian::stop_guard(Instance *instance) accordingly. NOTE - Guardian object should be locked by the calling function. + Guardian object should be locked by the caller. - RETURN - 0 - ok - 1 - error occurred */ -int Guardian::stop_instances() +void Guardian::stop_instances() { - LIST *node; - node= guarded_instances; - while (node != NULL) + Instance_map::Iterator instances_it(instance_map); + Instance *instance; + + instance_map->lock(); + + while ((instance= instances_it.next())) { - GUARD_NODE *current_node= (GUARD_NODE *) node->data; + instance->lock(); + + if (!instance->is_guarded() || + instance->get_state() == Instance::STOPPED) + { + instance->unlock(); + continue; + } + /* If instance is running or was running (and now probably hanging), request stop. */ - if (current_node->instance->is_mysqld_running() || - (current_node->state == STARTED)) + + if (instance->is_mysqld_running() || + instance->get_state() == Instance::STARTED) { - current_node->state= STOPPING; - current_node->last_checked= time(NULL); + instance->set_state(Instance::STOPPING); + instance->last_checked= time(NULL); } else - /* otherwise remove it from the list */ - guarded_instances= list_delete(guarded_instances, node); - /* But try to kill it anyway. Just in case */ - current_node->instance->kill_mysqld(SIGTERM); - node= node->next; + { + /* Otherwise mark it as STOPPED. */ + instance->set_state(Instance::STOPPED); + } + + /* Request mysqld to stop. */ + + instance->kill_mysqld(SIGTERM); + + instance->unlock(); } - return 0; + + instance_map->unlock(); } +/** + Lock Guardian. +*/ + void Guardian::lock() { pthread_mutex_lock(&LOCK_guardian); } +/** + Unlock Guardian. +*/ + void Guardian::unlock() { pthread_mutex_unlock(&LOCK_guardian); } - - -LIST *Guardian::find_instance_node(Instance *instance) -{ - LIST *node= guarded_instances; - - while (node != NULL) - { - /* - We compare only pointers, as we always use pointers from the - instance_map's MEM_ROOT. - */ - if (((GUARD_NODE *) node->data)->instance == instance) - return node; - - node= node->next; - } - - return NULL; -} - - -bool Guardian::is_active(Instance *instance) -{ - bool guarded; - - lock(); - - guarded= find_instance_node(instance) != NULL; - - /* is_running() can take a long time, so let's unlock mutex first. */ - unlock(); - - if (guarded) - return true; - - return instance->is_mysqld_running(); -} diff --git a/server-tools/instance-manager/guardian.h b/server-tools/instance-manager/guardian.h index 42d4f5e2ba4..d78058a6fc5 100644 --- a/server-tools/instance-manager/guardian.h +++ b/server-tools/instance-manager/guardian.h @@ -16,10 +16,12 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include "thread_registry.h" +#include <my_global.h> #include <my_sys.h> #include <my_list.h> +#include "thread_registry.h" + #if defined(__GNUC__) && defined(USE_PRAGMA_INTERFACE) #pragma interface #endif @@ -27,7 +29,6 @@ class Instance; class Instance_map; class Thread_registry; -struct GUARD_NODE; /** The guardian thread is responsible for monitoring and restarting of guarded @@ -37,97 +38,73 @@ struct GUARD_NODE; class Guardian: public Thread { public: - /* states of an instance */ - enum enum_instance_state { NOT_STARTED= 1, STARTING, STARTED, JUST_CRASHED, - CRASHED, CRASHED_AND_ABANDONED, STOPPING }; - - /* - The Guardian list node structure. Guardian utilizes it to store - guarded instances plus some additional info. - */ + Guardian(Thread_registry *thread_registry_arg, + Instance_map *instance_map_arg); + ~Guardian(); - struct GUARD_NODE - { - Instance *instance; - /* state of an instance (i.e. STARTED, CRASHED, etc.) */ - enum_instance_state state; - /* the amount of attemts to restart instance (cleaned up at success) */ - int restart_counter; - /* triggered at a crash */ - time_t crash_moment; - /* General time field. Used to provide timeouts (at shutdown and restart) */ - time_t last_checked; - }; - - /* Return client state name. */ - static const char *get_instance_state_name(enum_instance_state state); + void init(); - Guardian(Thread_registry *thread_registry_arg, - Instance_map *instance_map_arg, - uint monitoring_interval_arg); - virtual ~Guardian(); - /* Initialize or refresh the list of guarded instances */ - int init(); - /* Request guardian shutdown. Stop instances if needed */ +public: void request_shutdown(); - /* Start instance protection */ - int guard(Instance *instance, bool nolock= FALSE); - /* Stop instance protection */ - int stop_guard(Instance *instance); - /* Returns TRUE if guardian thread is stopped */ - int is_stopped(); + + bool is_stopped(); + void lock(); void unlock(); - /* - Return an internal list node for the given instance if the instance is - managed by Guardian. Otherwise, return NULL. + void ping(); - MT-NOTE: must be called under acquired lock. - */ - LIST *find_instance_node(Instance *instance); +protected: + virtual void run(); + +private: + void stop_instances(); - /* The operation is used to check if the instance is active or not. */ - bool is_active(Instance *instance); + void process_instance(Instance *instance); +private: /* - Return state of the given instance list node. The pointer must specify - a valid list node. + LOCK_guardian protectes the members in this section: + - shutdown_requested; + - stopped; + + Also, it is used for COND_guardian. */ - inline enum_instance_state get_instance_state(LIST *instance_node); -protected: - /* Main funtion of the thread */ - virtual void run(); + pthread_mutex_t LOCK_guardian; -public: + /* + Guardian's main loop waits on this condition. So, it should be signalled + each time, when instance state has been changed and we want Guardian to + wake up. + + TODO: Change this to having data-scoped conditions, i.e. conditions, + which indicate that some data has been changed. + */ pthread_cond_t COND_guardian; -private: - /* Prepares Guardian shutdown. Stops instances is needed */ - int stop_instances(); - /* check instance state and act accordingly */ - void process_instance(Instance *instance, GUARD_NODE *current_node, - LIST **guarded_instances, LIST *elem); + /* + This variable is set to TRUE, when Manager thread is shutting down. + The flag is used by Guardian thread to understand that it's time to + finish. + */ + bool shutdown_requested; + + /* + This flag is set to TRUE on shutdown by Guardian thread, when all guarded + mysqlds are stopped. - int stopped; + The flag is used in the Manager thread to wait for Guardian to stop all + mysqlds. + */ + bool stopped; -private: - pthread_mutex_t LOCK_guardian; Thread_info thread_info; - int monitoring_interval; Thread_registry *thread_registry; Instance_map *instance_map; - LIST *guarded_instances; - MEM_ROOT alloc; - /* this variable is set to TRUE when we want to stop Guardian thread */ - bool shutdown_requested; -}; - -inline Guardian::enum_instance_state -Guardian::get_instance_state(LIST *instance_node) -{ - return ((GUARD_NODE *) instance_node->data)->state; -} +private: + Guardian(const Guardian &); + Guardian&operator =(const Guardian &); +}; #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_GUARDIAN_H */ diff --git a/server-tools/instance-manager/instance.cc b/server-tools/instance-manager/instance.cc index 8254cdc302b..f1166fdd804 100644 --- a/server-tools/instance-manager/instance.cc +++ b/server-tools/instance-manager/instance.cc @@ -35,7 +35,9 @@ #include "thread_registry.h" #include "instance_map.h" -/* {{{ Platform-specific functions. */ +/************************************************************************* + {{{ Platform-specific functions. +*************************************************************************/ #ifndef __WIN__ typedef pid_t My_process_info; @@ -44,34 +46,6 @@ typedef PROCESS_INFORMATION My_process_info; #endif /* - Proxy thread is a simple way to avoid all pitfalls of the threads - implementation in the OS (e.g. LinuxThreads). With such a thread we - don't have to process SIGCHLD, which is a tricky business if we want - to do it in a portable way. -*/ - -class Instance_monitor: public Thread -{ -public: - Instance_monitor(Instance *instance_arg) :instance(instance_arg) {} -protected: - virtual void run(); - void start_and_monitor_instance(Instance_options *old_instance_options, - Instance_map *instance_map, - Thread_registry *thread_registry); -private: - Instance *instance; -}; - -void Instance_monitor::run() -{ - start_and_monitor_instance(&instance->options, - Manager::get_instance_map(), - Manager::get_thread_registry()); - delete this; -} - -/* Wait for an instance SYNOPSIS @@ -284,113 +258,149 @@ int kill(pid_t pid, int signum) } #endif -/* }}} */ +/************************************************************************* + }}} +*************************************************************************/ + -/* {{{ Static constants. */ +/************************************************************************* + {{{ Static constants. +*************************************************************************/ const LEX_STRING Instance::DFLT_INSTANCE_NAME= { C_STRING_WITH_LEN("mysqld") }; -/* }}} */ +/************************************************************************* + }}} +*************************************************************************/ -/* - Fork child, exec an instance and monitor it. +/************************************************************************* + {{{ Instance Monitor thread. +*************************************************************************/ - SYNOPSIS - start_and_monitor_instance() - old_instance_options Pointer to the options of the instance to be - launched. This info is likely to become obsolete - when function returns from wait_process() - instance_map Pointer to the instance_map. We use it to protect - the instance from deletion, while we are working - with it. +/** + Proxy thread is a simple way to avoid all pitfalls of the threads + implementation in the OS (e.g. LinuxThreads). With such a thread we + don't have to process SIGCHLD, which is a tricky business if we want + to do it in a portable way. - DESCRIPTION - Fork a child, then exec and monitor it. When the child is dead, - find appropriate instance (for this purpose we save its name), - set appropriate flags and wake all threads waiting for instance - to stop. - - NOTE - A separate thread for starting/monitoring instance is a simple way - to avoid all pitfalls of the threads implementation in the OS (e.g. - LinuxThreads). For one, with such a thread we don't have to process - SIGCHLD, which is a tricky business if we want to do it in a - portable way. + Instance Monitor Thread forks a child process, execs mysqld and waits for + the child to die. - RETURN - Function returns no value + Instance Monitor assumes that the monitoring instance will not be dropped. + This is guaranteed by having flag monitoring_thread_active and + Instance::is_active() operation. */ -void -Instance_monitor:: -start_and_monitor_instance(Instance_options *old_instance_options, - Instance_map *instance_map, - Thread_registry *thread_registry) +class Instance_monitor: public Thread { - Instance_name instance_name(&old_instance_options->instance_name); - Instance *current_instance; - My_process_info process_info; - Thread_info thread_info; +public: + Instance_monitor(Instance *instance_arg) :instance(instance_arg) {} +protected: + virtual void run(); + void start_and_monitor_instance(); +private: + Instance *instance; +}; + + +void Instance_monitor::run() +{ + start_and_monitor_instance(); + delete this; +} + + +void Instance_monitor::start_and_monitor_instance() +{ + Thread_registry *thread_registry= Manager::get_thread_registry(); + Guardian *guardian= Manager::get_guardian(); + + My_process_info mysqld_process_info; + Thread_info monitor_thread_info; log_info("Instance '%s': Monitor: started.", (const char *) instance->get_name()->str); - if (!old_instance_options->nonguarded) - { - /* - Register thread in Thread_registry to wait for it to stop on shutdown - only if instance is guarded. If instance is guarded, the thread will not - finish, because nonguarded instances are not stopped on shutdown. - */ - thread_registry->register_thread(&thread_info, FALSE); - } - /* - Lock instance map to guarantee that no instances are deleted during - strmake() and execv() calls. + For guarded instance register the thread in Thread_registry to wait for + the thread to stop on shutdown (nonguarded instances are not stopped on + shutdown, so the thread will no finish). */ - instance_map->lock(); - /* - Save the instance name in the case if Instance object we - are using is destroyed. (E.g. by "FLUSH INSTANCES") - */ + if (instance->is_guarded()) + { + thread_registry->register_thread(&monitor_thread_info, FALSE); + } + + /* Starting mysqld. */ log_info("Instance '%s': Monitor: starting mysqld...", (const char *) instance->get_name()->str); - if (start_process(old_instance_options, &process_info)) + if (start_process(&instance->options, &mysqld_process_info)) { - instance_map->unlock(); - return; /* error is logged */ + instance->lock(); + instance->monitoring_thread_active= FALSE; + instance->unlock(); + + return; } - /* allow users to delete instances */ - instance_map->unlock(); + /* Waiting for mysqld to die. */ log_info("Instance '%s': Monitor: waiting for mysqld to stop...", (const char *) instance->get_name()->str); - wait_process(&process_info); /* Don't check for return value. */ + wait_process(&mysqld_process_info); /* Don't check for return value. */ - instance_map->lock(); + log_info("Instance '%s': Monitor: mysqld stopped.", + (const char *) instance->get_name()->str); - current_instance= instance_map->find(instance_name.get_str()); + /* Update instance status. */ - if (current_instance) - current_instance->set_crash_flag_n_wake_all(); + instance->lock(); - instance_map->unlock(); + if (instance->is_guarded()) + thread_registry->unregister_thread(&monitor_thread_info); - if (!old_instance_options->nonguarded) - thread_registry->unregister_thread(&thread_info); + instance->crashed= TRUE; + instance->monitoring_thread_active= FALSE; log_info("Instance '%s': Monitor: finished.", (const char *) instance->get_name()->str); + + instance->unlock(); + + /* Wake up guardian. */ + + guardian->ping(); } +/************************************************************************** + }}} +**************************************************************************/ + + +/************************************************************************** + {{{ Static operations. +**************************************************************************/ + +/** + The operation is intended to check whether string is a well-formed + instance name or not. + + SYNOPSIS + is_name_valid() + name string to check + + RETURN + TRUE string is a valid instance name + FALSE string is not a valid instance name + + TODO: Move to Instance_name class: Instance_name::is_valid(). +*/ bool Instance::is_name_valid(const LEX_STRING *name) { @@ -404,21 +414,83 @@ bool Instance::is_name_valid(const LEX_STRING *name) } +/** + The operation is intended to check if the given instance name is + mysqld-compatible or not. + + SYNOPSIS + is_mysqld_compatible_name() + name name to check + + RETURN + TRUE name is mysqld-compatible + FALSE otherwise + + TODO: Move to Instance_name class: Instance_name::is_mysqld_compatible(). +*/ + bool Instance::is_mysqld_compatible_name(const LEX_STRING *name) { return strcmp(name->str, DFLT_INSTANCE_NAME.str) == 0; } +/** + Return client state name. Must not be used outside the class. + Use Instance::get_state_name() instead. +*/ + +const char * Instance::get_instance_state_name(enum_instance_state state) +{ + switch (state) { + case STOPPED: + return "offline"; + + case NOT_STARTED: + return "not started"; + + case STARTING: + return "starting"; + + case STARTED: + return "online"; + + case JUST_CRASHED: + return "failed"; + + case CRASHED: + return "crashed"; -/* {{{ Constructor & destructor */ + case CRASHED_AND_ABANDONED: + return "abandoned"; + + case STOPPING: + return "stopping"; + } + + return NULL; /* just to ignore compiler warning. */ +} + +/************************************************************************** + }}} +**************************************************************************/ + + +/************************************************************************** + {{{ Initialization & deinitialization. +**************************************************************************/ Instance::Instance() - :crashed(FALSE), - configured(FALSE) + :monitoring_thread_active(FALSE), + crashed(FALSE), + configured(FALSE), + /* mysqld_compatible is initialized in init() */ + state(NOT_STARTED), + restart_counter(0), + crash_moment(0), + last_checked(0) { pthread_mutex_init(&LOCK_instance, 0); - pthread_cond_init(&COND_instance_stopped, 0); } @@ -426,13 +498,11 @@ Instance::~Instance() { log_info("Instance '%s': destroying...", (const char *) get_name()->str); - pthread_cond_destroy(&COND_instance_stopped); pthread_mutex_destroy(&LOCK_instance); } -/* }}} */ -/* +/** Initialize instance options. SYNOPSIS @@ -452,7 +522,7 @@ bool Instance::init(const LEX_STRING *name_arg) } -/* +/** Complete instance options initialization. SYNOPSIS @@ -473,7 +543,47 @@ bool Instance::complete_initialization() */ } -/* +/************************************************************************** + }}} +**************************************************************************/ + + +/************************************************************************** + {{{ Instance: public interface implementation. +**************************************************************************/ + +/** + Determine if there is some activity with the instance. + + SYNOPSIS + is_active() + + DESCRIPTION + An instance is active if one of the following conditions is true: + - Instance-monitoring thread is running; + - Instance is guarded and its state is other than STOPPED; + - Corresponding mysqld-server accepts connections. + + MT-NOTE: instance must be locked before calling the operation. + + RETURN + TRUE - instance is active + FALSE - otherwise. +*/ + +bool Instance::is_active() +{ + if (monitoring_thread_active) + return TRUE; + + if (is_guarded() && get_state() != STOPPED) + return TRUE; + + return is_mysqld_running(); +} + + +/** Determine if mysqld is accepting connections. SYNOPSIS @@ -483,7 +593,7 @@ bool Instance::complete_initialization() Try to connect to mysqld with fake login/password to check whether it is accepting connections or not. - MT-NOTE: this operation must be called under acquired LOCK_instance. + MT-NOTE: instance must be locked before calling the operation. RETURN TRUE - mysqld is alive and accept connections @@ -507,8 +617,6 @@ bool Instance::is_mysqld_running() if (!port && !options.mysqld_socket) port= SERVER_DEFAULT_PORT; - pthread_mutex_lock(&LOCK_instance); - mysql_init(&mysql); /* try to connect to a server with a fake username/password pair */ if (mysql_real_connect(&mysql, LOCAL_HOST, username, @@ -522,7 +630,6 @@ bool Instance::is_mysqld_running() */ log_error("Instance '%s': was able to log into mysqld.", (const char *) get_name()->str); - pthread_mutex_unlock(&LOCK_instance); return_val= TRUE; /* server is alive */ } else @@ -530,145 +637,145 @@ bool Instance::is_mysqld_running() sizeof(access_denied_message) - 1)); mysql_close(&mysql); - pthread_mutex_unlock(&LOCK_instance); return return_val; } -/* - The method starts an instance. + +/** + Start mysqld. SYNOPSIS - start() + start_mysqld() + + DESCRIPTION + Reset flags and start Instance Monitor thread, which will start mysqld. + + MT-NOTE: instance must be locked before calling the operation. RETURN - 0 ok - ER_CANNOT_START_INSTANCE Cannot start instance - ER_INSTANCE_ALREADY_STARTED The instance on the specified port/socket - is already started + FALSE - ok + TRUE - could not start instance */ -int Instance::start() +bool Instance::start_mysqld() { - /* clear crash flag */ - pthread_mutex_lock(&LOCK_instance); - crashed= FALSE; - pthread_mutex_unlock(&LOCK_instance); + Instance_monitor *instance_monitor; + /* + Prepare instance to start Instance Monitor thread. - if (configured && !is_mysqld_running()) - { - Instance_monitor *instance_monitor; - remove_pid(); + NOTE: It's important to set these actions here in order to avoid + race conditions -- these actions must be done under acquired lock on + Instance. + */ - instance_monitor= new Instance_monitor(this); + crashed= FALSE; + monitoring_thread_active= TRUE; - if (instance_monitor == NULL || instance_monitor->start(Thread::DETACHED)) - { - delete instance_monitor; - log_error("Instance::start(): failed to create the monitoring thread" - " to start an instance"); - return ER_CANNOT_START_INSTANCE; - } - /* The monitoring thread will delete itself when it's finished. */ + remove_pid(); - return 0; - } + /* Create and start the Instance Monitor thread. */ - /* The instance is started already or misconfigured. */ - return configured ? ER_INSTANCE_ALREADY_STARTED : ER_INSTANCE_MISCONFIGURED; -} + instance_monitor= new Instance_monitor(this); -/* - The method sets the crash flag and wakes all waiters on - COND_instance_stopped and COND_guardian + if (instance_monitor == NULL || instance_monitor->start(Thread::DETACHED)) + { + delete instance_monitor; + monitoring_thread_active= FALSE; - SYNOPSIS - set_crash_flag_n_wake_all() + log_error("Instance '%s': can not create instance monitor thread.", + (const char *) get_name()->str); - DESCRIPTION - The method is called when an instance is crashed or terminated. - In the former case it might indicate that guardian probably should - restart it. + return TRUE; + } - RETURN - Function returns no value -*/ + ++restart_counter; -void Instance::set_crash_flag_n_wake_all() -{ - /* set instance state to crashed */ - pthread_mutex_lock(&LOCK_instance); - crashed= TRUE; - pthread_mutex_unlock(&LOCK_instance); + /* The Instance Monitor thread will delete itself when it's finished. */ - /* - Wake connection threads waiting for an instance to stop. This - is needed if a user issued command to stop an instance via - mysql connection. This is not the case if Guardian stop the thread. - */ - pthread_cond_signal(&COND_instance_stopped); - /* wake guardian */ - pthread_cond_signal(&Manager::get_guardian()->COND_guardian); + return FALSE; } -/* - Stop an instance. +/** + Stop mysqld. SYNOPSIS - stop() + stop_mysqld() - RETURN: - 0 ok - ER_INSTANCE_IS_NOT_STARTED Looks like the instance it is not started - ER_STOP_INSTANCE mysql_shutdown reported an error -*/ + DESCRIPTION + Try to stop mysqld gracefully. Otherwise kill it with SIGKILL. -int Instance::stop() -{ - struct timespec timeout; - uint waitchild= (uint) DEFAULT_SHUTDOWN_DELAY; + MT-NOTE: instance must be locked before calling the operation. - if (is_mysqld_running()) - { - waitchild= options.get_shutdown_delay(); + RETURN + FALSE - ok + TRUE - could not stop the instance +*/ - kill_mysqld(SIGTERM); - /* sleep on condition to wait for SIGCHLD */ +bool Instance::stop_mysqld() +{ + log_info("Instance '%s': stopping mysqld...", + (const char *) get_name()->str); - timeout.tv_sec= time(NULL) + waitchild; - timeout.tv_nsec= 0; - if (pthread_mutex_lock(&LOCK_instance)) - return ER_STOP_INSTANCE; + kill_mysqld(SIGTERM); - while (options.load_pid() != 0) /* while server isn't stopped */ - { - int status; + if (!wait_for_stop()) + { + log_info("Instance '%s': mysqld stopped gracefully.", + (const char *) get_name()->str); + return FALSE; + } - status= pthread_cond_timedwait(&COND_instance_stopped, - &LOCK_instance, - &timeout); - if (status == ETIMEDOUT || status == ETIME) - break; - } + log_info("Instance '%s': mysqld failed to stop gracefully within %d seconds.", + (const char *) get_name()->str, + (int) options.get_shutdown_delay()); - pthread_mutex_unlock(&LOCK_instance); + log_info("Instance'%s': killing mysqld...", + (const char *) get_name()->str); - kill_mysqld(SIGKILL); + kill_mysqld(SIGKILL); - return 0; + if (!wait_for_stop()) + { + log_info("Instance '%s': mysqld has been killed.", + (const char *) get_name()->str); + return FALSE; } - return ER_INSTANCE_IS_NOT_STARTED; + log_info("Instance '%s': can not kill mysqld within %d seconds.", + (const char *) get_name()->str, + (int) options.get_shutdown_delay()); + + return TRUE; } -/* +/** Send signal to mysqld. SYNOPSIS kill_mysqld() + + DESCRIPTION + Load pid from the pid file and send the given signal to that process. + If the signal is SIGKILL, remove the pid file after sending the signal. + + MT-NOTE: instance must be locked before calling the operation. + + TODO + This too low-level and OS-specific operation for public interface. + Also, it has some implicit behaviour for SIGKILL signal. Probably, we + should have the following public operations instead: + - start_mysqld() -- as is; + - stop_mysqld -- request mysqld to shutdown gracefully (send SIGTERM); + don't wait for complete shutdown; + - wait_for_stop() (or join_mysqld()) -- wait for mysqld to stop within + time interval; + - kill_mysqld() -- request to terminate mysqld; don't wait for + completion. + These operations should also be used in Guardian to manage instances. */ void Instance::kill_mysqld(int signum) @@ -706,27 +813,91 @@ void Instance::kill_mysqld(int signum) } } -/* - Return crashed flag. - SYNOPSIS - is_crashed() - - RETURN - TRUE - mysqld crashed - FALSE - mysqld hasn't crashed yet +/** + Lock instance. */ -bool Instance::is_crashed() +void Instance::lock() { - bool val; pthread_mutex_lock(&LOCK_instance); - val= crashed; +} + + +/** + Unlock instance. +*/ + +void Instance::unlock() +{ pthread_mutex_unlock(&LOCK_instance); - return val; } -/* + +/** + Return instance state name. + + SYNOPSIS + get_state_name() + + DESCRIPTION + The operation returns user-friendly state name. The operation can be + used both for guarded and non-guarded instances. + + MT-NOTE: instance must be locked before calling the operation. + + TODO: Replace with the static get_state_name(state_code) function. +*/ + +const char *Instance::get_state_name() +{ + if (!is_configured()) + return "misconfigured"; + + if (is_guarded()) + { + /* The instance is managed by Guardian: we can report precise state. */ + + return get_instance_state_name(get_state()); + } + + /* The instance is not managed by Guardian: we can report status only. */ + + return is_active() ? "online" : "offline"; +} + + +/** + Reset statistics. + + SYNOPSIS + reset_stat() + + DESCRIPTION + The operation resets statistics used for guarding the instance. + + MT-NOTE: instance must be locked before calling the operation. + + TODO: Make private. +*/ + +void Instance::reset_stat() +{ + restart_counter= 0; + crash_moment= 0; + last_checked= 0; +} + +/************************************************************************** + }}} +**************************************************************************/ + + +/************************************************************************** + {{{ Instance: implementation of private operations. +**************************************************************************/ + +/** Remove pid file. */ @@ -743,3 +914,36 @@ void Instance::remove_pid() (const char *) options.instance_name.str); } } + + +/** + Wait for mysqld to stop within shutdown interval. +*/ + +bool Instance::wait_for_stop() +{ + int start_time= time(NULL); + int finish_time= start_time + options.get_shutdown_delay(); + + log_info("Instance '%s': waiting for mysqld to stop " + "(timeout: %d seconds)...", + (const char *) get_name()->str, + (int) options.get_shutdown_delay()); + + while (true) + { + if (options.load_pid() == 0 && !is_mysqld_running()) + return FALSE; + + if (time(NULL) >= finish_time) + return TRUE; + + /* Sleep for 0.3 sec and check again. */ + + my_sleep(300000); + } +} + +/************************************************************************** + }}} +**************************************************************************/ diff --git a/server-tools/instance-manager/instance.h b/server-tools/instance-manager/instance.h index f6fe1fa2b31..e87bb49b49c 100644 --- a/server-tools/instance-manager/instance.h +++ b/server-tools/instance-manager/instance.h @@ -29,7 +29,7 @@ class Instance_map; class Thread_registry; -/* +/** Instance_name -- the class represents instance name -- a string of length less than MAX_INSTANCE_NAME_SIZE. @@ -67,72 +67,127 @@ private: class Instance { public: - /* - The following two constants defines name of the default mysqld-instance - ("mysqld"). + /* States of an instance. */ + enum enum_instance_state + { + STOPPED, + NOT_STARTED, + STARTING, + STARTED, + JUST_CRASHED, + CRASHED, + CRASHED_AND_ABANDONED, + STOPPING + }; + +public: + /** + The constant defines name of the default mysqld-instance ("mysqld"). */ static const LEX_STRING DFLT_INSTANCE_NAME; public: - /* - The operation is intended to check whether string is a well-formed - instance name or not. - */ static bool is_name_valid(const LEX_STRING *name); - - /* - The operation is intended to check if the given instance name is - mysqld-compatible or not. - */ static bool is_mysqld_compatible_name(const LEX_STRING *name); public: Instance(); - ~Instance(); + bool init(const LEX_STRING *name_arg); bool complete_initialization(); +public: + bool is_active(); + bool is_mysqld_running(); - int start(); - int stop(); - /* send a signal to the instance */ + + bool start_mysqld(); + bool stop_mysqld(); void kill_mysqld(int signo); - bool is_crashed(); - void set_crash_flag_n_wake_all(); - /* + void lock(); + void unlock(); + + const char *get_state_name(); + + void reset_stat(); + +public: + /** The operation is intended to check if the instance is mysqld-compatible or not. */ inline bool is_mysqld_compatible() const; - /* + /** The operation is intended to check if the instance is configured properly or not. Misconfigured instances are not managed. */ inline bool is_configured() const; + /** + The operation returns TRUE if the instance is guarded and FALSE otherwise. + */ + inline bool is_guarded() const; + + /** + The operation returns name of the instance. + */ inline const LEX_STRING *get_name() const; + /** + The operation returns the current state of the instance. + + NOTE: At the moment should be used only for guarded instances. + */ + inline enum_instance_state get_state() const; + + /** + The operation changes the state of the instance. + + NOTE: At the moment should be used only for guarded instances. + TODO: Make private. + */ + inline void set_state(enum_instance_state new_state); + + /** + The operation returns crashed flag. + */ + inline bool is_crashed(); + public: - enum { DEFAULT_SHUTDOWN_DELAY= 35 }; + /** + This attributes contains instance options. + + TODO: Make private. + */ Instance_options options; private: - /* This attributes is a flag, specifies if the instance has been crashed. */ + /** + monitoring_thread_active is TRUE if there is a thread that monitors the + corresponding mysqld-process. + */ + bool monitoring_thread_active; + + /** + crashed is TRUE when corresponding mysqld-process has been died after + start. + */ bool crashed; - /* - This attribute specifies if the instance is configured properly or not. + /** + configured is TRUE when the instance is configured and FALSE otherwise. Misconfigured instances are not managed. */ bool configured; /* - This attribute specifies whether the instance is mysqld-compatible or not. - Mysqld-compatible instances can contain only mysqld-specific options. - At the moment an instance is mysqld-compatible if its name is "mysqld". + mysqld_compatible specifies whether the instance is mysqld-compatible + or not. Mysqld-compatible instances can contain only mysqld-specific + options. At the moment an instance is mysqld-compatible if its name is + "mysqld". The idea is that [mysqld] section should contain only mysqld-specific options (no Instance Manager-specific options) to be readable by mysqld @@ -141,18 +196,36 @@ private: bool mysqld_compatible; /* - Mutex protecting the instance. Currently we use it to avoid the - double start of the instance. This happens when the instance is starting - and we issue the start command once more. + Mutex protecting the instance. */ pthread_mutex_t LOCK_instance; - /* - This condition variable is used to wake threads waiting for instance to - stop in Instance::stop() - */ - pthread_cond_t COND_instance_stopped; - void remove_pid(); +private: + /* Guarded-instance attributes. */ + + /* state of an instance (i.e. STARTED, CRASHED, etc.) */ + enum_instance_state state; + +public: + /* the amount of attemts to restart instance (cleaned up at success) */ + int restart_counter; + + /* triggered at a crash */ + time_t crash_moment; + + /* General time field. Used to provide timeouts (at shutdown and restart) */ + time_t last_checked; + +private: + static const char *get_instance_state_name(enum_instance_state state); + +private: + void remove_pid(); + + bool wait_for_stop(); + +private: + friend class Instance_monitor; }; @@ -168,9 +241,33 @@ inline bool Instance::is_configured() const } +inline bool Instance::is_guarded() const +{ + return !options.nonguarded; +} + + inline const LEX_STRING *Instance::get_name() const { return &options.instance_name; } + +inline Instance::enum_instance_state Instance::get_state() const +{ + return state; +} + + +inline void Instance::set_state(enum_instance_state new_state) +{ + state= new_state; +} + + +inline bool Instance::is_crashed() +{ + return crashed; +} + #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_INSTANCE_H */ diff --git a/server-tools/instance-manager/instance_map.cc b/server-tools/instance-manager/instance_map.cc index 204064233eb..5d492fe5c11 100644 --- a/server-tools/instance-manager/instance_map.cc +++ b/server-tools/instance-manager/instance_map.cc @@ -24,26 +24,18 @@ #include <mysql_com.h> #include "buffer.h" -#include "guardian.h" #include "instance.h" #include "log.h" -#include "manager.h" #include "mysqld_error.h" #include "mysql_manager_error.h" #include "options.h" #include "priv.h" -/* - Note: As we are going to suppost different types of connections, - we shouldn't have connection-specific functions. To avoid it we could - put such functions to the Command-derived class instead. - The command could be easily constructed for a specific connection if - we would provide a special factory for each connection. -*/ - C_MODE_START -/* Procedure needed for HASH initialization */ +/** + HASH-routines: get key of instance for storing in hash. +*/ static byte* get_instance_key(const byte* u, uint* len, my_bool __attribute__((unused)) t) @@ -53,14 +45,18 @@ static byte* get_instance_key(const byte* u, uint* len, return (byte *) instance->options.instance_name.str; } +/** + HASH-routines: cleanup handler. +*/ + static void delete_instance(void *u) { Instance *instance= (Instance *) u; delete instance; } -/* - The option handler to pass to the process_default_option_files finction. +/** + The option handler to pass to the process_default_option_files function. SYNOPSIS process_option() @@ -95,8 +91,8 @@ static int process_option(void *ctx, const char *group, const char *option) C_MODE_END -/* - Parse option string. +/** + Parse option string. SYNOPSIS parse_option() @@ -136,7 +132,7 @@ static void parse_option(const char *option_str, } -/* +/** Process one option from the configuration file. SYNOPSIS @@ -150,6 +146,10 @@ static void parse_option(const char *option_str, process_option(). The caller ensures proper locking of the instance map object. */ + /* + Process a given option and assign it to appropricate instance. This is + required for the option handler, passed to my_search_option_files(). + */ int Instance_map::process_one_option(const LEX_STRING *group, const char *option) @@ -212,92 +212,97 @@ int Instance_map::process_one_option(const LEX_STRING *group, } +/** + Instance_map constructor. +*/ + Instance_map::Instance_map() { pthread_mutex_init(&LOCK_instance_map, 0); } +/** + Initialize Instance_map internals. +*/ + bool Instance_map::init() { return hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0, get_instance_key, delete_instance, 0); } + +/** + Reset Instance_map data. +*/ + +bool Instance_map::reset() +{ + hash_free(&hash); + return init(); +} + + +/** + Instance_map destructor. +*/ + Instance_map::~Instance_map() { - pthread_mutex_lock(&LOCK_instance_map); + lock(); + + /* + NOTE: it's necessary to synchronize on each instance before removal, + because Instance-monitoring thread can be still alive an hold the mutex + (because it is detached and we have no control over it). + */ + + while (true) + { + Iterator it(this); + Instance *instance= it.next(); + + if (!instance) + break; + + instance->lock(); + instance->unlock(); + + remove_instance(instance); + } + hash_free(&hash); - pthread_mutex_unlock(&LOCK_instance_map); + unlock(); + pthread_mutex_destroy(&LOCK_instance_map); } +/** + Lock Instance_map. +*/ + void Instance_map::lock() { pthread_mutex_lock(&LOCK_instance_map); } +/** + Unlock Instance_map. +*/ + void Instance_map::unlock() { pthread_mutex_unlock(&LOCK_instance_map); } -/* - Re-read instance configuration file. - - SYNOPSIS - Instance_map::flush_instances() - DESCRIPTION - This function will: - - clear the current list of instances. This removes both - running and stopped instances. - - load a new instance configuration from the file. - - pass on the new map to the guardian thread: it will start - all instances that are marked `guarded' and not yet started. - Note, as the check whether an instance is started is currently - very simple (returns TRUE if there is a MySQL server running - at the given port), this function has some peculiar - side-effects: - * if the port number of a running instance was changed, the - old instance is forgotten, even if it was running. The new - instance will be started at the new port. - * if the configuration was changed in a way that two - instances swapped their port numbers, the guardian thread - will not notice that and simply report that both instances - are configured successfully and running. - In order to avoid such side effects one should never call - FLUSH INSTANCES without prior stop of all running instances. - - NOTE: The operation should be invoked with the following locks acquired: - - Guardian; - - Instance_map; +/** + Check if there is an active instance or not. */ -int Instance_map::flush_instances() -{ - int rc; - - /* - Guardian thread relies on the instance map repository for guarding - instances. This is why refreshing instance map, we need (1) to stop - guardian (2) reload the instance map (3) reinitialize the guardian - with new instances. - */ - hash_free(&hash); - hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0, - get_instance_key, delete_instance, 0); - - rc= load(); - /* don't init guardian if we failed to load instances */ - if (!rc) - guardian->init(); // TODO: check error status. - return rc; -} - - bool Instance_map::is_there_active_instance() { Instance *instance; @@ -305,29 +310,50 @@ bool Instance_map::is_there_active_instance() while ((instance= iterator.next())) { - if (guardian->find_instance_node(instance) != NULL || - instance->is_mysqld_running()) - { + bool active_instance_found; + + instance->lock(); + active_instance_found= instance->is_active(); + instance->unlock(); + + if (active_instance_found) return TRUE; - } } return FALSE; } +/** + Add an instance into the internal hash. + + MT-NOTE: Instance Map must be locked before calling the operation. +*/ + int Instance_map::add_instance(Instance *instance) { return my_hash_insert(&hash, (byte *) instance); } +/** + Remove instance from the internal hash. + + MT-NOTE: Instance Map must be locked before calling the operation. +*/ + int Instance_map::remove_instance(Instance *instance) { return hash_delete(&hash, (byte *) instance); } +/** + Create a new instance and register it in the internal hash. + + MT-NOTE: Instance Map must be locked before calling the operation. +*/ + int Instance_map::create_instance(const LEX_STRING *instance_name, const Named_value_arr *options) { @@ -391,12 +417,22 @@ int Instance_map::create_instance(const LEX_STRING *instance_name, } +/** + Return a pointer to the instance or NULL, if there is no such instance. + + MT-NOTE: Instance Map must be locked before calling the operation. +*/ + Instance * Instance_map::find(const LEX_STRING *name) { return (Instance *) hash_search(&hash, (byte *) name->str, name->length); } +/** + Init instances command line arguments after all options have been loaded. +*/ + bool Instance_map::complete_initialization() { bool mysqld_found; @@ -454,7 +490,10 @@ bool Instance_map::complete_initialization() } -/* load options from config files and create appropriate instance structures */ +/** + Load options from config files and create appropriate instance + structures. +*/ int Instance_map::load() { @@ -504,8 +543,9 @@ int Instance_map::load() } -/*--- Implementaton of the Instance map iterator class ---*/ - +/************************************************************************* + {{{ Instance_map::Iterator implementation. +*************************************************************************/ void Instance_map::Iterator::go_to_first() { @@ -521,29 +561,12 @@ Instance *Instance_map::Iterator::next() return NULL; } - -const char *Instance_map::get_instance_state_name(Instance *instance) -{ - LIST *instance_node; - - if (!instance->is_configured()) - return "misconfigured"; - - if ((instance_node= guardian->find_instance_node(instance)) != NULL) - { - /* The instance is managed by Guardian: we can report precise state. */ - - return Guardian::get_instance_state_name( - guardian->get_instance_state(instance_node)); - } - - /* The instance is not managed by Guardian: we can report status only. */ - - return instance->is_mysqld_running() ? "online" : "offline"; -} +/************************************************************************* + }}} +*************************************************************************/ -/* +/** Create a new configuration section for mysqld-instance in the config file. SYNOPSIS diff --git a/server-tools/instance-manager/instance_map.h b/server-tools/instance-manager/instance_map.h index 71ee7ec4c6e..af2f1868195 100644 --- a/server-tools/instance-manager/instance_map.h +++ b/server-tools/instance-manager/instance_map.h @@ -36,14 +36,17 @@ extern int create_instance_in_file(const LEX_STRING *instance_name, const Named_value_arr *options); -/* +/** Instance_map - stores all existing instances */ class Instance_map { public: - /* Instance_map iterator */ + /** + Instance_map iterator + */ + class Iterator { private: @@ -57,79 +60,43 @@ public: void go_to_first(); Instance *next(); }; - friend class Iterator; + public: - /* - Return a pointer to the instance or NULL, if there is no such instance. - MT-NOTE: must be called under acquired lock. - */ Instance *find(const LEX_STRING *name); - /* Clear the configuration cache and reload the configuration file. */ - int flush_instances(); - - /* The operation is used to check if there is an active instance or not. */ bool is_there_active_instance(); void lock(); void unlock(); bool init(); + bool reset(); - /* - Process a given option and assign it to appropricate instance. This is - required for the option handler, passed to my_search_option_files(). - */ - int process_one_option(const LEX_STRING *group, const char *option); + int load(); - /* - Add an instance into the internal hash. + int process_one_option(const LEX_STRING *group, const char *option); - MT-NOTE: the operation must be called under acquired lock. - */ int add_instance(Instance *instance); - /* - Remove instance from the internal hash. - - MT-NOTE: the operation must be called under acquired lock. - */ int remove_instance(Instance *instance); - /* - Create a new instance and register it in the internal hash. - - MT-NOTE: the operation must be called under acquired lock. - */ int create_instance(const LEX_STRING *instance_name, const Named_value_arr *options); +public: Instance_map(); ~Instance_map(); - /* - Retrieve client state name of the given instance. - - MT-NOTE: the options must be called under acquired locks of the following - objects: - - Instance_map; - - Guardian; - */ - const char *get_instance_state_name(Instance *instance); - -public: - const char *mysqld_path; - Guardian *guardian; - private: - /* loads options from config files */ - int load(); - /* inits instances argv's after all options have been loaded */ bool complete_initialization(); + private: enum { START_HASH_SIZE = 16 }; pthread_mutex_t LOCK_instance_map; HASH hash; + +private: + friend class Iterator; }; #endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_INSTANCE_MAP_H */ diff --git a/server-tools/instance-manager/instance_options.h b/server-tools/instance-manager/instance_options.h index e61aec86fe3..b0503815036 100644 --- a/server-tools/instance-manager/instance_options.h +++ b/server-tools/instance-manager/instance_options.h @@ -45,7 +45,6 @@ public: Instance_options(); ~Instance_options(); - /* fills in argv */ bool complete_initialization(); bool set_option(Named_value *option); diff --git a/server-tools/instance-manager/listener.cc b/server-tools/instance-manager/listener.cc index e2f3926bd81..43cc3f66c94 100644 --- a/server-tools/instance-manager/listener.cc +++ b/server-tools/instance-manager/listener.cc @@ -153,7 +153,7 @@ void Listener::run() else { shutdown(client_fd, SHUT_RDWR); - close(client_fd); + closesocket(client_fd); } } } @@ -165,7 +165,7 @@ void Listener::run() log_info("Listener: shutdown requested, exiting..."); for (i= 0; i < num_sockets; i++) - close(sockets[i]); + closesocket(sockets[i]); #ifndef __WIN__ unlink(unix_socket_address.sun_path); @@ -179,7 +179,7 @@ void Listener::run() err: // we have to close the ip sockets in case of error for (i= 0; i < num_sockets; i++) - close(sockets[i]); + closesocket(sockets[i]); thread_registry->unregister_thread(&thread_info); thread_registry->request_shutdown(); @@ -227,7 +227,7 @@ int Listener::create_tcp_socket() { log_error("Listener: bind(ip socket) failed: %s.", (const char *) strerror(errno)); - close(ip_socket); + closesocket(ip_socket); return -1; } @@ -235,7 +235,7 @@ int Listener::create_tcp_socket() { log_error("Listener: listen(ip socket) failed: %s.", (const char *) strerror(errno)); - close(ip_socket); + closesocket(ip_socket); return -1; } diff --git a/server-tools/instance-manager/manager.cc b/server-tools/instance-manager/manager.cc index afcba2247e7..e126b407522 100644 --- a/server-tools/instance-manager/manager.cc +++ b/server-tools/instance-manager/manager.cc @@ -36,6 +36,9 @@ #include "user_map.h" +/********************************************************************** + {{{ Platform-specific implementation. +**********************************************************************/ #ifndef __WIN__ void set_signals(sigset_t *mask) @@ -91,9 +94,13 @@ int my_sigwait(const sigset_t *set, int *sig) #endif +/********************************************************************** + }}} +**********************************************************************/ + /********************************************************************** - Implementation of checking the actual thread model. + {{{ Implementation of checking the actual thread model. ***********************************************************************/ namespace { /* no-indent */ @@ -136,6 +143,10 @@ bool check_if_linux_threads(bool *linux_threads) } +/********************************************************************** + }}} +***********************************************************************/ + /********************************************************************** Manager implementation @@ -151,25 +162,37 @@ bool Manager::linux_threads; #endif // __WIN__ +/** + Request shutdown of guardian and threads registered in Thread_registry. + + SYNOPSIS + stop_all_threads() +*/ + void Manager::stop_all_threads() { /* - Let guardian thread know that it should break it's processing cycle, + Let Guardian thread know that it should break it's processing cycle, once it wakes up. */ p_guardian->request_shutdown(); - /* wake guardian */ - pthread_cond_signal(&p_guardian->COND_guardian); - /* stop all threads */ + + /* Stop all threads. */ p_thread_registry->deliver_shutdown(); } -/* - manager - entry point to the main instance manager process: start - listener thread, write pid file and enter into signal handling. - See also comments in mysqlmanager.cc to picture general Instance Manager - architecture. +/** + Main manager function. + + SYNOPSIS + main() + + DESCRIPTION + This is an entry point to the main instance manager process: + start listener thread, write pid file and enter into signal handling. + See also comments in mysqlmanager.cc to picture general Instance Manager + architecture. TODO: how about returning error status. */ @@ -193,22 +216,33 @@ int Manager::main() (const char *) (linux_threads ? "LINUX threads" : "POSIX threads")); #endif // __WIN__ - Thread_registry thread_registry; /* - All objects created in the manager() function live as long as - thread_registry lives, and thread_registry is alive until there are - working threads. + All objects created in the Manager object live as long as thread_registry + lives, and thread_registry is alive until there are working threads. + + There are two main purposes of the Thread Registry: + 1. Interrupt blocking I/O and signal condition variables in case of + shutdown; + 2. Wait for detached threads before shutting down the main thread. + + NOTE: + 1. Handling shutdown can be done in more elegant manner by introducing + Event (or Condition) object with support of logical operations. + 2. Using Thread Registry to wait for detached threads is definitely not + the best way, because when Thread Registry unregisters an thread, the + thread is still alive. Accurate way to wait for threads to stop is + not using detached threads and join all threads before shutdown. */ + Thread_registry thread_registry; User_map user_map; Instance_map instance_map; - Guardian guardian(&thread_registry, &instance_map, - Options::Main::monitoring_interval); + Guardian guardian(&thread_registry, &instance_map); Listener listener(&thread_registry, &user_map); p_instance_map= &instance_map; - p_guardian= instance_map.guardian= &guardian; + p_guardian= &guardian; p_thread_registry= &thread_registry; p_user_map= &user_map; @@ -248,7 +282,7 @@ int Manager::main() } } - /* write Instance Manager pid file */ + /* Write Instance Manager pid file. */ log_info("IM pid file: '%s'; PID: %d.", (const char *) Options::Main::pid_file_name, @@ -289,6 +323,7 @@ int Manager::main() permitted to process instances. And before flush_instances() has completed, there are no instances to guard. */ + if (guardian.start(Thread::DETACHED)) { log_error("Can not start Guardian thread."); @@ -297,21 +332,11 @@ int Manager::main() /* Load instances. */ + if (Manager::flush_instances()) { - instance_map.guardian->lock(); - instance_map.lock(); - - int flush_instances_status= instance_map.flush_instances(); - - instance_map.unlock(); - instance_map.guardian->unlock(); - - if (flush_instances_status) - { - log_error("Can not init instances repository."); - stop_all_threads(); - goto err; - } + log_error("Can not init instances repository."); + stop_all_threads(); + goto err; } /* Initialize the Listener. */ @@ -327,7 +352,8 @@ int Manager::main() After the list of guarded instances have been initialized, Guardian should start them. */ - pthread_cond_signal(&guardian.COND_guardian); + + guardian.ping(); /* Main loop. */ @@ -380,7 +406,6 @@ int Manager::main() if (!guardian.is_stopped()) { guardian.request_shutdown(); - pthread_cond_signal(&guardian.COND_guardian); } else { @@ -405,3 +430,64 @@ err: #endif return rc; } + + +/** + Re-read instance configuration file. + + SYNOPSIS + flush_instances() + + DESCRIPTION + This function will: + - clear the current list of instances. This removes both + running and stopped instances. + - load a new instance configuration from the file. + - pass on the new map to the guardian thread: it will start + all instances that are marked `guarded' and not yet started. + + Note, as the check whether an instance is started is currently + very simple (returns TRUE if there is a MySQL server running + at the given port), this function has some peculiar + side-effects: + * if the port number of a running instance was changed, the + old instance is forgotten, even if it was running. The new + instance will be started at the new port. + * if the configuration was changed in a way that two + instances swapped their port numbers, the guardian thread + will not notice that and simply report that both instances + are configured successfully and running. + + In order to avoid such side effects one should never call + FLUSH INSTANCES without prior stop of all running instances. +*/ + +bool Manager::flush_instances() +{ + p_instance_map->lock(); + + if (p_instance_map->is_there_active_instance()) + { + p_instance_map->unlock(); + return TRUE; + } + + if (p_instance_map->reset()) + { + p_instance_map->unlock(); + return TRUE; + } + + if (p_instance_map->load()) + { + p_instance_map->unlock(); + return TRUE; /* Don't init guardian if we failed to load instances. */ + } + + get_guardian()->init(); /* TODO: check error status. */ + get_guardian()->ping(); + + p_instance_map->unlock(); + + return FALSE; +} diff --git a/server-tools/instance-manager/manager.h b/server-tools/instance-manager/manager.h index 9d970483dde..16322ddb71f 100644 --- a/server-tools/instance-manager/manager.h +++ b/server-tools/instance-manager/manager.h @@ -19,6 +19,7 @@ #if defined(__GNUC__) && defined(USE_PRAGMA_INTERFACE) #pragma interface #endif + #include <my_global.h> class Guardian; @@ -30,8 +31,12 @@ class Manager { public: static int main(); + + static bool flush_instances(); + +public: /** - These methods return a non-zero value only for the duration + These methods return a non-NULL value only for the duration of main(). */ static Instance_map *get_instance_map() { return p_instance_map; } @@ -39,6 +44,7 @@ public: static Thread_registry *get_thread_registry() { return p_thread_registry; } static User_map *get_user_map() { return p_user_map; } +public: #ifndef __WIN__ static bool is_linux_threads() { return linux_threads; } #endif // __WIN__ diff --git a/server-tools/instance-manager/user_map.cc b/server-tools/instance-manager/user_map.cc index 3829415b93a..48cce142db6 100644 --- a/server-tools/instance-manager/user_map.cc +++ b/server-tools/instance-manager/user_map.cc @@ -41,7 +41,7 @@ int User::init(const char *line) if (name_end == 0 || name_end[1] != ':') { log_error("Invalid format (unmatched quote) of user line (%s).", - (const char *) line); + (const char *) line); return 1; } password= name_end + 2; @@ -53,7 +53,7 @@ int User::init(const char *line) if (name_end == 0) { log_error("Invalid format (no delimiter) of user line (%s).", - (const char *) line); + (const char *) line); return 1; } password= name_end + 1; @@ -63,10 +63,10 @@ int User::init(const char *line) if (user_length > USERNAME_LENGTH) { log_error("User name is too long (%d). Max length: %d. " - "User line: '%s'.", - (int) user_length, - (int) USERNAME_LENGTH, - (const char *) line); + "User line: '%s'.", + (int) user_length, + (int) USERNAME_LENGTH, + (const char *) line); return 1; } @@ -74,10 +74,10 @@ int User::init(const char *line) if (password_length > SCRAMBLED_PASSWORD_CHAR_LENGTH) { log_error("Password is too long (%d). Max length: %d." - "User line: '%s'.", - (int) password_length, - (int) SCRAMBLED_PASSWORD_CHAR_LENGTH, - line); + "User line: '%s'.", + (int) password_length, + (int) SCRAMBLED_PASSWORD_CHAR_LENGTH, + (const char *) line); return 1; } diff --git a/sql/event_data_objects.cc b/sql/event_data_objects.cc index eef45f93b7a..54b043bd916 100644 --- a/sql/event_data_objects.cc +++ b/sql/event_data_objects.cc @@ -394,7 +394,14 @@ Event_parse_data::init_starts(THD *thd) if ((not_used= item_starts->get_date(<ime, TIME_NO_ZERO_DATE))) goto wrong_value; - /* Let's check whether time is in the past */ + /* + Let's check whether time is in the past. + Note: This call is not post year 2038 safe. That's because + thd->query_start() is of time_t, while gmt_sec_to_TIME() + wants my_time_t. In the case time_t is larger than my_time_t + an overflow might happen and events subsystem will not work as + expected. + */ thd->variables.time_zone->gmt_sec_to_TIME(&time_tmp, (my_time_t) thd->query_start()); @@ -406,12 +413,12 @@ Event_parse_data::init_starts(THD *thd) goto wrong_value; /* - This may result in a 1970-01-01 date if ltime is > 2037-xx-xx. - CONVERT_TZ has similar problem. - mysql_priv.h currently lists + Again, after 2038 this code won't work. As + mysql_priv.h currently lists #define TIMESTAMP_MAX_YEAR 2038 (see TIME_to_timestamp()) */ - my_tz_UTC->gmt_sec_to_TIME(<ime,t=TIME_to_timestamp(thd, <ime, ¬_used)); + my_tz_UTC->gmt_sec_to_TIME(<ime,t=TIME_to_timestamp(thd, <ime, + ¬_used)); if (!t) goto wrong_value; @@ -464,13 +471,13 @@ Event_parse_data::init_ends(THD *thd) goto error_bad_params; /* - This may result in a 1970-01-01 date if ltime is > 2037-xx-xx. - CONVERT_TZ has similar problem. - mysql_priv.h currently lists + Again, after 2038 this code won't work. As + mysql_priv.h currently lists #define TIMESTAMP_MAX_YEAR 2038 (see TIME_to_timestamp()) */ DBUG_PRINT("info", ("get the UTC time")); - my_tz_UTC->gmt_sec_to_TIME(<ime,t=TIME_to_timestamp(thd, <ime, ¬_used)); + my_tz_UTC->gmt_sec_to_TIME(<ime,t=TIME_to_timestamp(thd, <ime, + ¬_used)); if (!t) goto error_bad_params; diff --git a/sql/event_queue.cc b/sql/event_queue.cc index 6ff5fe55cd6..9a740114193 100644 --- a/sql/event_queue.cc +++ b/sql/event_queue.cc @@ -159,7 +159,6 @@ Event_queue::init_queue(THD *thd, Event_db_repository *db_repo) { sql_print_error("SCHEDULER: sizeof(my_time_t) != sizeof(time_t) ." "The scheduler may not work correctly. Stopping"); - DBUG_ASSERT(0); goto err; } diff --git a/sql/handler.cc b/sql/handler.cc index e0955132998..3d47a6a2eaf 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -691,6 +691,19 @@ int ha_commit_trans(THD *thd, bool all) ha_rollback_trans(thd, all); DBUG_RETURN(1); } + + if ( is_real_trans + && opt_readonly + && ! (thd->security_ctx->master_access & SUPER_ACL) + && ! thd->slave_thread + ) + { + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + ha_rollback_trans(thd, all); + error= 1; + goto end; + } + DBUG_EXECUTE_IF("crash_commit_before", abort();); /* Close all cursors that can not survive COMMIT */ diff --git a/sql/lock.cc b/sql/lock.cc index 2bcee1e4baa..533307c6b85 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -150,6 +150,23 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, } } + if ( write_lock_used + && opt_readonly + && ! (thd->security_ctx->master_access & SUPER_ACL) + && ! thd->slave_thread + ) + { + /* + Someone has issued SET GLOBAL READ_ONLY=1 and we want a write lock. + We do not wait for READ_ONLY=0, and fail. + */ + reset_lock_data(sql_lock); + my_free((gptr) sql_lock, MYF(0)); + sql_lock=0; + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + break; + } + thd->proc_info="System lock"; DBUG_PRINT("info", ("thd->proc_info %s", thd->proc_info)); if (lock_external(thd, tables, count)) diff --git a/sql/set_var.cc b/sql/set_var.cc index b998548a6ab..4d2195b2306 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -369,7 +369,7 @@ sys_var_thd_ulong sys_preload_buff_size("preload_buffer_size", &SV::preload_buff_size); sys_var_thd_ulong sys_read_buff_size("read_buffer_size", &SV::read_buff_size); -sys_var_bool_ptr sys_readonly("read_only", &opt_readonly); +sys_var_opt_readonly sys_readonly("read_only", &opt_readonly); sys_var_thd_ulong sys_read_rnd_buff_size("read_rnd_buffer_size", &SV::read_rnd_buff_size); sys_var_thd_ulong sys_div_precincrement("div_precision_increment", @@ -3857,6 +3857,70 @@ bool sys_var_trust_routine_creators::update(THD *thd, set_var *var) return sys_var_bool_ptr::update(thd, var); } +bool sys_var_opt_readonly::update(THD *thd, set_var *var) +{ + bool result; + + DBUG_ENTER("sys_var_opt_readonly::update"); + + /* Prevent self dead-lock */ + if (thd->locked_tables || thd->active_transaction()) + { + my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0)); + DBUG_RETURN(true); + } + + if (thd->global_read_lock) + { + /* + This connection already holds the global read lock. + This can be the case with: + - FLUSH TABLES WITH READ LOCK + - SET GLOBAL READ_ONLY = 1 + */ + result= sys_var_bool_ptr::update(thd, var); + DBUG_RETURN(result); + } + + /* + Perform a 'FLUSH TABLES WITH READ LOCK'. + This is a 3 step process: + - [1] lock_global_read_lock() + - [2] close_cached_tables() + - [3] make_global_read_lock_block_commit() + [1] prevents new connections from obtaining tables locked for write. + [2] waits until all existing connections close their tables. + [3] prevents transactions from being committed. + */ + + if (lock_global_read_lock(thd)) + DBUG_RETURN(true); + + /* + This call will be blocked by any connection holding a READ or WRITE lock. + Ideally, we want to wait only for pending WRITE locks, but since: + con 1> LOCK TABLE T FOR READ; + con 2> LOCK TABLE T FOR WRITE; (blocked by con 1) + con 3> SET GLOBAL READ ONLY=1; (blocked by con 2) + can cause to wait on a read lock, it's required for the client application + to unlock everything, and acceptable for the server to wait on all locks. + */ + if (close_cached_tables(thd, true, NULL, false)) + goto end_with_read_lock; + + if (result= make_global_read_lock_block_commit(thd)) + goto end_with_read_lock; + + /* Change the opt_readonly system variable, safe because the lock is held */ + result= sys_var_bool_ptr::update(thd, var); + +end_with_read_lock: + /* Release the lock */ + unlock_global_read_lock(thd); + DBUG_RETURN(result); +} + + /* even session variable here requires SUPER, because of -#o,file */ bool sys_var_thd_dbug::check(THD *thd, set_var *var) { diff --git a/sql/set_var.h b/sql/set_var.h index 3b0aa6cde9c..f957ec931d6 100644 --- a/sql/set_var.h +++ b/sql/set_var.h @@ -904,6 +904,20 @@ public: }; +/** + Handler for setting the system variable --read-only. +*/ + +class sys_var_opt_readonly :public sys_var_bool_ptr +{ +public: + sys_var_opt_readonly(const char *name_arg, my_bool *value_arg) : + sys_var_bool_ptr(name_arg, value_arg) {}; + ~sys_var_opt_readonly() {}; + bool update(THD *thd, set_var *var); +}; + + class sys_var_thd_lc_time_names :public sys_var_thd { public: diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 714202b0864..2b65e2b345f 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -628,27 +628,6 @@ sp_head::create(THD *thd) DBUG_PRINT("info", ("type: %d name: %s params: %s body: %s", m_type, m_name.str, m_params.str, m_body.str)); -#ifndef DBUG_OFF - optimize(); - { - String s; - sp_instr *i; - uint ip= 0; - while ((i = get_instr(ip))) - { - char buf[8]; - - sprintf(buf, "%4u: ", ip); - s.append(buf); - i->print(&s); - s.append('\n'); - ip+= 1; - } - s.append('\0'); - DBUG_PRINT("info", ("Code %s\n%s", m_qname.str, s.ptr())); - } -#endif - if (m_type == TYPE_ENUM_FUNCTION) ret= sp_create_function(thd, this); else @@ -2229,7 +2208,7 @@ sp_head::show_create_function(THD *thd) This is the main mark and move loop; it relies on the following methods in sp_instr and its subclasses: - opt_mark() Mark instruction as reachable (will recurse for jumps) + opt_mark() Mark instruction as reachable opt_shortcut_jump() Shortcut jumps to the final destination; used by opt_mark(). opt_move() Update moved instruction @@ -2242,7 +2221,7 @@ void sp_head::optimize() sp_instr *i; uint src, dst; - opt_mark(0); + opt_mark(); bp.empty(); src= dst= 0; @@ -2276,13 +2255,50 @@ void sp_head::optimize() bp.empty(); } +void sp_head::add_mark_lead(uint ip, List<sp_instr> *leads) +{ + sp_instr *i= get_instr(ip); + + if (i && ! i->marked) + leads->push_front(i); +} + void -sp_head::opt_mark(uint ip) +sp_head::opt_mark() { + uint ip; sp_instr *i; + List<sp_instr> leads; - while ((i= get_instr(ip)) && !i->marked) - ip= i->opt_mark(this); + /* + Forward flow analysis algorithm in the instruction graph: + - first, add the entry point in the graph (the first instruction) to the + 'leads' list of paths to explore. + - while there are still leads to explore: + - pick one lead, and follow the path forward. Mark instruction reached. + Stop only if the end of the routine is reached, or the path converge + to code already explored (marked). + - while following a path, collect in the 'leads' list any fork to + another path (caused by conditional jumps instructions), so that these + paths can be explored as well. + */ + + /* Add the entry point */ + i= get_instr(0); + leads.push_front(i); + + /* For each path of code ... */ + while (leads.elements != 0) + { + i= leads.pop(); + + /* Mark the entire path, collecting new leads. */ + while (i && ! i->marked) + { + ip= i->opt_mark(this, & leads); + i= get_instr(ip); + } + } } @@ -2684,7 +2700,7 @@ sp_instr_jump::print(String *str) } uint -sp_instr_jump::opt_mark(sp_head *sp) +sp_instr_jump::opt_mark(sp_head *sp, List<sp_instr> *leads) { m_dest= opt_shortcut_jump(sp, this); if (m_dest != m_ip+1) /* Jumping to following instruction? */ @@ -2778,7 +2794,7 @@ sp_instr_jump_if_not::print(String *str) uint -sp_instr_jump_if_not::opt_mark(sp_head *sp) +sp_instr_jump_if_not::opt_mark(sp_head *sp, List<sp_instr> *leads) { sp_instr *i; @@ -2788,13 +2804,13 @@ sp_instr_jump_if_not::opt_mark(sp_head *sp) m_dest= i->opt_shortcut_jump(sp, this); m_optdest= sp->get_instr(m_dest); } - sp->opt_mark(m_dest); + sp->add_mark_lead(m_dest, leads); if ((i= sp->get_instr(m_cont_dest))) { m_cont_dest= i->opt_shortcut_jump(sp, this); m_cont_optdest= sp->get_instr(m_cont_dest); } - sp->opt_mark(m_cont_dest); + sp->add_mark_lead(m_cont_dest, leads); return m_ip+1; } @@ -2915,7 +2931,7 @@ sp_instr_hpush_jump::print(String *str) uint -sp_instr_hpush_jump::opt_mark(sp_head *sp) +sp_instr_hpush_jump::opt_mark(sp_head *sp, List<sp_instr> *leads) { sp_instr *i; @@ -2925,7 +2941,7 @@ sp_instr_hpush_jump::opt_mark(sp_head *sp) m_dest= i->opt_shortcut_jump(sp, this); m_optdest= sp->get_instr(m_dest); } - sp->opt_mark(m_dest); + sp->add_mark_lead(m_dest, leads); return m_ip+1; } @@ -2990,15 +3006,13 @@ sp_instr_hreturn::print(String *str) uint -sp_instr_hreturn::opt_mark(sp_head *sp) +sp_instr_hreturn::opt_mark(sp_head *sp, List<sp_instr> *leads) { if (m_dest) - return sp_instr_jump::opt_mark(sp); - else - { - marked= 1; - return UINT_MAX; - } + return sp_instr_jump::opt_mark(sp, leads); + + marked= 1; + return UINT_MAX; } @@ -3341,7 +3355,7 @@ sp_instr_set_case_expr::print(String *str) } uint -sp_instr_set_case_expr::opt_mark(sp_head *sp) +sp_instr_set_case_expr::opt_mark(sp_head *sp, List<sp_instr> *leads) { sp_instr *i; @@ -3351,7 +3365,7 @@ sp_instr_set_case_expr::opt_mark(sp_head *sp) m_cont_dest= i->opt_shortcut_jump(sp, this); m_cont_optdest= sp->get_instr(m_cont_dest); } - sp->opt_mark(m_cont_dest); + sp->add_mark_lead(m_cont_dest, leads); return m_ip+1; } diff --git a/sql/sp_head.h b/sql/sp_head.h index 2c554d50bd8..0085608ae40 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -303,8 +303,19 @@ public: void restore_thd_mem_root(THD *thd); + /** + Optimize the code. + */ void optimize(); - void opt_mark(uint ip); + + /** + Helper used during flow analysis during code optimization. + See the implementation of <code>opt_mark()</code>. + @param ip the instruction to add to the leads list + @param leads the list of remaining paths to explore in the graph that + represents the code, during flow analysis. + */ + void add_mark_lead(uint ip, List<sp_instr> *leads); void recursion_level_error(THD *thd); @@ -413,6 +424,12 @@ private: bool execute(THD *thd); + /** + Perform a forward flow analysis in the generated code. + Mark reachable instructions, for the optimizer. + */ + void opt_mark(); + /* Merge the list of tables used by query into the multi-set of tables used by routine. @@ -480,10 +497,10 @@ public: /* Mark this instruction as reachable during optimization and return the - index to the next instruction. Jump instruction will mark their - destination too recursively. + index to the next instruction. Jump instruction will add their + destination to the leads list. */ - virtual uint opt_mark(sp_head *sp) + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads) { marked= 1; return m_ip+1; @@ -755,7 +772,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp); + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads); virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start); @@ -805,7 +822,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp); + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads); /* Override sp_instr_jump's shortcut; we stop here */ virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start) @@ -851,7 +868,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp) + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads) { marked= 1; return UINT_MAX; @@ -888,7 +905,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp); + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads); /* Override sp_instr_jump's shortcut; we stop here. */ virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start) @@ -953,7 +970,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp); + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads); private: @@ -1123,7 +1140,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp) + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads) { marked= 1; return UINT_MAX; @@ -1156,7 +1173,7 @@ public: virtual void print(String *str); - virtual uint opt_mark(sp_head *sp); + virtual uint opt_mark(sp_head *sp, List<sp_instr> *leads); virtual void opt_move(uint dst, List<sp_instr> *ibp); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index af66fd2d3de..9e3c2442f57 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -984,6 +984,13 @@ void select_result::cleanup() /* do nothing */ } +bool select_result::check_simple_select() const +{ + my_error(ER_SP_BAD_CURSOR_QUERY, MYF(0)); + return TRUE; +} + + static String default_line_term("\n",default_charset_info); static String default_escaped("\\",default_charset_info); static String default_field_term("\t",default_charset_info); @@ -1624,6 +1631,13 @@ int select_dumpvar::prepare(List<Item> &list, SELECT_LEX_UNIT *u) } +bool select_dumpvar::check_simple_select() const +{ + my_error(ER_SP_BAD_CURSOR_SELECT, MYF(0)); + return TRUE; +} + + void select_dumpvar::cleanup() { row_count= 0; diff --git a/sql/sql_class.h b/sql/sql_class.h index 94535d6f57b..8c86a66392a 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1702,7 +1702,14 @@ public: virtual bool initialize_tables (JOIN *join=0) { return 0; } virtual void send_error(uint errcode,const char *err); virtual bool send_eof()=0; - virtual bool simple_select() { return 0; } + /** + Check if this query returns a result set and therefore is allowed in + cursors and set an error message if it is not the case. + + @retval FALSE success + @retval TRUE error, an error message is set + */ + virtual bool check_simple_select() const; virtual void abort() {} /* Cleanup instance of this class for next execution of a prepared @@ -1740,7 +1747,7 @@ public: bool send_fields(List<Item> &list, uint flags); bool send_data(List<Item> &items); bool send_eof(); - bool simple_select() { return 1; } + virtual bool check_simple_select() const { return FALSE; } void abort(); }; @@ -2186,6 +2193,7 @@ public: int prepare(List<Item> &list, SELECT_LEX_UNIT *u); bool send_data(List<Item> &items); bool send_eof(); + virtual bool check_simple_select() const; void cleanup(); }; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 18d30494701..1c399503498 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -1193,7 +1193,6 @@ void st_select_lex::init_select() options= 0; sql_cache= SQL_CACHE_UNSPECIFIED; braces= 0; - when_list.empty(); expr_list.empty(); interval_list.empty(); use_index.empty(); diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 12e41d12899..fd76ec7ac51 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -552,7 +552,6 @@ public: SQL_LIST order_list; /* ORDER clause */ List<List_item> expr_list; - List<List_item> when_list; /* WHEN clause (expression) */ SQL_LIST *gorder_list; Item *select_limit, *offset_limit; /* LIMIT clause parameters */ // Arrays of pointers to top elements of all_fields list diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 2a162216add..d95c434390c 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2945,10 +2945,9 @@ bool Prepared_statement::execute(String *expanded_query, bool open_cursor) in INSERT ... SELECT and similar commands. */ - if (open_cursor && lex->result && !lex->result->simple_select()) + if (open_cursor && lex->result && lex->result->check_simple_select()) { DBUG_PRINT("info",("Cursor asked for not SELECT stmt")); - my_error(ER_SP_BAD_CURSOR_QUERY, MYF(0)); return TRUE; } diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 4d8eda06faf..6938a7b10d1 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -107,6 +107,187 @@ static bool is_native_function(THD *thd, const LEX_STRING *name) return false; } + +/** + Helper action for a case statement (entering the CASE). + This helper is used for both 'simple' and 'searched' cases. + This helper, with the other case_stmt_action_..., is executed when + the following SQL code is parsed: +<pre> +CREATE PROCEDURE proc_19194_simple(i int) +BEGIN + DECLARE str CHAR(10); + + CASE i + WHEN 1 THEN SET str="1"; + WHEN 2 THEN SET str="2"; + WHEN 3 THEN SET str="3"; + ELSE SET str="unknown"; + END CASE; + + SELECT str; +END +</pre> + The actions are used to generate the following code: +<pre> +SHOW PROCEDURE CODE proc_19194_simple; +Pos Instruction +0 set str@1 NULL +1 set_case_expr (12) 0 i@0 +2 jump_if_not 5(12) (case_expr@0 = 1) +3 set str@1 _latin1'1' +4 jump 12 +5 jump_if_not 8(12) (case_expr@0 = 2) +6 set str@1 _latin1'2' +7 jump 12 +8 jump_if_not 11(12) (case_expr@0 = 3) +9 set str@1 _latin1'3' +10 jump 12 +11 set str@1 _latin1'unknown' +12 stmt 0 "SELECT str" +</pre> + + @param lex the parser lex context +*/ + +void case_stmt_action_case(LEX *lex) +{ + lex->sphead->new_cont_backpatch(NULL); + + /* + BACKPATCH: Creating target label for the jump to + "case_stmt_action_end_case" + (Instruction 12 in the example) + */ + + lex->spcont->push_label((char *)"", lex->sphead->instructions()); +} + +/** + Helper action for a case expression statement (the expr in 'CASE expr'). + This helper is used for 'searched' cases only. + @param lex the parser lex context + @param expr the parsed expression + @return 0 on success +*/ + +int case_stmt_action_expr(LEX *lex, Item* expr) +{ + sp_head *sp= lex->sphead; + sp_pcontext *parsing_ctx= lex->spcont; + int case_expr_id= parsing_ctx->register_case_expr(); + sp_instr_set_case_expr *i; + + if (parsing_ctx->push_case_expr_id(case_expr_id)) + return 1; + + i= new sp_instr_set_case_expr(sp->instructions(), + parsing_ctx, case_expr_id, expr, lex); + + sp->add_cont_backpatch(i); + sp->add_instr(i); + + return 0; +} + +/** + Helper action for a case when condition. + This helper is used for both 'simple' and 'searched' cases. + @param lex the parser lex context + @param when the parsed expression for the WHEN clause + @param simple true for simple cases, false for searched cases +*/ + +void case_stmt_action_when(LEX *lex, Item *when, bool simple) +{ + sp_head *sp= lex->sphead; + sp_pcontext *ctx= lex->spcont; + uint ip= sp->instructions(); + sp_instr_jump_if_not *i; + Item_case_expr *var; + Item *expr; + + if (simple) + { + var= new Item_case_expr(ctx->get_current_case_expr_id()); + +#ifndef DBUG_OFF + if (var) + { + var->m_sp= sp; + } +#endif + + expr= new Item_func_eq(var, when); + i= new sp_instr_jump_if_not(ip, ctx, expr, lex); + } + else + i= new sp_instr_jump_if_not(ip, ctx, when, lex); + + /* + BACKPATCH: Registering forward jump from + "case_stmt_action_when" to "case_stmt_action_then" + (jump_if_not from instruction 2 to 5, 5 to 8 ... in the example) + */ + + sp->push_backpatch(i, ctx->push_label((char *)"", 0)); + sp->add_cont_backpatch(i); + sp->add_instr(i); +} + +/** + Helper action for a case then statements. + This helper is used for both 'simple' and 'searched' cases. + @param lex the parser lex context +*/ + +void case_stmt_action_then(LEX *lex) +{ + sp_head *sp= lex->sphead; + sp_pcontext *ctx= lex->spcont; + uint ip= sp->instructions(); + sp_instr_jump *i = new sp_instr_jump(ip, ctx); + sp->add_instr(i); + + /* + BACKPATCH: Resolving forward jump from + "case_stmt_action_when" to "case_stmt_action_then" + (jump_if_not from instruction 2 to 5, 5 to 8 ... in the example) + */ + + sp->backpatch(ctx->pop_label()); + + /* + BACKPATCH: Registering forward jump from + "case_stmt_action_then" to "case_stmt_action_end_case" + (jump from instruction 4 to 12, 7 to 12 ... in the example) + */ + + sp->push_backpatch(i, ctx->last_label()); +} + +/** + Helper action for an end case. + This helper is used for both 'simple' and 'searched' cases. + @param lex the parser lex context + @param simple true for simple cases, false for searched cases +*/ + +void case_stmt_action_end_case(LEX *lex, bool simple) +{ + /* + BACKPATCH: Resolving forward jump from + "case_stmt_action_then" to "case_stmt_action_end_case" + (jump from instruction 4 to 12, 7 to 12 ... in the example) + */ + lex->sphead->backpatch(lex->spcont->pop_label()); + + if (simple) + lex->spcont->pop_case_expr_id(); + + lex->sphead->do_cont_backpatch(); +} + %} %union { int num; @@ -886,7 +1067,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); select_item_list select_item values_list no_braces opt_limit_clause delete_limit_clause fields opt_values values procedure_list procedure_list2 procedure_item - when_list2 expr_list2 udf_expr_list3 handler + expr_list2 udf_expr_list3 handler opt_precision opt_ignore opt_column opt_restrict grant revoke set lock unlock string_list field_options field_option field_opt_list opt_binary table_lock_list table_lock @@ -922,10 +1103,11 @@ END_OF_INPUT %type <NONE> call sp_proc_stmts sp_proc_stmts1 sp_proc_stmt %type <NONE> sp_proc_stmt_statement sp_proc_stmt_return -%type <NONE> sp_proc_stmt_if sp_proc_stmt_case_simple sp_proc_stmt_case +%type <NONE> sp_proc_stmt_if %type <NONE> sp_labeled_control sp_proc_stmt_unlabeled sp_proc_stmt_leave %type <NONE> sp_proc_stmt_iterate %type <NONE> sp_proc_stmt_open sp_proc_stmt_fetch sp_proc_stmt_close +%type <NONE> case_stmt_specification simple_case_stmt searched_case_stmt %type <num> sp_decl_idents sp_opt_inout sp_handler_type sp_hcond_list %type <spcondtype> sp_cond sp_hcond @@ -1546,8 +1728,7 @@ ev_sql_stmt_inner: sp_proc_stmt_statement | sp_proc_stmt_return | sp_proc_stmt_if - | sp_proc_stmt_case_simple - | sp_proc_stmt_case + | case_stmt_specification | sp_labeled_control | sp_proc_stmt_unlabeled | sp_proc_stmt_leave @@ -2311,8 +2492,7 @@ sp_proc_stmt: sp_proc_stmt_statement | sp_proc_stmt_return | sp_proc_stmt_if - | sp_proc_stmt_case_simple - | sp_proc_stmt_case + | case_stmt_specification | sp_labeled_control | sp_proc_stmt_unlabeled | sp_proc_stmt_leave @@ -2402,49 +2582,6 @@ sp_proc_stmt_return: } ; -sp_proc_stmt_case_simple: - CASE_SYM WHEN_SYM - { - Lex->sphead->m_flags&= ~sp_head::IN_SIMPLE_CASE; - Lex->sphead->new_cont_backpatch(NULL); - } - sp_case END CASE_SYM { Lex->sphead->do_cont_backpatch(); } - ; - -sp_proc_stmt_case: - CASE_SYM - { - Lex->sphead->reset_lex(YYTHD); - Lex->sphead->new_cont_backpatch(NULL); - } - expr WHEN_SYM - { - LEX *lex= Lex; - sp_head *sp= lex->sphead; - sp_pcontext *parsing_ctx= lex->spcont; - int case_expr_id= parsing_ctx->register_case_expr(); - sp_instr_set_case_expr *i; - - if (parsing_ctx->push_case_expr_id(case_expr_id)) - YYABORT; - - i= new sp_instr_set_case_expr(sp->instructions(), - parsing_ctx, - case_expr_id, - $3, - lex); - sp->add_cont_backpatch(i); - sp->add_instr(i); - sp->m_flags|= sp_head::IN_SIMPLE_CASE; - sp->restore_lex(YYTHD); - } - sp_case END CASE_SYM - { - Lex->spcont->pop_case_expr_id(); - Lex->sphead->do_cont_backpatch(); - } - ; - sp_proc_stmt_unlabeled: { /* Unlabeled controls get a secret label. */ LEX *lex= Lex; @@ -2669,72 +2806,114 @@ sp_elseifs: | ELSE sp_proc_stmts1 ; -sp_case: - { Lex->sphead->reset_lex(YYTHD); } - expr THEN_SYM - { - LEX *lex= Lex; - sp_head *sp= lex->sphead; - sp_pcontext *ctx= Lex->spcont; - uint ip= sp->instructions(); - sp_instr_jump_if_not *i; - - if (! (sp->m_flags & sp_head::IN_SIMPLE_CASE)) - i= new sp_instr_jump_if_not(ip, ctx, $2, lex); - else - { /* Simple case: <caseval> = <whenval> */ +case_stmt_specification: + simple_case_stmt + | searched_case_stmt + ; - Item_case_expr *var; - Item *expr; +simple_case_stmt: + CASE_SYM + { + LEX *lex= Lex; + case_stmt_action_case(lex); + lex->sphead->reset_lex(YYTHD); /* For expr $3 */ + } + expr + { + LEX *lex= Lex; + if (case_stmt_action_expr(lex, $3)) + YYABORT; - var= new Item_case_expr(ctx->get_current_case_expr_id()); + lex->sphead->restore_lex(YYTHD); /* For expr $3 */ + } + simple_when_clause_list + else_clause_opt + END + CASE_SYM + { + LEX *lex= Lex; + case_stmt_action_end_case(lex, true); + } + ; -#ifndef DBUG_OFF - if (var) - var->m_sp= sp; -#endif +searched_case_stmt: + CASE_SYM + { + LEX *lex= Lex; + case_stmt_action_case(lex); + } + searched_when_clause_list + else_clause_opt + END + CASE_SYM + { + LEX *lex= Lex; + case_stmt_action_end_case(lex, false); + } + ; - expr= new Item_func_eq(var, $2); +simple_when_clause_list: + simple_when_clause + | simple_when_clause_list simple_when_clause + ; - i= new sp_instr_jump_if_not(ip, ctx, expr, lex); - } - sp->push_backpatch(i, ctx->push_label((char *)"", 0)); - sp->add_cont_backpatch(i); - sp->add_instr(i); - sp->restore_lex(YYTHD); - } - sp_proc_stmts1 - { - sp_head *sp= Lex->sphead; - sp_pcontext *ctx= Lex->spcont; - uint ip= sp->instructions(); - sp_instr_jump *i = new sp_instr_jump(ip, ctx); +searched_when_clause_list: + searched_when_clause + | searched_when_clause_list searched_when_clause + ; - sp->add_instr(i); - sp->backpatch(ctx->pop_label()); - sp->push_backpatch(i, ctx->push_label((char *)"", 0)); - } - sp_whens - { - LEX *lex= Lex; +simple_when_clause: + WHEN_SYM + { + Lex->sphead->reset_lex(YYTHD); /* For expr $3 */ + } + expr + { + /* Simple case: <caseval> = <whenval> */ - lex->sphead->backpatch(lex->spcont->pop_label()); - } - ; + LEX *lex= Lex; + case_stmt_action_when(lex, $3, true); + lex->sphead->restore_lex(YYTHD); /* For expr $3 */ + } + THEN_SYM + sp_proc_stmts1 + { + LEX *lex= Lex; + case_stmt_action_then(lex); + } + ; -sp_whens: - /* Empty */ - { - sp_head *sp= Lex->sphead; - uint ip= sp->instructions(); - sp_instr_error *i= new sp_instr_error(ip, Lex->spcont, - ER_SP_CASE_NOT_FOUND); +searched_when_clause: + WHEN_SYM + { + Lex->sphead->reset_lex(YYTHD); /* For expr $3 */ + } + expr + { + LEX *lex= Lex; + case_stmt_action_when(lex, $3, false); + lex->sphead->restore_lex(YYTHD); /* For expr $3 */ + } + THEN_SYM + sp_proc_stmts1 + { + LEX *lex= Lex; + case_stmt_action_then(lex); + } + ; - sp->add_instr(i); - } - | ELSE sp_proc_stmts1 {} - | WHEN_SYM sp_case {} - ; +else_clause_opt: + /* empty */ + { + LEX *lex= Lex; + sp_head *sp= lex->sphead; + uint ip= sp->instructions(); + sp_instr_error *i= new sp_instr_error(ip, lex->spcont, + ER_SP_CASE_NOT_FOUND); + sp->add_instr(i); + } + | ELSE sp_proc_stmts1 + ; sp_labeled_control: label_ident ':' @@ -4922,7 +5101,6 @@ opt_ev_sql_stmt: /* empty*/ { $$= 0;} | DO_SYM ev_sql_stmt { $$= 1; } ; - ident_or_empty: /* empty */ { $$.str= 0; $$.length= 0; } | ident { $$= $1; }; @@ -6130,8 +6308,8 @@ simple_expr: if (!$$) YYABORT; } - | CASE_SYM opt_expr WHEN_SYM when_list opt_else END - { $$= new (YYTHD->mem_root) Item_func_case(* $4, $2, $5 ); } + | CASE_SYM opt_expr when_list opt_else END + { $$= new (YYTHD->mem_root) Item_func_case(* $3, $2, $4 ); } | CONVERT_SYM '(' expr ',' cast_type ')' { $$= create_func_cast(YYTHD, $3, $5, @@ -6349,7 +6527,7 @@ function_call_nonkeyword: ; /* - Functions calls using a non reserved keywork, and using a regular syntax. + Functions calls using a non reserved keyword, and using a regular syntax. Because the non reserved keyword is used in another part of the grammar, a dedicated rule is needed here. */ @@ -6564,7 +6742,7 @@ function_call_generic: parser and the implementation in item_create.cc clean, since this will change with WL#2128 (SQL PATH): - INFORMATION_SCHEMA.version() is the SQL 99 syntax for the native - funtion version(), + function version(), - MySQL.version() is the SQL 2003 syntax for the native function version() (a vendor can specify any schema). */ @@ -6666,7 +6844,7 @@ sum_expr: { $$=new Item_sum_min($3); } /* According to ANSI SQL, DISTINCT is allowed and has - no sence inside MIN and MAX grouping functions; so MIN|MAX(DISTINCT ...) + no sense inside MIN and MAX grouping functions; so MIN|MAX(DISTINCT ...) is processed like an ordinary MIN | MAX() */ | MIN_SYM '(' DISTINCT in_sum_expr ')' @@ -6830,23 +7008,19 @@ opt_else: | ELSE expr { $$= $2; }; when_list: - { Select->when_list.push_front(new List<Item>); } - when_list2 - { $$= Select->when_list.pop(); }; - -when_list2: - expr THEN_SYM expr - { - SELECT_LEX *sel=Select; - sel->when_list.head()->push_back($1); - sel->when_list.head()->push_back($3); - } - | when_list2 WHEN_SYM expr THEN_SYM expr - { - SELECT_LEX *sel=Select; - sel->when_list.head()->push_back($3); - sel->when_list.head()->push_back($5); - }; + WHEN_SYM expr THEN_SYM expr + { + $$= new List<Item>; + $$->push_back($2); + $$->push_back($4); + } + | when_list WHEN_SYM expr THEN_SYM expr + { + $1->push_back($3); + $1->push_back($5); + $$= $1; + } + ; /* Warning - may return NULL in case of incomplete SELECT */ table_ref: diff --git a/storage/csv/ha_tina.cc b/storage/csv/ha_tina.cc index b957bcef3ea..5211dda9055 100644 --- a/storage/csv/ha_tina.cc +++ b/storage/csv/ha_tina.cc @@ -516,14 +516,16 @@ int ha_tina::encode_quote(byte *buf) const char *end_ptr; /* - Write an empty string to the buffer in case of a NULL value. + CSV does not support nulls. Write quoted 0 to the buffer. In fact, + (*field)->val_str(&attribute,&attribute) would usually return 0 + in this case but we write it explicitly here. Basically this is a safety check, as no one ensures that the field content is cleaned up every time we use Field::set_null() in the code. */ if ((*field)->is_null()) { - buffer.append(STRING_WITH_LEN("\"\",")); + buffer.append(STRING_WITH_LEN("\"0\",")); continue; } else diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index a00b5414101..e239400873f 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -15683,6 +15683,33 @@ static void test_bug21635() DBUG_VOID_RETURN; } +/* + Bug#24179 "select b into $var" fails with --cursor_protocol" + The failure is correct, check that the returned message is meaningful. +*/ + +static void test_bug24179() +{ + int rc; + MYSQL_STMT *stmt; + + DBUG_ENTER("test_bug24179"); + myheader("test_bug24179"); + + stmt= open_cursor("select 1 into @a"); + rc= mysql_stmt_execute(stmt); + DIE_UNLESS(rc); + if (!opt_silent) + { + printf("Got error (as expected): %d %s\n", + mysql_stmt_errno(stmt), + mysql_stmt_error(stmt)); + } + DIE_UNLESS(mysql_stmt_errno(stmt) == 1323); + + DBUG_VOID_RETURN; +} + /* Read and parse arguments and MySQL options from my.cnf @@ -15965,6 +15992,7 @@ static struct my_tests_st my_tests[]= { { "test_bug23383", test_bug23383 }, { "test_bug21635", test_bug21635 }, { "test_status", test_status}, + { "test_bug24179", test_bug24179 }, { 0, 0 } }; |