Merge lp:~tsarev/percona-server/test55_691404 into lp:percona-server/5.5

[Oleg Tsarev]

Fix bug #691404 - add log-slow-admin-statements to GLOBAL_VARIABLES
(dev-info: remove command-line option from sql/mysqld.cc, add sys_var_... to sql/sys_vars.cc (affects to CLI too))

> 1) added to SHOW VARIABLES
> 2) can be set online (global variable is enough)

a) Added to system variables.
b) Wrote tests to online variable setup

> 3) ensure it logs ALL statements.
I reviewed code in sql/sql_parse.cc, sql/log.cc - all administraive commands go to slow query log sucessfully.

Clean Jenkins results: http://jenkins.percona.com/view/Percona%20Server%205.5/job/percona-server-5.5-param/137/

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.5/job/percona-server-5.5-param/106/

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.5/job/percona-server-5.5-param/111/

[Laurynas Biveinis (laurynas-biveinis)]

Please move percona_log_slow_admin_statements.test to sys_vars suite with a name log_slow_admin_statements_basic.test. This way all_vars test will not consider this variable unhandled.

review: Needs Fixing

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.5/job/percona-server-5.5-param/119

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.5/job/percona-server-5.5-param/122/
Clean jenkins results

[Laurynas Biveinis (laurynas-biveinis)]

The test needs renaming as in my previous comment

review: Needs Fixing

[Alexey Kopytov (akopytov)]

1. What's the point to declare log-slow-admin-statements as OPT_ARG? What was wrong with NO_ARG?
2. What's the point in commenting out command line options in mysqld.cc instead of removing them? For this specific MP, it is sufficient to only remove log-slow-admin-statetements. There's no need to remove all of commented ones in this MP.

review: Needs Fixing

[Oleg Tsarev (tsarev)]

Similar to bug #712387
With NO_ARG syntax --variable_name=ON not work

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.5/job/percona-server-5.5-param/122/
Clean jenkins results

[Alexey Kopytov (akopytov)]

(1) does not actually apply to 5.5 because option handling in 5.5 is different.

But (2) still holds. I don't see any point whatsoever in commenting some code instead of removing it. We are abusing the concept of a version control system already. Let's not abuse the concept of a patch at least. Thanks.

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

My comments have been addressed, please achieve consensus with Alexey for what else is required to do.

review: Abstain

[Alexey Kopytov (akopytov)]

Let's read bug #691404:

"
Ensure --log-slow-admin-statements is:

1) added to SHOW VARIABLES
2) can be set online (global variable is enough)
3) ensure it logs ALL statements.
"

But the revision comment only mentions "add log-slow-admin-statements to GLOBAL_VARIABLES", i.e. 1). And 1) is the only condition covered by tests. What about testing 2) and 3)?

review: Needs Fixing

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'patches/mysql-test.diff'
--- patches/mysql-test.diff	2011-09-25 07:11:59 +0000
+++ patches/mysql-test.diff	2011-09-29 06:09:30 +0000
@@ -1489,6 +1489,7 @@
 +LOG_OUTPUT
 +LOG_QUERIES_NOT_USING_INDEXES
 +LOG_SLAVE_UPDATES
++LOG_SLOW_ADMIN_STATEMENTS
 +LOG_SLOW_FILTER
 +LOG_SLOW_QUERIES
 +LOG_SLOW_RATE_LIMIT
@@ -1863,6 +1864,7 @@
 +LOG_OUTPUT
 +LOG_QUERIES_NOT_USING_INDEXES
 +LOG_SLAVE_UPDATES
++LOG_SLOW_ADMIN_STATEMENTS
 +LOG_SLOW_FILTER
 +LOG_SLOW_QUERIES
 +LOG_SLOW_RATE_LIMIT

=== modified file 'patches/slow_extended.patch'
--- patches/slow_extended.patch	2011-09-21 16:16:06 +0000
+++ patches/slow_extended.patch	2011-09-29 06:09:30 +0000
@@ -424,10 +424,14 @@
  my_bool lower_case_file_system= 0;
  my_bool opt_large_pages= 0;
  my_bool opt_super_large_pages= 0;
-@@ -5896,10 +5900,10 @@
-    "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements to "
-    "the slow log if it is open.", &opt_log_slow_admin_statements,
-    &opt_log_slow_admin_statements, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+@@ -5892,14 +5896,10 @@
+    "Don't log extra information to update and slow-query logs.",
+    &opt_short_log_format, &opt_short_log_format,
+    0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+-  {"log-slow-admin-statements", 0,
+-   "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements to "
+-   "the slow log if it is open.", &opt_log_slow_admin_statements,
+-   &opt_log_slow_admin_statements, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
 - {"log-slow-slave-statements", 0,
 + /*{"log-slow-slave-statements", 0,
    "Log slow statements executed by slave thread to the slow log if it is open.",
@@ -437,7 +441,7 @@
    {"log-slow-queries", OPT_SLOW_QUERY_LOG,
     "Log slow queries to a table or log file. Defaults logging to table "
     "mysql.slow_log or hostname-slow.log if --log-output=file is used. "
-@@ -7288,6 +7292,10 @@
+@@ -7288,6 +7288,10 @@
  
  C_MODE_END
  
@@ -448,7 +452,7 @@
  /**
    Get server options from the command line,
    and perform related server initializations.
-@@ -7437,6 +7445,8 @@
+@@ -7437,6 +7441,8 @@
    global_system_variables.long_query_time= (ulonglong)
      (global_system_variables.long_query_time_double * 1e6);
  
@@ -979,7 +983,7 @@
  static bool fix_low_prio_updates(sys_var *self, THD *thd, enum_var_type type)
  {
    if (type == OPT_SESSION)
-@@ -2898,6 +2921,117 @@
+@@ -2898,6 +2921,123 @@
         DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
         ON_UPDATE(fix_log_state));
  
@@ -1054,6 +1058,12 @@
 +       "Log queries replayed be the slave SQL thread",
 +       GLOBAL_VAR(opt_log_slow_slave_statements), CMD_LINE(OPT_ARG),
 +       DEFAULT(FALSE));
++static Sys_var_mybool Sys_log_slow_admin_statements(
++       "log_slow_admin_statements",
++       "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements"
++       " to the slow log if it is open.",
++       GLOBAL_VAR(opt_log_slow_admin_statements), CMD_LINE(OPT_ARG),
++       DEFAULT(FALSE));
 +static Sys_var_mybool Sys_log_slow_sp_statements(
 +       "log_slow_sp_statements",
 +       "Log slow statements executed by stored procedure to the slow log if it is open.",
@@ -2415,3 +2425,152 @@
 +--let grep_pattern = Last_errno: 1050
 +--source include/log_grep.inc
 +DROP TABLE t;
+--- /dev/null
++++ b/mysql-test/suite/sys_vars/t/log_slow_admin_statements_basic.test
+@@ -0,0 +1 @@
++SELECT @@global.log_slow_admin_statements;
+--- /dev/null
++++ b/mysql-test/suite/sys_vars/r/log_slow_admin_statements_basic.result
+@@ -0,0 +1,3 @@
++SELECT @@global.log_slow_admin_statements;
++@@global.log_slow_admin_statements
++0
+--- /dev/null
++++ b/mysql-test/r/percona_log_slow_admin_statements.result
+@@ -0,0 +1,35 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	OFF
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	OFF
++SET GLOBAL log_slow_admin_statements=true;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	ON
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	ON
++SET GLOBAL log_slow_admin_statements=false;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	OFF
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	OFF
++SET GLOBAL log_slow_admin_statements=foo;
++ERROR 42000: Variable 'log_slow_admin_statements' can't be set to the value of 'foo'
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	OFF
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	OFF
++SET GLOBAL log_slow_admin_statements=default;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	OFF
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	OFF
+--- /dev/null
++++ b/mysql-test/r/percona_log_slow_admin_statements-config_false.result
+@@ -0,0 +1,6 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	OFF
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	OFF
+--- /dev/null
++++ b/mysql-test/r/percona_log_slow_admin_statements-config_foo.result
+@@ -0,0 +1,7 @@
++call mtr.add_suppression("option 'log_slow_admin_statements': boolean value 'foo' wasn't recognized. Set to OFF.");
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	OFF
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	OFF
+--- /dev/null
++++ b/mysql-test/r/percona_log_slow_admin_statements-config_true.result
+@@ -0,0 +1,6 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	ON
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	ON
+--- /dev/null
++++ b/mysql-test/r/percona_log_slow_admin_statements-config.result
+@@ -0,0 +1,6 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++Variable_name	Value
++log_slow_admin_statements	ON
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++VARIABLE_NAME	VARIABLE_VALUE
++LOG_SLOW_ADMIN_STATEMENTS	ON
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config_false.cnf
+@@ -0,0 +1,2 @@
++[mysqld.1]
++log-slow-admin-statements=false
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config_foo.cnf
+@@ -0,0 +1,2 @@
++[mysqld.1]
++log-slow-admin-statements=foo
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config_true.cnf
+@@ -0,0 +1,2 @@
++[mysqld.1]
++log-slow-admin-statements=true
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config.cnf
+@@ -0,0 +1,2 @@
++[mysqld.1]
++log-slow-admin-statements
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements.test
+@@ -0,0 +1,20 @@
++# default value
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++# set value to 'true'
++SET GLOBAL log_slow_admin_statements=true;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++# set value to 'false'
++SET GLOBAL log_slow_admin_statements=false;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++# set value to 'foo'
++--error ER_WRONG_VALUE_FOR_VAR
++SET GLOBAL log_slow_admin_statements=foo;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
++# set value to default
++SET GLOBAL log_slow_admin_statements=default;
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config_false.test
+@@ -0,0 +1,2 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config_foo.test
+@@ -0,0 +1,3 @@
++call mtr.add_suppression("option 'log_slow_admin_statements': boolean value 'foo' wasn't recognized. Set to OFF.");
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config_true.test
+@@ -0,0 +1,2 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
+--- /dev/null
++++ b/mysql-test/t/percona_log_slow_admin_statements-config.test
+@@ -0,0 +1,2 @@
++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';