Percona Server moved to https://jira.percona.com/projects/PS

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

  1. test55_691404
  2. Merge into 5.5
Proposed by Oleg Tsarev
Status: Superseded
Proposed branch: lp:~tsarev/percona-server/test55_691404
Merge into: lp:percona-server/5.5
Diff against target: 253 lines (+170/-9)
2 files modified
patches/mysql-test.diff (+4/-2)
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) Needs Fixing
Laurynas Biveinis Pending
Review via email: mp+77212@code.launchpad.net

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

This proposal has been superseded by a proposal from 2011-09-29.

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))

to Laurynas: all renamed as you describe
to Alexey:

1) NO_ARG doesn't work as you described:
sys_vars.h:324: Sys_var_mybool::Sys_var_mybool(const char*, const char*, int, ptrdiff_t, size_t, CMD_LINE, my_bool, PolyLock*, sys_var::binlog_status_enum, bool (*)(sys_var*, THD*, set_var*), bool (*)(sys_var*, THD*, enum_var_type), uint, const char*, int): Assertion `getopt.arg_type == OPT_ARG || getopt.id == -1' failed.
mysql-test-run: *** ERROR: Could not find version of MySQL
2) OPT_ARG works fine.
3) REQUIRED_ARG works as incorrect: see bug #712387
4) We have commented "log-slow-slave-statements". I just change comment to both construction: "log-slow-admin-statements" and "log-slow-admin-statements"

Also, logically right use "OPT_ARG", because we have following syntaxes:
--log_slow_admin_statements
--log_slow_admin_statements=1
--log_slow_admin_statements=0

I replace comment of code in mysqld.cc by code removal.

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

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

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

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

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

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

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

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

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

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

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

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 :

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

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

Subscribers

People subscribed via source and target branches