Merge lp:~gl-az/percona-server/ST-31919-5.6 into lp:percona-server/5.6

Proposed by George Ormond Lorch III
Status: Merged
Approved by: Alexey Kopytov
Approved revision: no longer in the source branch.
Merged at revision: 391
Proposed branch: lp:~gl-az/percona-server/ST-31919-5.6
Merge into: lp:percona-server/5.6
Diff against target: 349 lines (+0/-160)
3 files modified
Percona-Server/mysql-test/suite/federated/federated_bug_68354.result (+0/-80)
Percona-Server/mysql-test/suite/federated/federated_bug_68354.test (+0/-79)
Percona-Server/storage/federated/ha_federated.cc (+0/-1)
To merge this branch: bzr merge lp:~gl-az/percona-server/ST-31919-5.6
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Review via email: mp+171374@code.launchpad.net

This proposal supersedes a proposal from 2013-06-18.

Description of the change

Resubmitting with correct GCA procedure followed whcih will cause issues now when merging to trunk but _should_ leave the trunk in a clean state.

Jenkins --suite=federated: http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/166/

Fix for upstream 68354 and lp 1182572:
  Server crashes on update/join FEDERATED + local table when only 1 local row

This issue seems to be the result of a poor or changed assumption in the order
in which handler calls are made. In this specific scenario (a join with a local
table that has exactly one matching record) there is an assumption in the code
that a call to position will always come after a 'rnd_init/index_init' and
'rnd_next/index_read' but before 'rnd_end/index_end' is called.

The failing call order is:
  ha_federated::index_init
  ha_federated::index_read
  ha_federated::index_end
  ha_federated::position
  ha_federated::rnd_init
  ha_federated::extra
  ha_federated::rnd_pos

So here you can see position is getting called _outside_ of the scope of an
_init.._end table and/or index scan. This does indicate a change/problem in
the server but that is outside the scope of this fix.

As results are fetched, they are added to an interal list of result sets called
'results'. The last result set/current position is also held in the
'stored_result' value. If 'index_end' or 'rnd_end' is called before 'position'
has been called, the 'stored_result' is zeroed out and that result is removed
from the 'stored_results' list. When 'position' is called, a NULL result is
returned, which is then passed back through 'rnd_pos', which then dereferences
the NULL value and tries to manipulate some of the result internals, crashing.

So seems that the intention of the whole 'stored_result' was to have a place
to track all results allocated for the purposes of keeping them valid for the
'position' and 'rnd_pos' calls.

My approch to fixing this is to restructure this somewhan and instead of
tracking all results allocated in the 'results' list and popping them out when
they are discovered to not be needed, I went the other way. The results only
get addes to a storage list when it has been proven that they are in fact
needed later. So now for each new result set, it is temporarily stored in a
new value called 'last_result'. Whenever a new last_result needs created, it is
set by calling a new function 'set_last_result'. This function will check to
see if 'position' has been called on that result and if so, add it to the
(renamed) list 'stored_results' and clears the 'position_called' flag, else it
will free the 'last_result'. It then sets the new 'last_result' value.

Because of the out of scan scope call of position, we can no longer clear the
'stored_results' lists on '_end'. It will be cleared now when 'reset' is
called.

I have created a test case (suite/federated/federated_bug_68354.test) and
verified that it fails without the fix. Everything tests out under the federated
suite and the issue appears fixed.

The only downside is that a lengthy query could use a lot of memory while
hanging onto old 'position'ed results but I can't see how that can be safely
avoided in this particular handler.

To post a comment you must log in.
Revision history for this message
George Ormond Lorch III (gl-az) 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

Same comments as in the 5.5 MP.

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

Approved with the same comments as in the 5.5 MP.

review: Approve
Revision history for this message
George Ormond Lorch III (gl-az) wrote : Posted in a previous version of this proposal

Fixed as in 5.6 and pushed to same branch.

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
=== added file 'Percona-Server/mysql-test/suite/federated/federated_bug_68354.result'
--- Percona-Server/mysql-test/suite/federated/federated_bug_68354.result 1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/federated/federated_bug_68354.result 2013-06-25 17:39:44 +0000
@@ -0,0 +1,80 @@
1CREATE DATABASE federated;
2CREATE DATABASE federated;
3SET @OLD_MASTER_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
4SET @@GLOBAL.CONCURRENT_INSERT= 0;
5SET @OLD_SLAVE_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
6SET @@GLOBAL.CONCURRENT_INSERT= 0;
7DROP TABLE IF EXISTS federated.t1;
8Warnings:
9Note 1051 Unknown table 'federated.t1'
10CREATE TABLE federated.t1 (
11`id` int(20) NOT NULL,
12`dummy` int(20) DEFAULT NULL,
13PRIMARY KEY (`id`))
14ENGINE="MyISAM"
15 DEFAULT CHARSET=latin1;
16INSERT INTO federated.t1 (`id`) VALUES
17(1001), (1002), (1003), (1004), (1005),
18(1006), (1007), (1008), (1009), (1010);
19CREATE TABLE federated.t1 (
20`id` int(20) NOT NULL,
21`dummy` int(20) DEFAULT NULL,
22PRIMARY KEY (`id`))
23ENGINE="FEDERATED"
24 DEFAULT CHARSET=latin1
25CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/t1';
26CREATE TABLE federated.t2 (
27`id` int(20) NOT NULL,
28`dummy` int(20) NOT NULL,
29PRIMARY KEY (`id`))
30ENGINE="MyISAM"
31 DEFAULT CHARSET=latin1;
32INSERT INTO federated.t2 (`id`, `dummy`) VALUES (1005, 5);
33SELECT * FROM federated.t1;
34id dummy
351001 NULL
361002 NULL
371003 NULL
381004 NULL
391005 NULL
401006 NULL
411007 NULL
421008 NULL
431009 NULL
441010 NULL
45UPDATE federated.t1 ft1 INNER JOIN federated.t2 ft2 ON ft1.id = ft2.id SET ft1.dummy = ft2.dummy WHERE ft1.dummy IS NULL;
46SELECT * FROM federated.t1;
47id dummy
481001 NULL
491002 NULL
501003 NULL
511004 NULL
521005 5
531006 NULL
541007 NULL
551008 NULL
561009 NULL
571010 NULL
58DELETE FROM federated.t1 WHERE id = 1005;
59SELECT * FROM federated.t1 WHERE id = 1005;
60id dummy
61SELECT * FROM federated.t1;
62id dummy
631001 NULL
641002 NULL
651003 NULL
661004 NULL
671006 NULL
681007 NULL
691008 NULL
701009 NULL
711010 NULL
72DROP TABLE federated.t2;
73DROP TABLE federated.t1;
74DROP TABLE federated.t1;
75SET @@GLOBAL.CONCURRENT_INSERT= @OLD_MASTER_CONCURRENT_INSERT;
76SET @@GLOBAL.CONCURRENT_INSERT= @OLD_SLAVE_CONCURRENT_INSERT;
77DROP TABLE IF EXISTS federated.t1;
78DROP DATABASE federated;
79DROP TABLE IF EXISTS federated.t1;
80DROP DATABASE federated;
081
=== removed file 'Percona-Server/mysql-test/suite/federated/federated_bug_68354.result'
--- Percona-Server/mysql-test/suite/federated/federated_bug_68354.result 2013-06-18 18:39:42 +0000
+++ Percona-Server/mysql-test/suite/federated/federated_bug_68354.result 1970-01-01 00:00:00 +0000
@@ -1,80 +0,0 @@
1CREATE DATABASE federated;
2CREATE DATABASE federated;
3SET @OLD_MASTER_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
4SET @@GLOBAL.CONCURRENT_INSERT= 0;
5SET @OLD_SLAVE_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
6SET @@GLOBAL.CONCURRENT_INSERT= 0;
7DROP TABLE IF EXISTS federated.t1;
8Warnings:
9Note 1051 Unknown table 'federated.t1'
10CREATE TABLE federated.t1 (
11`id` int(20) NOT NULL,
12`dummy` int(20) DEFAULT NULL,
13PRIMARY KEY (`id`))
14ENGINE="MyISAM"
15 DEFAULT CHARSET=latin1;
16INSERT INTO federated.t1 (`id`) VALUES
17(1001), (1002), (1003), (1004), (1005),
18(1006), (1007), (1008), (1009), (1010);
19CREATE TABLE federated.t1 (
20`id` int(20) NOT NULL,
21`dummy` int(20) DEFAULT NULL,
22PRIMARY KEY (`id`))
23ENGINE="FEDERATED"
24 DEFAULT CHARSET=latin1
25CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/t1';
26CREATE TABLE federated.t2 (
27`id` int(20) NOT NULL,
28`dummy` int(20) NOT NULL,
29PRIMARY KEY (`id`))
30ENGINE="MyISAM"
31 DEFAULT CHARSET=latin1;
32INSERT INTO federated.t2 (`id`, `dummy`) VALUES (1005, 5);
33SELECT * FROM federated.t1;
34id dummy
351001 NULL
361002 NULL
371003 NULL
381004 NULL
391005 NULL
401006 NULL
411007 NULL
421008 NULL
431009 NULL
441010 NULL
45UPDATE federated.t1 ft1 INNER JOIN federated.t2 ft2 ON ft1.id = ft2.id SET ft1.dummy = ft2.dummy WHERE ft1.dummy IS NULL;
46SELECT * FROM federated.t1;
47id dummy
481001 NULL
491002 NULL
501003 NULL
511004 NULL
521005 5
531006 NULL
541007 NULL
551008 NULL
561009 NULL
571010 NULL
58DELETE FROM federated.t1 WHERE id = 1005;
59SELECT * FROM federated.t1 WHERE id = 1005;
60id dummy
61SELECT * FROM federated.t1;
62id dummy
631001 NULL
641002 NULL
651003 NULL
661004 NULL
671006 NULL
681007 NULL
691008 NULL
701009 NULL
711010 NULL
72DROP TABLE federated.t2;
73DROP TABLE federated.t1;
74DROP TABLE federated.t1;
75SET @@GLOBAL.CONCURRENT_INSERT= @OLD_MASTER_CONCURRENT_INSERT;
76SET @@GLOBAL.CONCURRENT_INSERT= @OLD_SLAVE_CONCURRENT_INSERT;
77DROP TABLE IF EXISTS federated.t1;
78DROP DATABASE federated;
79DROP TABLE IF EXISTS federated.t1;
80DROP DATABASE federated;
810
=== added file 'Percona-Server/mysql-test/suite/federated/federated_bug_68354.test'
--- Percona-Server/mysql-test/suite/federated/federated_bug_68354.test 1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/federated/federated_bug_68354.test 2013-06-25 17:39:44 +0000
@@ -0,0 +1,79 @@
1# Bug 1182572 68354: Server crashes on update/join FEDERATED + local table when
2# only 1 local row.
3#
4# When updating a federated table with UPDATE ... JOIN, the server consistently
5# crashes with Signal 11 when only 1 row exists in the local table involved in
6# the join and that 1 row can be joined with a row in the federated table.
7#
8
9--source suite/federated/include/federated.inc
10
11connection default;
12
13# Disable concurrent inserts to avoid test failures when reading
14# data from concurrent connections (insert might return before
15# the data is actually in the table).
16SET @OLD_MASTER_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
17SET @@GLOBAL.CONCURRENT_INSERT= 0;
18
19connection slave;
20SET @OLD_SLAVE_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
21SET @@GLOBAL.CONCURRENT_INSERT= 0;
22DROP TABLE IF EXISTS federated.t1;
23CREATE TABLE federated.t1 (
24 `id` int(20) NOT NULL,
25 `dummy` int(20) DEFAULT NULL,
26 PRIMARY KEY (`id`))
27 ENGINE="MyISAM"
28 DEFAULT CHARSET=latin1;
29
30INSERT INTO federated.t1 (`id`) VALUES
31 (1001), (1002), (1003), (1004), (1005),
32 (1006), (1007), (1008), (1009), (1010);
33
34connection master;
35--replace_result $SLAVE_MYPORT SLAVE_PORT
36eval CREATE TABLE federated.t1 (
37 `id` int(20) NOT NULL,
38 `dummy` int(20) DEFAULT NULL,
39 PRIMARY KEY (`id`))
40 ENGINE="FEDERATED"
41 DEFAULT CHARSET=latin1
42 CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/t1';
43
44CREATE TABLE federated.t2 (
45 `id` int(20) NOT NULL,
46 `dummy` int(20) NOT NULL,
47 PRIMARY KEY (`id`))
48 ENGINE="MyISAM"
49 DEFAULT CHARSET=latin1;
50
51INSERT INTO federated.t2 (`id`, `dummy`) VALUES (1005, 5);
52
53SELECT * FROM federated.t1;
54
55# master would crash here before bug fix
56UPDATE federated.t1 ft1 INNER JOIN federated.t2 ft2 ON ft1.id = ft2.id SET ft1.dummy = ft2.dummy WHERE ft1.dummy IS NULL;
57
58# sanity test to make sure correct rows are being returned and server doesn't
59# also crash during DELETE
60SELECT * FROM federated.t1;
61DELETE FROM federated.t1 WHERE id = 1005;
62SELECT * FROM federated.t1 WHERE id = 1005;
63
64SELECT * FROM federated.t1;
65
66DROP TABLE federated.t2;
67DROP TABLE federated.t1;
68
69connection slave;
70DROP TABLE federated.t1;
71
72connection default;
73
74SET @@GLOBAL.CONCURRENT_INSERT= @OLD_MASTER_CONCURRENT_INSERT;
75connection slave;
76SET @@GLOBAL.CONCURRENT_INSERT= @OLD_SLAVE_CONCURRENT_INSERT;
77
78connection default;
79--source suite/federated/include/federated_cleanup.inc
080
=== removed file 'Percona-Server/mysql-test/suite/federated/federated_bug_68354.test'
--- Percona-Server/mysql-test/suite/federated/federated_bug_68354.test 2013-06-18 18:39:42 +0000
+++ Percona-Server/mysql-test/suite/federated/federated_bug_68354.test 1970-01-01 00:00:00 +0000
@@ -1,79 +0,0 @@
1# Bug 1182572 68354: Server crashes on update/join FEDERATED + local table when
2# only 1 local row.
3#
4# When updating a federated table with UPDATE ... JOIN, the server consistently
5# crashes with Signal 11 when only 1 row exists in the local table involved in
6# the join and that 1 row can be joined with a row in the federated table.
7#
8
9--source suite/federated/include/federated.inc
10
11connection default;
12
13# Disable concurrent inserts to avoid test failures when reading
14# data from concurrent connections (insert might return before
15# the data is actually in the table).
16SET @OLD_MASTER_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
17SET @@GLOBAL.CONCURRENT_INSERT= 0;
18
19connection slave;
20SET @OLD_SLAVE_CONCURRENT_INSERT= @@GLOBAL.CONCURRENT_INSERT;
21SET @@GLOBAL.CONCURRENT_INSERT= 0;
22DROP TABLE IF EXISTS federated.t1;
23CREATE TABLE federated.t1 (
24 `id` int(20) NOT NULL,
25 `dummy` int(20) DEFAULT NULL,
26 PRIMARY KEY (`id`))
27 ENGINE="MyISAM"
28 DEFAULT CHARSET=latin1;
29
30INSERT INTO federated.t1 (`id`) VALUES
31 (1001), (1002), (1003), (1004), (1005),
32 (1006), (1007), (1008), (1009), (1010);
33
34connection master;
35--replace_result $SLAVE_MYPORT SLAVE_PORT
36eval CREATE TABLE federated.t1 (
37 `id` int(20) NOT NULL,
38 `dummy` int(20) DEFAULT NULL,
39 PRIMARY KEY (`id`))
40 ENGINE="FEDERATED"
41 DEFAULT CHARSET=latin1
42 CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/t1';
43
44CREATE TABLE federated.t2 (
45 `id` int(20) NOT NULL,
46 `dummy` int(20) NOT NULL,
47 PRIMARY KEY (`id`))
48 ENGINE="MyISAM"
49 DEFAULT CHARSET=latin1;
50
51INSERT INTO federated.t2 (`id`, `dummy`) VALUES (1005, 5);
52
53SELECT * FROM federated.t1;
54
55# master would crash here before bug fix
56UPDATE federated.t1 ft1 INNER JOIN federated.t2 ft2 ON ft1.id = ft2.id SET ft1.dummy = ft2.dummy WHERE ft1.dummy IS NULL;
57
58# sanity test to make sure correct rows are being returned and server doesn't
59# also crash during DELETE
60SELECT * FROM federated.t1;
61DELETE FROM federated.t1 WHERE id = 1005;
62SELECT * FROM federated.t1 WHERE id = 1005;
63
64SELECT * FROM federated.t1;
65
66DROP TABLE federated.t2;
67DROP TABLE federated.t1;
68
69connection slave;
70DROP TABLE federated.t1;
71
72connection default;
73
74SET @@GLOBAL.CONCURRENT_INSERT= @OLD_MASTER_CONCURRENT_INSERT;
75connection slave;
76SET @@GLOBAL.CONCURRENT_INSERT= @OLD_SLAVE_CONCURRENT_INSERT;
77
78connection default;
79--source suite/federated/include/federated_cleanup.inc
800
=== modified file 'Percona-Server/storage/federated/ha_federated.cc'
--- Percona-Server/storage/federated/ha_federated.cc 2013-06-20 15:16:00 +0000
+++ Percona-Server/storage/federated/ha_federated.cc 2013-06-25 17:39:44 +0000
@@ -2738,7 +2738,6 @@
2738 /* Store data cursor position. */2738 /* Store data cursor position. */
2739 memcpy(ref + sizeof(MYSQL_RES *), &current_position,2739 memcpy(ref + sizeof(MYSQL_RES *), &current_position,
2740 sizeof(MYSQL_ROW_OFFSET));2740 sizeof(MYSQL_ROW_OFFSET));
2741
2742 position_called= true;2741 position_called= true;
2743 DBUG_VOID_RETURN;2742 DBUG_VOID_RETURN;
2744}2743}

Subscribers

People subscribed via source and target branches