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

Merge lp:~tsarev/percona-server/12952_5.5 into lp:percona-server/5.5

  1. 12952_5.5
  2. Merge into 5.5
Proposed by Oleg Tsarev
Status: Merged
Approved by: Alexey Kopytov
Approved revision: no longer in the source branch.
Merged at revision: 148
Proposed branch: lp:~tsarev/percona-server/12952_5.5
Merge into: lp:percona-server/5.5
Diff against target: 384 lines (+255/-73)
1 file modified
patches/show_slave_status_nolock.patch (+255/-73)
To merge this branch: bzr merge lp:~tsarev/percona-server/12952_5.5
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Review via email: mp+76270@code.launchpad.net

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

Description of the change

Add test-case for bug #851011: SHOW SLAVE STATUS NOLOCK acquire lock on
  sql/slave.cc: 1650
    pthread_mutex_lock(&mi->run_lock);
Fix: wrap mutex lock/unlock by check to \"UNLOCK\" statement
Issue number: #12952.

Checked mutexes, like 5.1. The same.

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/115/

Revision history for this message
Oleg Tsarev (tsarev) wrote :

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

Revision history for this message
Alexey Kopytov (akopytov) wrote :

OK to push, assuming Jenkins build is OK.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'patches/show_slave_status_nolock.patch'
2--- patches/show_slave_status_nolock.patch 2011-09-12 12:08:08 +0000
3+++ patches/show_slave_status_nolock.patch 2011-09-20 18:12:22 +0000
4@@ -55,7 +55,7 @@
5 sql_command_flags[SQLCOM_SHOW_CREATE_PROC]= CF_STATUS_COMMAND;
6 sql_command_flags[SQLCOM_SHOW_CREATE_FUNC]= CF_STATUS_COMMAND;
7 sql_command_flags[SQLCOM_SHOW_CREATE_TRIGGER]= CF_STATUS_COMMAND;
8-@@ -2363,12 +2364,16 @@
9+@@ -2365,12 +2366,17 @@
10 mysql_mutex_unlock(&LOCK_active_mi);
11 break;
12 }
13@@ -66,22 +66,32 @@
14 if (check_global_access(thd, SUPER_ACL | REPL_CLIENT_ACL))
15 goto error;
16 - mysql_mutex_lock(&LOCK_active_mi);
17-+ if(SQLCOM_SHOW_SLAVE_NOLOCK_STAT != lex->sql_command)
18++ bool do_lock=SQLCOM_SHOW_SLAVE_NOLOCK_STAT != lex->sql_command;
19++ if(do_lock)
20 + {
21 + mysql_mutex_lock(&LOCK_active_mi);
22 + }
23 if (active_mi != NULL)
24 {
25 res = show_master_info(thd, active_mi);
26-@@ -2379,7 +2384,10 @@
27+@@ -2381,7 +2387,19 @@
28 WARN_NO_MASTER_INFO, ER(WARN_NO_MASTER_INFO));
29 my_ok(thd);
30 }
31 - mysql_mutex_unlock(&LOCK_active_mi);
32-+ if(SQLCOM_SHOW_SLAVE_NOLOCK_STAT != lex->sql_command)
33++ if(do_lock)
34 + {
35 + mysql_mutex_unlock(&LOCK_active_mi);
36 + }
37++ DBUG_EXECUTE_IF("after_show_slave_status",
38++ {
39++ const char act[]=
40++ "now "
41++ "signal signal.after_show_slave_status";
42++ DBUG_ASSERT(opt_debug_sync_timeout > 0);
43++ DBUG_ASSERT(!debug_sync_set_action(current_thd,
44++ STRING_WITH_LEN(act)));
45++ };);
46 break;
47 }
48 case SQLCOM_SHOW_MASTER_STAT:
49@@ -95,10 +105,11 @@
50 %token STDDEV_SAMP_SYM /* SQL-2003-N */
51 %token STD_SYM
52 %token STOP_SYM
53-@@ -11086,6 +11087,10 @@
54+@@ -11086,6 +11087,11 @@
55 {
56 Lex->sql_command = SQLCOM_SHOW_SLAVE_STAT;
57 }
58++ /* SHOW SLAVE STATUS NOLOCK */
59 + | SLAVE STATUS_SYM NOLOCK_SYM
60 + {
61 + Lex->sql_command = SQLCOM_SHOW_SLAVE_NOLOCK_STAT; //SQLCOM_SHOW_SLAVE_NOLOCK_STAT;
62@@ -107,83 +118,254 @@
63 {
64 #ifdef HAVE_RESPONSE_TIME_DISTRIBUTION
65 --- /dev/null
66++++ b/mysql-test/t/percona_show_slave_status_nolock.test
67+@@ -0,0 +1,90 @@
68++--source include/master-slave.inc
69++--source include/have_debug_sync.inc
70++--source include/have_binlog_format_statement.inc
71++
72++call mtr.add_suppression("Slave SQL: Request to stop slave SQL Thread received while applying a group that has non-transactional changes");
73++
74++--let $rpl_connection_name=slave_lock
75++--let $rpl_server_number=2
76++--source include/rpl_connect.inc
77++
78++--let $rpl_connection_name=slave_nolock
79++--let $rpl_server_number=2
80++--source include/rpl_connect.inc
81++
82++--let $show_statement= SHOW PROCESSLIST
83++--let $field= Info
84++
85++connection master;
86++--echo [master]
87++--disable_warnings
88++DROP TABLE IF EXISTS t;
89++--enable_warnings
90++CREATE TABLE t(id INT);
91++sync_slave_with_master;
92++
93++connection slave;
94++--echo [slave]
95++SET DEBUG_SYNC='RESET';
96++SET GLOBAL DEBUG="+d,after_mysql_insert,after_show_slave_status";
97++
98++connection master;
99++--echo [master]
100++INSERT INTO t VALUES(0);
101++
102++connection slave;
103++--echo [slave]
104++--let $condition= 'INSERT INTO t VALUES(0)'
105++--source include/wait_show_condition.inc
106++
107++--echo check 'SHOW SLAVE STATUS' and 'SHOW SLAVE STATUS NOLOCK' - both should work fine
108++--source include/percona_show_slave_status_nolock.inc
109++
110++connection master;
111++--echo [master]
112++INSERT INTO t VALUES(1);
113++
114++connection slave;
115++--echo [slave]
116++--let $condition= 'INSERT INTO t VALUES(1)'
117++--source include/wait_show_condition.inc
118++
119++--let $rpl_connection_name=slave_stop
120++--let $rpl_server_number=2
121++--source include/rpl_connect.inc
122++
123++connection slave_stop;
124++--echo [slave_stop]
125++send STOP SLAVE;
126++
127++connection slave;
128++--echo [slave]
129++--let $condition= 'STOP SLAVE'
130++--source include/wait_show_condition.inc
131++
132++--echo check 'SHOW SLAVE STATUS' and 'SHOW SLAVE STATUS NOLOCK' - just NOLOCK version should works fine
133++--source include/percona_show_slave_status_nolock.inc
134++
135++
136++connection slave_stop;
137++--echo [slave_stop]
138++reap;
139++--source include/wait_for_slave_to_stop.inc
140++START SLAVE;
141++--source include/wait_for_slave_to_start.inc
142++
143++connection master;
144++--echo [master]
145++SET DEBUG_SYNC='RESET';
146++
147++connection slave;
148++--echo [slave]
149++SET GLOBAL DEBUG='';
150++SET DEBUG_SYNC='RESET';
151++
152++connection master;
153++--echo [master]
154++DROP TABLE t;
155++sync_slave_with_master;
156++
157++--source include/rpl_end.inc
158+--- /dev/null
159 +++ b/mysql-test/r/percona_show_slave_status_nolock.result
160-@@ -0,0 +1,21 @@
161+@@ -0,0 +1,69 @@
162 +include/master-slave.inc
163 +[connection master]
164++call mtr.add_suppression("Slave SQL: Request to stop slave SQL Thread received while applying a group that has non-transactional changes");
165++include/rpl_connect.inc [creating slave_lock]
166++include/rpl_connect.inc [creating slave_nolock]
167++[master]
168 +DROP TABLE IF EXISTS t;
169 +CREATE TABLE t(id INT);
170-+INSERT INTO t SELECT SLEEP(10);
171++[slave]
172++SET DEBUG_SYNC='RESET';
173++SET GLOBAL DEBUG="+d,after_mysql_insert,after_show_slave_status";
174++[master]
175++INSERT INTO t VALUES(0);
176++[slave]
177++check 'SHOW SLAVE STATUS' and 'SHOW SLAVE STATUS NOLOCK' - both should work fine
178++
179++[slave_lock]
180++SHOW SLAVE STATUS;
181++SET DEBUG_SYNC='now WAIT_FOR signal.after_show_slave_status TIMEOUT 1';
182++SIGNAL after SHOW SLAVE STATUS is 'signal.after_show_slave_status'
183++[slave]
184++SET DEBUG_SYNC='now SIGNAL signal.empty';
185++[slave_nolock]
186++SHOW SLAVE STATUS NOLOCK;
187++SET DEBUG_SYNC='now WAIT_FOR signal.after_show_slave_status TIMEOUT 1';
188++# should be 'signal.after_show_slave_status'
189++SIGNAL after SHOW SLAVE STATUS NOLOCK is 'signal.after_show_slave_status'
190++[slave]
191++SET DEBUG_SYNC='now SIGNAL signal.continue';
192++[slave]
193++SET DEBUG_SYNC='now SIGNAL signal.empty';
194++
195++[master]
196++INSERT INTO t VALUES(1);
197++[slave]
198++include/rpl_connect.inc [creating slave_stop]
199++[slave_stop]
200 +STOP SLAVE;
201-+Warnings:
202-+Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system function that may return a different value on the slave.
203-+master count(*)
204-+master 1
205-+slave count(*)
206-+slave 0
207++[slave]
208++check 'SHOW SLAVE STATUS' and 'SHOW SLAVE STATUS NOLOCK' - just NOLOCK version should works fine
209++
210++[slave_lock]
211++SHOW SLAVE STATUS;
212++SET DEBUG_SYNC='now WAIT_FOR signal.after_show_slave_status TIMEOUT 1';
213++SIGNAL after SHOW SLAVE STATUS is 'signal.empty'
214++[slave]
215++SET DEBUG_SYNC='now SIGNAL signal.empty';
216++[slave_nolock]
217 +SHOW SLAVE STATUS NOLOCK;
218++SET DEBUG_SYNC='now WAIT_FOR signal.after_show_slave_status TIMEOUT 1';
219++# should be 'signal.after_show_slave_status'
220++SIGNAL after SHOW SLAVE STATUS NOLOCK is 'signal.after_show_slave_status'
221++[slave]
222++SET DEBUG_SYNC='now SIGNAL signal.continue';
223++[slave]
224++SET DEBUG_SYNC='now SIGNAL signal.empty';
225++
226++[slave_stop]
227 +include/wait_for_slave_to_stop.inc
228 +START SLAVE;
229 +include/wait_for_slave_to_start.inc
230-+slave count(*)
231-+slave 1
232++[master]
233++SET DEBUG_SYNC='RESET';
234++[slave]
235++SET GLOBAL DEBUG='';
236++SET DEBUG_SYNC='RESET';
237++[master]
238 +DROP TABLE t;
239-+STOP SLAVE;
240-+include/wait_for_slave_to_stop.inc
241++include/rpl_end.inc
242 --- /dev/null
243-+++ b/mysql-test/t/percona_show_slave_status_nolock.test
244-@@ -0,0 +1,53 @@
245-+--source include/master-slave.inc
246-+--source include/have_binlog_format_statement.inc
247-+--disable_query_log
248-+call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system function that may return a different value on the slave. Statement:");
249-+call mtr.add_suppression("Slave SQL.*Request to stop slave SQL Thread received while applying a group that has non-transactional changes; waiting for completion of the group");
250-+--enable_query_log
251-+connection master;
252-+ --disable_warnings
253-+ DROP TABLE IF EXISTS t;
254-+ --enable_warnings
255-+ CREATE TABLE t(id INT);
256-+ sync_slave_with_master;
257-+
258-+connection master;
259-+ send INSERT INTO t SELECT SLEEP(10);
260-+
261-+connection slave;
262-+ sleep 15;
263-+ send STOP SLAVE;
264-+
265-+connection master;
266-+ reap;
267-+
268-+ --disable_query_log
269-+ select "master",count(*) from t;
270-+ --enable_query_log
271-+
272-+connection slave1;
273-+ --disable_query_log
274-+ select "slave",count(*) from t;
275-+ --enable_query_log
276-+
277-+ --disable_result_log
278-+ SHOW SLAVE STATUS NOLOCK;
279-+ --enable_result_log
280-+
281-+connection slave;
282-+ reap;
283-+
284-+ --source include/wait_for_slave_to_stop.inc
285-+ START SLAVE;
286-+ --source include/wait_for_slave_to_start.inc
287-+
288-+ --disable_query_log
289-+ select "slave",count(*) from t;
290-+ --enable_query_log
291-+
292-+connection master;
293-+ DROP TABLE t;
294-+sync_slave_with_master;
295-+STOP SLAVE;
296-+--source include/wait_for_slave_to_stop.inc
297-+
298-\ No newline at end of file
299++++ b/mysql-test/include/percona_show_slave_status_nolock.inc
300+@@ -0,0 +1,56 @@
301++--echo
302++--disable_result_log
303++connection slave_lock;
304++--echo [slave_lock]
305++send SHOW SLAVE STATUS;
306++
307++connection slave;
308++--let $condition= 'SHOW SLAVE STATUS'
309++--source include/wait_show_condition.inc
310++
311++--disable_warnings
312++SET DEBUG_SYNC='now WAIT_FOR signal.after_show_slave_status TIMEOUT 1';
313++--enable_warnings
314++
315++--let current=`SELECT SUBSTR(Variable_value FROM 22) FROM INFORMATION_SCHEMA.SESSION_VARIABLES WHERE Variable_name = 'DEBUG_SYNC'`
316++--echo SIGNAL after SHOW SLAVE STATUS is $current
317++
318++connection slave;
319++--echo [slave]
320++SET DEBUG_SYNC='now SIGNAL signal.empty';
321++
322++connection slave_nolock;
323++--echo [slave_nolock]
324++send SHOW SLAVE STATUS NOLOCK;
325++
326++connection slave;
327++--let $condition= 'SHOW SLAVE STATUS NOLOCK'
328++--source include/wait_show_condition.inc
329++
330++--disable_warnings
331++SET DEBUG_SYNC='now WAIT_FOR signal.after_show_slave_status TIMEOUT 1';
332++--enable_warnings
333++
334++--echo # should be 'signal.after_show_slave_status'
335++--let current=`SELECT SUBSTR(Variable_value FROM 22) FROM INFORMATION_SCHEMA.SESSION_VARIABLES WHERE Variable_name = 'DEBUG_SYNC'`
336++--echo SIGNAL after SHOW SLAVE STATUS NOLOCK is $current
337++
338++connection slave;
339++--echo [slave]
340++SET DEBUG_SYNC='now SIGNAL signal.continue';
341++
342++connection slave_lock;
343++--disable_result_log
344++reap;
345++--enable_result_log
346++
347++connection slave_nolock;
348++--disable_result_log
349++reap;
350++--enable_result_log
351++
352++connection slave;
353++--echo [slave]
354++SET DEBUG_SYNC='now SIGNAL signal.empty';
355++--enable_result_log
356++--echo
357+--- a/sql/slave.cc
358++++ b/sql/slave.cc
359+@@ -1816,6 +1816,7 @@
360+
361+ if (mi->host[0])
362+ {
363++ bool do_lock=SQLCOM_SHOW_SLAVE_NOLOCK_STAT != thd->lex->sql_command;
364+ DBUG_PRINT("info",("host is set: '%s'", mi->host));
365+ String *packet= &thd->packet;
366+ protocol->prepare_for_resend();
367+@@ -1824,9 +1825,15 @@
368+ slave_running can be accessed without run_lock but not other
369+ non-volotile members like mi->io_thd, which is guarded by the mutex.
370+ */
371+- mysql_mutex_lock(&mi->run_lock);
372++ if (do_lock)
373++ {
374++ mysql_mutex_lock(&mi->run_lock);
375++ }
376+ protocol->store(mi->io_thd ? mi->io_thd->proc_info : "", &my_charset_bin);
377+- mysql_mutex_unlock(&mi->run_lock);
378++ if (do_lock)
379++ {
380++ mysql_mutex_unlock(&mi->run_lock);
381++ }
382+
383+ mysql_mutex_lock(&mi->data_lock);
384+ mysql_mutex_lock(&mi->rli.data_lock);

Subscribers

People subscribed via source and target branches