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

Proposed by Oleg Tsarev
Status: Merged
Approved by: Alexey Kopytov
Approved revision: no longer in the source branch.
Merged at revision: 178
Proposed branch: lp:~tsarev/percona-server/test55_691404
Merge into: lp:percona-server/5.5
Diff against target: 235 lines (+168/-7)
2 files modified
patches/mysql-test.diff (+2/-0)
patches/slow_extended.patch (+166/-7)
To merge this branch: bzr merge lp:~tsarev/percona-server/test55_691404
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Laurynas Biveinis Pending
Review via email: mp+77455@code.launchpad.net

This proposal supersedes a proposal from 2011-09-27.

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal
Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal
Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal

The test needs renaming as in my previous comment

review: Needs Fixing
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

(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
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal

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

review: Abstain
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Alexey Kopytov (akopytov) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1489+LOG_OUTPUT1489+LOG_OUTPUT
1490+LOG_QUERIES_NOT_USING_INDEXES1490+LOG_QUERIES_NOT_USING_INDEXES
1491+LOG_SLAVE_UPDATES1491+LOG_SLAVE_UPDATES
1492+LOG_SLOW_ADMIN_STATEMENTS
1492+LOG_SLOW_FILTER1493+LOG_SLOW_FILTER
1493+LOG_SLOW_QUERIES1494+LOG_SLOW_QUERIES
1494+LOG_SLOW_RATE_LIMIT1495+LOG_SLOW_RATE_LIMIT
@@ -1863,6 +1864,7 @@
1863+LOG_OUTPUT1864+LOG_OUTPUT
1864+LOG_QUERIES_NOT_USING_INDEXES1865+LOG_QUERIES_NOT_USING_INDEXES
1865+LOG_SLAVE_UPDATES1866+LOG_SLAVE_UPDATES
1867+LOG_SLOW_ADMIN_STATEMENTS
1866+LOG_SLOW_FILTER1868+LOG_SLOW_FILTER
1867+LOG_SLOW_QUERIES1869+LOG_SLOW_QUERIES
1868+LOG_SLOW_RATE_LIMIT1870+LOG_SLOW_RATE_LIMIT
18691871
=== 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 @@
424 my_bool lower_case_file_system= 0;424 my_bool lower_case_file_system= 0;
425 my_bool opt_large_pages= 0;425 my_bool opt_large_pages= 0;
426 my_bool opt_super_large_pages= 0;426 my_bool opt_super_large_pages= 0;
427@@ -5896,10 +5900,10 @@427@@ -5892,14 +5896,10 @@
428 "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements to "428 "Don't log extra information to update and slow-query logs.",
429 "the slow log if it is open.", &opt_log_slow_admin_statements,429 &opt_short_log_format, &opt_short_log_format,
430 &opt_log_slow_admin_statements, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},430 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
431- {"log-slow-admin-statements", 0,
432- "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements to "
433- "the slow log if it is open.", &opt_log_slow_admin_statements,
434- &opt_log_slow_admin_statements, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
431- {"log-slow-slave-statements", 0,435- {"log-slow-slave-statements", 0,
432+ /*{"log-slow-slave-statements", 0,436+ /*{"log-slow-slave-statements", 0,
433 "Log slow statements executed by slave thread to the slow log if it is open.",437 "Log slow statements executed by slave thread to the slow log if it is open.",
@@ -437,7 +441,7 @@
437 {"log-slow-queries", OPT_SLOW_QUERY_LOG,441 {"log-slow-queries", OPT_SLOW_QUERY_LOG,
438 "Log slow queries to a table or log file. Defaults logging to table "442 "Log slow queries to a table or log file. Defaults logging to table "
439 "mysql.slow_log or hostname-slow.log if --log-output=file is used. "443 "mysql.slow_log or hostname-slow.log if --log-output=file is used. "
440@@ -7288,6 +7292,10 @@444@@ -7288,6 +7288,10 @@
441 445
442 C_MODE_END446 C_MODE_END
443 447
@@ -448,7 +452,7 @@
448 /**452 /**
449 Get server options from the command line,453 Get server options from the command line,
450 and perform related server initializations.454 and perform related server initializations.
451@@ -7437,6 +7445,8 @@455@@ -7437,6 +7441,8 @@
452 global_system_variables.long_query_time= (ulonglong)456 global_system_variables.long_query_time= (ulonglong)
453 (global_system_variables.long_query_time_double * 1e6);457 (global_system_variables.long_query_time_double * 1e6);
454 458
@@ -979,7 +983,7 @@
979 static bool fix_low_prio_updates(sys_var *self, THD *thd, enum_var_type type)983 static bool fix_low_prio_updates(sys_var *self, THD *thd, enum_var_type type)
980 {984 {
981 if (type == OPT_SESSION)985 if (type == OPT_SESSION)
982@@ -2898,6 +2921,117 @@986@@ -2898,6 +2921,123 @@
983 DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),987 DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
984 ON_UPDATE(fix_log_state));988 ON_UPDATE(fix_log_state));
985 989
@@ -1054,6 +1058,12 @@
1054+ "Log queries replayed be the slave SQL thread",1058+ "Log queries replayed be the slave SQL thread",
1055+ GLOBAL_VAR(opt_log_slow_slave_statements), CMD_LINE(OPT_ARG),1059+ GLOBAL_VAR(opt_log_slow_slave_statements), CMD_LINE(OPT_ARG),
1056+ DEFAULT(FALSE));1060+ DEFAULT(FALSE));
1061+static Sys_var_mybool Sys_log_slow_admin_statements(
1062+ "log_slow_admin_statements",
1063+ "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements"
1064+ " to the slow log if it is open.",
1065+ GLOBAL_VAR(opt_log_slow_admin_statements), CMD_LINE(OPT_ARG),
1066+ DEFAULT(FALSE));
1057+static Sys_var_mybool Sys_log_slow_sp_statements(1067+static Sys_var_mybool Sys_log_slow_sp_statements(
1058+ "log_slow_sp_statements",1068+ "log_slow_sp_statements",
1059+ "Log slow statements executed by stored procedure to the slow log if it is open.",1069+ "Log slow statements executed by stored procedure to the slow log if it is open.",
@@ -2415,3 +2425,152 @@
2415+--let grep_pattern = Last_errno: 10502425+--let grep_pattern = Last_errno: 1050
2416+--source include/log_grep.inc2426+--source include/log_grep.inc
2417+DROP TABLE t;2427+DROP TABLE t;
2428--- /dev/null
2429+++ b/mysql-test/suite/sys_vars/t/log_slow_admin_statements_basic.test
2430@@ -0,0 +1 @@
2431+SELECT @@global.log_slow_admin_statements;
2432--- /dev/null
2433+++ b/mysql-test/suite/sys_vars/r/log_slow_admin_statements_basic.result
2434@@ -0,0 +1,3 @@
2435+SELECT @@global.log_slow_admin_statements;
2436+@@global.log_slow_admin_statements
2437+0
2438--- /dev/null
2439+++ b/mysql-test/r/percona_log_slow_admin_statements.result
2440@@ -0,0 +1,35 @@
2441+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2442+Variable_name Value
2443+log_slow_admin_statements OFF
2444+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2445+VARIABLE_NAME VARIABLE_VALUE
2446+LOG_SLOW_ADMIN_STATEMENTS OFF
2447+SET GLOBAL log_slow_admin_statements=true;
2448+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2449+Variable_name Value
2450+log_slow_admin_statements ON
2451+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2452+VARIABLE_NAME VARIABLE_VALUE
2453+LOG_SLOW_ADMIN_STATEMENTS ON
2454+SET GLOBAL log_slow_admin_statements=false;
2455+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2456+Variable_name Value
2457+log_slow_admin_statements OFF
2458+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2459+VARIABLE_NAME VARIABLE_VALUE
2460+LOG_SLOW_ADMIN_STATEMENTS OFF
2461+SET GLOBAL log_slow_admin_statements=foo;
2462+ERROR 42000: Variable 'log_slow_admin_statements' can't be set to the value of 'foo'
2463+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2464+Variable_name Value
2465+log_slow_admin_statements OFF
2466+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2467+VARIABLE_NAME VARIABLE_VALUE
2468+LOG_SLOW_ADMIN_STATEMENTS OFF
2469+SET GLOBAL log_slow_admin_statements=default;
2470+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2471+Variable_name Value
2472+log_slow_admin_statements OFF
2473+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2474+VARIABLE_NAME VARIABLE_VALUE
2475+LOG_SLOW_ADMIN_STATEMENTS OFF
2476--- /dev/null
2477+++ b/mysql-test/r/percona_log_slow_admin_statements-config_false.result
2478@@ -0,0 +1,6 @@
2479+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2480+Variable_name Value
2481+log_slow_admin_statements OFF
2482+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2483+VARIABLE_NAME VARIABLE_VALUE
2484+LOG_SLOW_ADMIN_STATEMENTS OFF
2485--- /dev/null
2486+++ b/mysql-test/r/percona_log_slow_admin_statements-config_foo.result
2487@@ -0,0 +1,7 @@
2488+call mtr.add_suppression("option 'log_slow_admin_statements': boolean value 'foo' wasn't recognized. Set to OFF.");
2489+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2490+Variable_name Value
2491+log_slow_admin_statements OFF
2492+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2493+VARIABLE_NAME VARIABLE_VALUE
2494+LOG_SLOW_ADMIN_STATEMENTS OFF
2495--- /dev/null
2496+++ b/mysql-test/r/percona_log_slow_admin_statements-config_true.result
2497@@ -0,0 +1,6 @@
2498+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2499+Variable_name Value
2500+log_slow_admin_statements ON
2501+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2502+VARIABLE_NAME VARIABLE_VALUE
2503+LOG_SLOW_ADMIN_STATEMENTS ON
2504--- /dev/null
2505+++ b/mysql-test/r/percona_log_slow_admin_statements-config.result
2506@@ -0,0 +1,6 @@
2507+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2508+Variable_name Value
2509+log_slow_admin_statements ON
2510+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2511+VARIABLE_NAME VARIABLE_VALUE
2512+LOG_SLOW_ADMIN_STATEMENTS ON
2513--- /dev/null
2514+++ b/mysql-test/t/percona_log_slow_admin_statements-config_false.cnf
2515@@ -0,0 +1,2 @@
2516+[mysqld.1]
2517+log-slow-admin-statements=false
2518--- /dev/null
2519+++ b/mysql-test/t/percona_log_slow_admin_statements-config_foo.cnf
2520@@ -0,0 +1,2 @@
2521+[mysqld.1]
2522+log-slow-admin-statements=foo
2523--- /dev/null
2524+++ b/mysql-test/t/percona_log_slow_admin_statements-config_true.cnf
2525@@ -0,0 +1,2 @@
2526+[mysqld.1]
2527+log-slow-admin-statements=true
2528--- /dev/null
2529+++ b/mysql-test/t/percona_log_slow_admin_statements-config.cnf
2530@@ -0,0 +1,2 @@
2531+[mysqld.1]
2532+log-slow-admin-statements
2533--- /dev/null
2534+++ b/mysql-test/t/percona_log_slow_admin_statements.test
2535@@ -0,0 +1,20 @@
2536+# default value
2537+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2538+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2539+# set value to 'true'
2540+SET GLOBAL log_slow_admin_statements=true;
2541+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2542+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2543+# set value to 'false'
2544+SET GLOBAL log_slow_admin_statements=false;
2545+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2546+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2547+# set value to 'foo'
2548+--error ER_WRONG_VALUE_FOR_VAR
2549+SET GLOBAL log_slow_admin_statements=foo;
2550+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2551+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2552+# set value to default
2553+SET GLOBAL log_slow_admin_statements=default;
2554+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2555+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2556--- /dev/null
2557+++ b/mysql-test/t/percona_log_slow_admin_statements-config_false.test
2558@@ -0,0 +1,2 @@
2559+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2560+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2561--- /dev/null
2562+++ b/mysql-test/t/percona_log_slow_admin_statements-config_foo.test
2563@@ -0,0 +1,3 @@
2564+call mtr.add_suppression("option 'log_slow_admin_statements': boolean value 'foo' wasn't recognized. Set to OFF.");
2565+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2566+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2567--- /dev/null
2568+++ b/mysql-test/t/percona_log_slow_admin_statements-config_true.test
2569@@ -0,0 +1,2 @@
2570+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2571+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
2572--- /dev/null
2573+++ b/mysql-test/t/percona_log_slow_admin_statements-config.test
2574@@ -0,0 +1,2 @@
2575+SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
2576+SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';

Subscribers

People subscribed via source and target branches