Merge lp:~laurynas-biveinis/percona-server/bug1191580-1191589-5.1 into lp:percona-server/5.1

Proposed by Laurynas Biveinis on 2013-09-25
Status: Merged
Approved by: Vlad Lesin on 2013-09-26
Approved revision: 579
Merged at revision: 591
Proposed branch: lp:~laurynas-biveinis/percona-server/bug1191580-1191589-5.1
Merge into: lp:percona-server/5.1
Prerequisite: lp:~laurynas-biveinis/percona-server/bug1217002-5.1
Diff against target: 158 lines (+73/-7)
4 files modified
Percona-Server/mysql-test/suite/innodb_plugin/r/percona_changed_page_bmp_debug.result (+6/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/percona_changed_page_bmp_debug.test (+31/-0)
Percona-Server/storage/innodb_plugin/handler/i_s.cc (+2/-0)
Percona-Server/storage/innodb_plugin/log/log0online.c (+34/-7)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bug1191580-1191589-5.1
Reviewer Review Type Date Requested Status
Vlad Lesin (community) g2 2013-09-25 Approve on 2013-09-26
Registry Administrators 2013-09-25 Pending
Review via email: mp+187456@code.launchpad.net

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

Description of the change

No BT or ST, 5.6 QA blocker.

2nd MP:

http://jenkins.percona.com/job/percona-server-5.1-param/577/

Changes from the 1st MP:
- Addressed schema_store_table_record() comment.
- Merged bug 1204075 change from XtraBackup.

1st MP:

http://jenkins.percona.com/job/percona-server-5.1-param/574/

Fix
- bug 1191580 (InnoDB: Failing assertion:
  bitmap_files->files[0].seq_num == first_file_seq_num in file
  log0online.cc line 1423 | abort in
  log_online_setup_bitmap_file_range);
- bug 1191589 (mysqld-debug: .../sql/protocol.cc:518: void
  Protocol::end_statement(): Assertion `0' failed on a
  INFORMATION_SCHEMA.INNODB_CHANGED_PAGES query).

The issue is bug 1191580 is a race condition between the two bitmap
file enumeration passes in log_online_setup_bitmap_file_range(). If
the 1st bitmap file in the range is deleted at that point, then the
debug build will crash as reported on this bug. For release builds
this is not an issue as the missing bitmap file will be detected and
diagnosed on an attempt to open it, as fixed by bug 1179974. The
current bug shows that the fix for the latter was incomplete.

Fixed by adjusting the range check code in
log_online_setup_bitmap_file_range() to check for the above condition,
print diagnostics, and return range setup error. Since these
diagnostics are now printed in two places instead of one, pull it out
to a new static function log_online_diagnose_inconsistent_dir().
Adjust it to print "Warning" instead of "Error" as it's not a fatal
issue.

At the same time fix bug 1191589. The issue there is that on
log_online_bitmap_iterator_init() failure (such as bug 1191580), the
error is not returned to client properly. Fixed by returning
ER_CANT_FIND_SYSTEM_REC. At the same time replace
schema_table_store_record() return value check with an OK() assert to
avoid having to return some error in the case of its failure as well.

Add a testcase, common for both bugs, to
percona_changed_page_bmp_debug.test.

To post a comment you must log in.
Vlad Lesin (vlad-lesin) wrote : Posted in a previous version of this proposal

This change

@@ -4029,11 +4030,7 @@
                if (cond && !cond->val_int())
                        continue;

- if (schema_table_store_record(thd, table))
- {
- log_online_bitmap_iterator_release(&i);
- DBUG_RETURN(1);
- }
+ OK(schema_table_store_record(thd, table));

                ++output_rows_num;
        }

should be reverted because if schema_table_store_record() fails (for example there is no free space for MyISAM temp table) there will be memory leak as heap memory allocation was done during initialization of "i" iterator.

review: Needs Fixing (g2)
Vlad Lesin (vlad-lesin) :
review: Approve (g2)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/mysql-test/suite/innodb_plugin/r/percona_changed_page_bmp_debug.result'
2--- Percona-Server/mysql-test/suite/innodb_plugin/r/percona_changed_page_bmp_debug.result 2013-09-09 13:09:38 +0000
3+++ Percona-Server/mysql-test/suite/innodb_plugin/r/percona_changed_page_bmp_debug.result 2013-09-25 07:59:41 +0000
4@@ -33,5 +33,11 @@
5 SET @@GLOBAL.innodb_track_changed_pages=TRUE;
6 SET @@GLOBAL.innodb_track_redo_log_now=TRUE;
7 SET DEBUG_SYNC="now SIGNAL finish_alter_table";
8+SET DEBUG_SYNC="setup_bitmap_range_middle SIGNAL changed_pages_query_ready WAIT_FOR finish_changed_pages_query";
9+SELECT COUNT(*) FROM INFORMATION_SCHEMA.INNODB_CHANGED_PAGES;
10+SET DEBUG_SYNC="now WAIT_FOR changed_pages_query_ready";
11+call mtr.add_suppression("InnoDB: Warning: inconsistent bitmap file directory");
12+SET DEBUG_SYNC="now SIGNAL finish_changed_pages_query";
13+ERROR HY000: Can't read record in system table
14 SET DEBUG_SYNC="RESET";
15 DROP TABLE t2;
16
17=== modified file 'Percona-Server/mysql-test/suite/innodb_plugin/t/percona_changed_page_bmp_debug.test'
18--- Percona-Server/mysql-test/suite/innodb_plugin/t/percona_changed_page_bmp_debug.test 2013-09-09 13:09:38 +0000
19+++ Percona-Server/mysql-test/suite/innodb_plugin/t/percona_changed_page_bmp_debug.test 2013-09-25 07:59:41 +0000
20@@ -93,6 +93,37 @@
21
22 --connection default
23 reap;
24+
25+#
26+# Test for
27+# - bug 1191580 (InnoDB: Failing assertion: bitmap_files->files[0].seq_num
28+# == first_file_seq_num in file log0online.cc line 1423 | abort in
29+# log_online_setup_bitmap_file_range)
30+# - bug 1191589 (mysqld-debug: .../sql/protocol.cc:518: void
31+# Protocol::end_statement(): Assertion `0' failed on a
32+# INFORMATION_SCHEMA.INNODB_CHANGED_PAGES query)
33+#
34+
35+SET DEBUG_SYNC="setup_bitmap_range_middle SIGNAL changed_pages_query_ready WAIT_FOR finish_changed_pages_query";
36+
37+send SELECT COUNT(*) FROM INFORMATION_SCHEMA.INNODB_CHANGED_PAGES;
38+
39+--connection con2
40+
41+SET DEBUG_SYNC="now WAIT_FOR changed_pages_query_ready";
42+
43+# Remove the first bitmap file in the range after it has been enumerated
44+remove_files_wildcard $MYSQLD_DATADIR ib_modified_log_1_*.xdb;
45+
46+call mtr.add_suppression("InnoDB: Warning: inconsistent bitmap file directory");
47+
48+SET DEBUG_SYNC="now SIGNAL finish_changed_pages_query";
49+
50+--connection default
51+
52+--error ER_CANT_FIND_SYSTEM_REC
53+reap;
54+
55 SET DEBUG_SYNC="RESET";
56
57 disconnect con2;
58
59=== modified file 'Percona-Server/storage/innodb_plugin/handler/i_s.cc'
60--- Percona-Server/storage/innodb_plugin/handler/i_s.cc 2013-08-05 13:11:52 +0000
61+++ Percona-Server/storage/innodb_plugin/handler/i_s.cc 2013-09-25 07:59:41 +0000
62@@ -3969,6 +3969,7 @@
63 }
64
65 if (!log_online_bitmap_iterator_init(&i, min_lsn, max_lsn)) {
66+ my_error(ER_CANT_FIND_SYSTEM_REC, MYF(0));
67 DBUG_RETURN(1);
68 }
69
70@@ -4032,6 +4033,7 @@
71 if (schema_table_store_record(thd, table))
72 {
73 log_online_bitmap_iterator_release(&i);
74+ my_error(ER_CANT_FIND_SYSTEM_REC, MYF(0));
75 DBUG_RETURN(1);
76 }
77
78
79=== modified file 'Percona-Server/storage/innodb_plugin/log/log0online.c'
80--- Percona-Server/storage/innodb_plugin/log/log0online.c 2013-05-31 09:06:13 +0000
81+++ Percona-Server/storage/innodb_plugin/log/log0online.c 2013-09-25 07:59:41 +0000
82@@ -34,6 +34,14 @@
83 #include "trx0sys.h"
84 #include "ut0rbt.h"
85
86+#ifdef __WIN__
87+/* error LNK2001: unresolved external symbol _debug_sync_C_callback_ptr */
88+# define DEBUG_SYNC_C(dummy) ((void) 0)
89+#else
90+# include "m_string.h" /* for my_sys.h */
91+# include "my_sys.h" /* DEBUG_SYNC_C */
92+#endif
93+
94 enum { FOLLOW_SCAN_SIZE = 4 * (UNIV_PAGE_SIZE_MAX) };
95
96 /** Log parsing and bitmap output data structure */
97@@ -1248,6 +1256,24 @@
98 }
99
100 /*********************************************************************//**
101+Diagnose a bitmap file range setup failure and free the partially-initialized
102+bitmap file range. */
103+static
104+void
105+log_online_diagnose_inconsistent_dir(
106+/*=================================*/
107+ log_online_bitmap_file_range_t *bitmap_files) /*!<in/out: bitmap file
108+ range */
109+{
110+ fprintf(stderr,
111+ "InnoDB: Warning: inconsistent bitmap file "
112+ "directory for a "
113+ "INFORMATION_SCHEMA.INNODB_CHANGED_PAGES query"
114+ "\n");
115+ free(bitmap_files->files);
116+}
117+
118+/*********************************************************************//**
119 List the bitmap files in srv_data_home and setup their range that contains the
120 specified LSN interval. This range, if non-empty, will start with a file that
121 has the greatest LSN equal to or less than the start LSN and will include all
122@@ -1348,6 +1374,8 @@
123
124 bitmap_files->count = last_file_seq_num - first_file_seq_num + 1;
125
126+ DEBUG_SYNC_C("setup_bitmap_range_middle");
127+
128 /* 2nd pass: get the file names in the file_seq_num order */
129
130 bitmap_dir = os_file_opendir(srv_data_home, FALSE);
131@@ -1383,12 +1411,7 @@
132 array_pos = file_seq_num - first_file_seq_num;
133 if (UNIV_UNLIKELY(array_pos >= bitmap_files->count)) {
134
135- fprintf(stderr,
136- "InnoDB: Error: inconsistent bitmap file "
137- "directory for a "
138- "INFORMATION_SCHEMA.INNODB_CHANGED_PAGES query"
139- "\n");
140- free(bitmap_files->files);
141+ log_online_diagnose_inconsistent_dir(bitmap_files);
142 return FALSE;
143 }
144
145@@ -1415,8 +1438,12 @@
146 }
147
148 #ifdef UNIV_DEBUG
149+ if (!bitmap_files->files[0].seq_num) {
150+
151+ log_online_diagnose_inconsistent_dir(bitmap_files);
152+ return FALSE;
153+ }
154 ut_ad(bitmap_files->files[0].seq_num == first_file_seq_num);
155- ut_ad(bitmap_files->files[0].start_lsn == first_file_start_lsn);
156 {
157 size_t i;
158 for (i = 1; i < bitmap_files->count; i++) {

Subscribers

People subscribed via source and target branches