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

Proposed by Oleg Tsarev on 2011-09-29
Status: Merged
Approved by: Alexey Kopytov on 2011-09-29
Approved revision: 151
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) 2011-09-29 Approve on 2011-09-29
Laurynas Biveinis 2011-09-29 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.
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
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
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
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

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
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
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
Alexey Kopytov (akopytov) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'patches/mysql-test.diff'
2--- patches/mysql-test.diff 2011-09-25 07:11:59 +0000
3+++ patches/mysql-test.diff 2011-09-29 06:09:30 +0000
4@@ -1489,6 +1489,7 @@
5 +LOG_OUTPUT
6 +LOG_QUERIES_NOT_USING_INDEXES
7 +LOG_SLAVE_UPDATES
8++LOG_SLOW_ADMIN_STATEMENTS
9 +LOG_SLOW_FILTER
10 +LOG_SLOW_QUERIES
11 +LOG_SLOW_RATE_LIMIT
12@@ -1863,6 +1864,7 @@
13 +LOG_OUTPUT
14 +LOG_QUERIES_NOT_USING_INDEXES
15 +LOG_SLAVE_UPDATES
16++LOG_SLOW_ADMIN_STATEMENTS
17 +LOG_SLOW_FILTER
18 +LOG_SLOW_QUERIES
19 +LOG_SLOW_RATE_LIMIT
20
21=== modified file 'patches/slow_extended.patch'
22--- patches/slow_extended.patch 2011-09-21 16:16:06 +0000
23+++ patches/slow_extended.patch 2011-09-29 06:09:30 +0000
24@@ -424,10 +424,14 @@
25 my_bool lower_case_file_system= 0;
26 my_bool opt_large_pages= 0;
27 my_bool opt_super_large_pages= 0;
28-@@ -5896,10 +5900,10 @@
29- "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements to "
30- "the slow log if it is open.", &opt_log_slow_admin_statements,
31- &opt_log_slow_admin_statements, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
32+@@ -5892,14 +5896,10 @@
33+ "Don't log extra information to update and slow-query logs.",
34+ &opt_short_log_format, &opt_short_log_format,
35+ 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
36+- {"log-slow-admin-statements", 0,
37+- "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements to "
38+- "the slow log if it is open.", &opt_log_slow_admin_statements,
39+- &opt_log_slow_admin_statements, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
40 - {"log-slow-slave-statements", 0,
41 + /*{"log-slow-slave-statements", 0,
42 "Log slow statements executed by slave thread to the slow log if it is open.",
43@@ -437,7 +441,7 @@
44 {"log-slow-queries", OPT_SLOW_QUERY_LOG,
45 "Log slow queries to a table or log file. Defaults logging to table "
46 "mysql.slow_log or hostname-slow.log if --log-output=file is used. "
47-@@ -7288,6 +7292,10 @@
48+@@ -7288,6 +7288,10 @@
49
50 C_MODE_END
51
52@@ -448,7 +452,7 @@
53 /**
54 Get server options from the command line,
55 and perform related server initializations.
56-@@ -7437,6 +7445,8 @@
57+@@ -7437,6 +7441,8 @@
58 global_system_variables.long_query_time= (ulonglong)
59 (global_system_variables.long_query_time_double * 1e6);
60
61@@ -979,7 +983,7 @@
62 static bool fix_low_prio_updates(sys_var *self, THD *thd, enum_var_type type)
63 {
64 if (type == OPT_SESSION)
65-@@ -2898,6 +2921,117 @@
66+@@ -2898,6 +2921,123 @@
67 DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
68 ON_UPDATE(fix_log_state));
69
70@@ -1054,6 +1058,12 @@
71 + "Log queries replayed be the slave SQL thread",
72 + GLOBAL_VAR(opt_log_slow_slave_statements), CMD_LINE(OPT_ARG),
73 + DEFAULT(FALSE));
74++static Sys_var_mybool Sys_log_slow_admin_statements(
75++ "log_slow_admin_statements",
76++ "Log slow OPTIMIZE, ANALYZE, ALTER and other administrative statements"
77++ " to the slow log if it is open.",
78++ GLOBAL_VAR(opt_log_slow_admin_statements), CMD_LINE(OPT_ARG),
79++ DEFAULT(FALSE));
80 +static Sys_var_mybool Sys_log_slow_sp_statements(
81 + "log_slow_sp_statements",
82 + "Log slow statements executed by stored procedure to the slow log if it is open.",
83@@ -2415,3 +2425,152 @@
84 +--let grep_pattern = Last_errno: 1050
85 +--source include/log_grep.inc
86 +DROP TABLE t;
87+--- /dev/null
88++++ b/mysql-test/suite/sys_vars/t/log_slow_admin_statements_basic.test
89+@@ -0,0 +1 @@
90++SELECT @@global.log_slow_admin_statements;
91+--- /dev/null
92++++ b/mysql-test/suite/sys_vars/r/log_slow_admin_statements_basic.result
93+@@ -0,0 +1,3 @@
94++SELECT @@global.log_slow_admin_statements;
95++@@global.log_slow_admin_statements
96++0
97+--- /dev/null
98++++ b/mysql-test/r/percona_log_slow_admin_statements.result
99+@@ -0,0 +1,35 @@
100++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
101++Variable_name Value
102++log_slow_admin_statements OFF
103++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
104++VARIABLE_NAME VARIABLE_VALUE
105++LOG_SLOW_ADMIN_STATEMENTS OFF
106++SET GLOBAL log_slow_admin_statements=true;
107++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
108++Variable_name Value
109++log_slow_admin_statements ON
110++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
111++VARIABLE_NAME VARIABLE_VALUE
112++LOG_SLOW_ADMIN_STATEMENTS ON
113++SET GLOBAL log_slow_admin_statements=false;
114++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
115++Variable_name Value
116++log_slow_admin_statements OFF
117++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
118++VARIABLE_NAME VARIABLE_VALUE
119++LOG_SLOW_ADMIN_STATEMENTS OFF
120++SET GLOBAL log_slow_admin_statements=foo;
121++ERROR 42000: Variable 'log_slow_admin_statements' can't be set to the value of 'foo'
122++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
123++Variable_name Value
124++log_slow_admin_statements OFF
125++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
126++VARIABLE_NAME VARIABLE_VALUE
127++LOG_SLOW_ADMIN_STATEMENTS OFF
128++SET GLOBAL log_slow_admin_statements=default;
129++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
130++Variable_name Value
131++log_slow_admin_statements OFF
132++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
133++VARIABLE_NAME VARIABLE_VALUE
134++LOG_SLOW_ADMIN_STATEMENTS OFF
135+--- /dev/null
136++++ b/mysql-test/r/percona_log_slow_admin_statements-config_false.result
137+@@ -0,0 +1,6 @@
138++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
139++Variable_name Value
140++log_slow_admin_statements OFF
141++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
142++VARIABLE_NAME VARIABLE_VALUE
143++LOG_SLOW_ADMIN_STATEMENTS OFF
144+--- /dev/null
145++++ b/mysql-test/r/percona_log_slow_admin_statements-config_foo.result
146+@@ -0,0 +1,7 @@
147++call mtr.add_suppression("option 'log_slow_admin_statements': boolean value 'foo' wasn't recognized. Set to OFF.");
148++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
149++Variable_name Value
150++log_slow_admin_statements OFF
151++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
152++VARIABLE_NAME VARIABLE_VALUE
153++LOG_SLOW_ADMIN_STATEMENTS OFF
154+--- /dev/null
155++++ b/mysql-test/r/percona_log_slow_admin_statements-config_true.result
156+@@ -0,0 +1,6 @@
157++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
158++Variable_name Value
159++log_slow_admin_statements ON
160++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
161++VARIABLE_NAME VARIABLE_VALUE
162++LOG_SLOW_ADMIN_STATEMENTS ON
163+--- /dev/null
164++++ b/mysql-test/r/percona_log_slow_admin_statements-config.result
165+@@ -0,0 +1,6 @@
166++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
167++Variable_name Value
168++log_slow_admin_statements ON
169++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
170++VARIABLE_NAME VARIABLE_VALUE
171++LOG_SLOW_ADMIN_STATEMENTS ON
172+--- /dev/null
173++++ b/mysql-test/t/percona_log_slow_admin_statements-config_false.cnf
174+@@ -0,0 +1,2 @@
175++[mysqld.1]
176++log-slow-admin-statements=false
177+--- /dev/null
178++++ b/mysql-test/t/percona_log_slow_admin_statements-config_foo.cnf
179+@@ -0,0 +1,2 @@
180++[mysqld.1]
181++log-slow-admin-statements=foo
182+--- /dev/null
183++++ b/mysql-test/t/percona_log_slow_admin_statements-config_true.cnf
184+@@ -0,0 +1,2 @@
185++[mysqld.1]
186++log-slow-admin-statements=true
187+--- /dev/null
188++++ b/mysql-test/t/percona_log_slow_admin_statements-config.cnf
189+@@ -0,0 +1,2 @@
190++[mysqld.1]
191++log-slow-admin-statements
192+--- /dev/null
193++++ b/mysql-test/t/percona_log_slow_admin_statements.test
194+@@ -0,0 +1,20 @@
195++# default value
196++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
197++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
198++# set value to 'true'
199++SET GLOBAL log_slow_admin_statements=true;
200++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
201++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
202++# set value to 'false'
203++SET GLOBAL log_slow_admin_statements=false;
204++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
205++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
206++# set value to 'foo'
207++--error ER_WRONG_VALUE_FOR_VAR
208++SET GLOBAL log_slow_admin_statements=foo;
209++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
210++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
211++# set value to default
212++SET GLOBAL log_slow_admin_statements=default;
213++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
214++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
215+--- /dev/null
216++++ b/mysql-test/t/percona_log_slow_admin_statements-config_false.test
217+@@ -0,0 +1,2 @@
218++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
219++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
220+--- /dev/null
221++++ b/mysql-test/t/percona_log_slow_admin_statements-config_foo.test
222+@@ -0,0 +1,3 @@
223++call mtr.add_suppression("option 'log_slow_admin_statements': boolean value 'foo' wasn't recognized. Set to OFF.");
224++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
225++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
226+--- /dev/null
227++++ b/mysql-test/t/percona_log_slow_admin_statements-config_true.test
228+@@ -0,0 +1,2 @@
229++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
230++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';
231+--- /dev/null
232++++ b/mysql-test/t/percona_log_slow_admin_statements-config.test
233+@@ -0,0 +1,2 @@
234++SHOW GLOBAL VARIABLES like 'log_slow_admin_statements';
235++SELECT * FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES WHERE VARIABLE_NAME='log_slow_admin_statements';

Subscribers

People subscribed via source and target branches