Merge lp:~stewart/percona-server/bug758788 into lp:percona-server/5.1

Proposed by Stewart Smith on 2011-12-01
Status: Work in progress
Proposed branch: lp:~stewart/percona-server/bug758788
Merge into: lp:percona-server/5.1
Diff against target: 306 lines (+123/-11)
16 files modified
Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_basic.result (+9/-0)
Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_evict.result (+6/-0)
Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_small.result (+9/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic-master.opt (+1/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic.test (+9/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict-master.opt (+1/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict.test (+34/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small-master.opt (+1/-0)
Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small.test (+8/-0)
Percona-Server/storage/innodb_plugin/dict/dict0dict.c (+5/-2)
Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc (+15/-1)
Percona-Server/storage/innodb_plugin/include/dict0dict.h (+1/-0)
Percona-Server/storage/innodb_plugin/include/srv0srv.h (+1/-0)
Percona-Server/storage/innodb_plugin/row/row0ins.c (+21/-7)
Percona-Server/storage/innodb_plugin/srv/srv0srv.c (+1/-0)
Percona-Server/storage/innodb_plugin/sync/sync0rw.c (+1/-1)
To merge this branch: bzr merge lp:~stewart/percona-server/bug758788
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2011-12-01 Needs Fixing on 2011-12-01
Review via email: mp+84066@code.launchpad.net

Description of the change

fixes crash when innodb_dict_size is set (now our various test cases with dbqp pass) as well as adds a counter for the number of cache evictions that have occurred. It also adds test cases, one of which must be --big-test as it works on 30,000 tables.

http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-param/204/

To post a comment you must log in.
Alexey Kopytov (akopytov) wrote :

Hm, so the "fix" is just to raise the minimum value for innodb_dict_size to 100M to lower probability of a race condition. But people in the bug report say the crash is reproducible with 128M and 100M. So either we are fixing a different problem (i.e. the one found by dbpq and the original one are different), or 100M is not sufficient to avoid the crash?

Some minor comments on the patch:

- percona_innodb_dict_size_basic.reject was added by mistake?
- it looks like -master.opt don't really need --innodb. But need new
  lines :)
- I think the warning about innodb_dict_size_limit should be prefixed
  with "InnoDB:" or "InnoDB: Warning: ", just for consistency with
  other messages
- I don't understand the change in sync0rw.c. What is it for?

review: Needs Fixing
Alexey Kopytov (akopytov) wrote :

There's also a new status variable being added along with the bugfix. Should a separate bug or BP be filed so it's documented eventually?

Stewart Smith (stewart) wrote :

> There's also a new status variable being added along with the bugfix. Should a
> separate bug or BP be filed so it's documented eventually?

It makes writing a test case to check that evictions are taking place rather easy, so that's why I just included it here.

Stewart Smith (stewart) wrote :

> Hm, so the "fix" is just to raise the minimum value for innodb_dict_size to
> 100M to lower probability of a race condition. But people in the bug report
> say the crash is reproducible with 128M and 100M. So either we are fixing a
> different problem (i.e. the one found by dbpq and the original one are
> different), or 100M is not sufficient to avoid the crash?

There's a couple of fixes: some of which is the original Yasufumi fix and the next part is setting a minimum size (which is what we were hitting for some of the dbqp test).

> - percona_innodb_dict_size_basic.reject was added by mistake?

fixed.

> - it looks like -master.opt don't really need --innodb. But need new
> lines :)

fixed.

> - I think the warning about innodb_dict_size_limit should be prefixed
> with "InnoDB:" or "InnoDB: Warning: ", just for consistency with
> other messages

fixed.

lp:~stewart/percona-server/bug758788 updated on 2011-12-02
414. By Stewart Smith on 2011-12-02

remove percona_innodb_dict_size_basic.reject that was added by mistake

415. By Stewart Smith on 2011-12-02

fix up innodb_dict_size_limit tests to properly have the --soucre include/have_innodb_plugin.inc instead of --innodb in master.opt. Turns out that I'd mostly forgotten that MySQL doesn't run with InnoDB by default for any test.

416. By Stewart Smith on 2011-12-02

prepend 'InnoDB: ' to dict_size_limit warnings

Stewart Smith (stewart) wrote :

> - I don't understand the change in sync0rw.c. What is it for?

Originally from:
http://bazaar.launchpad.net/~percona-dev/percona-server/5.1-bug758788-trial2/revision/282

Which was for http://bugs.mysql.com/62692 ("Possible race condition at row_ins_check_foreign_constraints()") and presented as possible solution to one of the crashes.

Alexey Kopytov (akopytov) wrote :

On 02.12.11 4:26, Stewart Smith wrote:
>> Hm, so the "fix" is just to raise the minimum value for innodb_dict_size to
>> 100M to lower probability of a race condition. But people in the bug report
>> say the crash is reproducible with 128M and 100M. So either we are fixing a
>> different problem (i.e. the one found by dbpq and the original one are
>> different), or 100M is not sufficient to avoid the crash?
>
> There's a couple of fixes: some of which is the original Yasufumi fix and the next part is setting a minimum size (which is what we were hitting for some of the dbqp test).
>

But that means the problem found by dbqp is different from the one
reported as bug #758788, because otherwise people wound not see the
crash with dict_size_limit>=100M.

Which also means we don't really know if the other part of the fix
actually fixes the original problem, because we don't have a test case
for it. So it's a shot in the dark, which is probably the only available
option here. But the original bug reporter seems to be ready to build
from source to verify the fix in their environment (see comment #11).
Perhaps we can ask them to do so for this fix?

Alexey Kopytov (akopytov) wrote :

On 02.12.11 4:16, Stewart Smith wrote:
>> There's also a new status variable being added along with the bugfix. Should a
>> separate bug or BP be filed so it's documented eventually?
>
> It makes writing a test case to check that evictions are taking place rather easy, so that's why I just included it here.

Sure, I understand the purpose. My concern is that it will never be
documented. I guess it may be useful for users as well?

Unmerged revisions

416. By Stewart Smith on 2011-12-02

prepend 'InnoDB: ' to dict_size_limit warnings

415. By Stewart Smith on 2011-12-02

fix up innodb_dict_size_limit tests to properly have the --soucre include/have_innodb_plugin.inc instead of --innodb in master.opt. Turns out that I'd mostly forgotten that MySQL doesn't run with InnoDB by default for any test.

414. By Stewart Smith on 2011-12-02

remove percona_innodb_dict_size_basic.reject that was added by mistake

413. By Stewart Smith on 2011-11-30

add innodb_dict_evictions counter. This counts the number of evictions from the data dictionary cache.

412. By Stewart Smith on 2011-11-30

Add a minimal size for innodb_dict_size_limit (100MB) as this avoids a race condition when using index entry and removing index entry when the list is too small

411. By Stewart Smith on 2011-11-30

Apply bug758788 patch originally by Yasufumi. Aim is to prevent/stop crashes with innodb_dict_size set

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_basic.result'
2--- Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_basic.result 1970-01-01 00:00:00 +0000
3+++ Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_basic.result 2011-12-02 00:30:31 +0000
4@@ -0,0 +1,9 @@
5+show variables like 'innodb_dict_size_limit';
6+Variable_name Value
7+innodb_dict_size_limit 104857600
8+create table t1 (a int) engine=innodb;
9+insert into t1 values (1);
10+select '1';
11+1
12+1
13+drop table t1;
14
15=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_evict.result'
16--- Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_evict.result 1970-01-01 00:00:00 +0000
17+++ Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_evict.result 2011-12-02 00:30:31 +0000
18@@ -0,0 +1,6 @@
19+create tables
20+insert a row into tables
21+drop tables
22+SELECT VARIABLE_VALUE > 0 FROM INFORMATION_SCHEMA.GLOBAL_STATUS where VARIABLE_NAME='innodb_dict_evictions';
23+VARIABLE_VALUE > 0
24+1
25
26=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_small.result'
27--- Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_small.result 1970-01-01 00:00:00 +0000
28+++ Percona-Server/mysql-test/suite/innodb_plugin/r/innodb_dict_size_small.result 2011-12-02 00:30:31 +0000
29@@ -0,0 +1,9 @@
30+show variables like 'innodb_dict_size_limit';
31+Variable_name Value
32+innodb_dict_size_limit 104857600
33+create table t1 (a int) engine=innodb;
34+insert into t1 values (1);
35+select '1';
36+1
37+1
38+drop table t1;
39
40=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic-master.opt'
41--- Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic-master.opt 1970-01-01 00:00:00 +0000
42+++ Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic-master.opt 2011-12-02 00:30:31 +0000
43@@ -0,0 +1,1 @@
44+--innodb-dict-size-limit=100M
45\ No newline at end of file
46
47=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic.test'
48--- Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic.test 1970-01-01 00:00:00 +0000
49+++ Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_basic.test 2011-12-02 00:30:31 +0000
50@@ -0,0 +1,9 @@
51+-- source include/have_innodb_plugin.inc
52+show variables like 'innodb_dict_size_limit';
53+let $start_tables= `select VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS where VARIABLE_NAME='innodb_dict_tables'`;
54+create table t1 (a int) engine=innodb;
55+insert into t1 values (1);
56+let $end_tables= `select VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS where VARIABLE_NAME='innodb_dict_tables'`;
57+let $result= `select '$end_tables' - '$start_tables'`;
58+eval select '$result';
59+drop table t1;
60\ No newline at end of file
61
62=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict-master.opt'
63--- Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict-master.opt 1970-01-01 00:00:00 +0000
64+++ Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict-master.opt 2011-12-02 00:30:31 +0000
65@@ -0,0 +1,1 @@
66+--innodb-dict-size-limit=100M --innodb-flush-method=nosync --sync-frm=0 --innodb-flush-log-at-trx-commit=2 --innodb-data-file-path=ibdata1:100M:autoextend --innodb-autoextend-increment=100
67\ No newline at end of file
68
69=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict.test'
70--- Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict.test 1970-01-01 00:00:00 +0000
71+++ Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_evict.test 2011-12-02 00:30:31 +0000
72@@ -0,0 +1,34 @@
73+#this is a big test
74+--source include/big_test.inc
75+-- source include/have_innodb_plugin.inc
76+
77+--echo create tables
78+--disable_query_log
79+let $i = 30000;
80+while($i)
81+{
82+ eval create table t$i (a int primary key, b varchar(100) default null, c int default null, d int default null, e int default null, f varchar(1000) default null, g int default null, h int default null, i char(10) default null, j int default null, k int, l char(10), m int, n int, o int, p int, q int) engine=innodb;
83+ dec $i;
84+}
85+--enable_query_log
86+
87+--echo insert a row into tables
88+--disable_query_log
89+let $i = 30000;
90+while($i)
91+{
92+ eval insert into t$i (a) values(1);
93+ dec $i;
94+}
95+--enable_query_log
96+
97+--echo drop tables
98+--disable_query_log
99+let $i = 30000;
100+while($i)
101+{
102+ eval drop table t$i;
103+ dec $i;
104+}
105+--enable_query_log
106+SELECT VARIABLE_VALUE > 0 FROM INFORMATION_SCHEMA.GLOBAL_STATUS where VARIABLE_NAME='innodb_dict_evictions';
107\ No newline at end of file
108
109=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small-master.opt'
110--- Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small-master.opt 1970-01-01 00:00:00 +0000
111+++ Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small-master.opt 2011-12-02 00:30:31 +0000
112@@ -0,0 +1,1 @@
113+--innodb-dict-size-limit=1M --innodb
114\ No newline at end of file
115
116=== added file 'Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small.test'
117--- Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small.test 1970-01-01 00:00:00 +0000
118+++ Percona-Server/mysql-test/suite/innodb_plugin/t/innodb_dict_size_small.test 2011-12-02 00:30:31 +0000
119@@ -0,0 +1,8 @@
120+show variables like 'innodb_dict_size_limit';
121+let $start_tables= `select VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS where VARIABLE_NAME='innodb_dict_tables'`;
122+create table t1 (a int) engine=innodb;
123+insert into t1 values (1);
124+let $end_tables= `select VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS where VARIABLE_NAME='innodb_dict_tables'`;
125+let $result= `select '$end_tables' - '$start_tables'`;
126+eval select '$result';
127+drop table t1;
128\ No newline at end of file
129
130=== modified file 'Percona-Server/storage/innodb_plugin/dict/dict0dict.c'
131--- Percona-Server/storage/innodb_plugin/dict/dict0dict.c 2011-11-24 16:33:30 +0000
132+++ Percona-Server/storage/innodb_plugin/dict/dict0dict.c 2011-12-02 00:30:31 +0000
133@@ -692,6 +692,7 @@
134 / (DICT_POOL_PER_TABLE_HASH
135 * UNIV_WORD_SIZE));
136 dict_sys->size = 0;
137+ dict_sys->nr_evictions = 0;
138
139 UT_LIST_INIT(dict_sys->table_LRU);
140
141@@ -1276,13 +1277,13 @@
142 + dict_sys->size) > srv_dict_size_limit ) {
143 prev_table = UT_LIST_GET_PREV(table_LRU, table);
144
145- if (table == self || table->n_mysql_handles_opened || table->is_corrupt)
146+ if (table == self || table->n_mysql_handles_opened || table->n_foreign_key_checks_running || UT_LIST_GET_LEN(table->locks) || table->is_corrupt)
147 goto next_loop;
148
149 cached_foreign_tables = 0;
150 foreign = UT_LIST_GET_FIRST(table->foreign_list);
151 while (foreign != NULL) {
152- if (foreign->referenced_table)
153+ if (foreign->referenced_table && foreign->referenced_table != table)
154 cached_foreign_tables++;
155 foreign = UT_LIST_GET_NEXT(foreign_list, foreign);
156 }
157@@ -1302,6 +1303,8 @@
158 + dict_sys->table_id_hash->n_cells) * sizeof(hash_cell_t)
159 + dict_sys->size) > srv_dict_size_limit )
160 goto retry;
161+
162+ dict_sys->nr_evictions+= n_removed;
163 }
164
165 /****************************************************************//**
166
167=== modified file 'Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc'
168--- Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc 2011-11-24 16:33:30 +0000
169+++ Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc 2011-12-02 00:30:31 +0000
170@@ -584,6 +584,8 @@
171 (char*) &export_vars.innodb_dblwr_writes, SHOW_LONG},
172 {"dict_tables",
173 (char*) &export_vars.innodb_dict_tables, SHOW_LONG},
174+ {"dict_evictions",
175+ (char*) &export_vars.innodb_dict_evictions, SHOW_LONG},
176 {"have_atomic_builtins",
177 (char*) &export_vars.innodb_have_atomic_builtins, SHOW_BOOL},
178 {"log_waits",
179@@ -2191,6 +2193,17 @@
180 goto error;
181 }
182
183+ if (srv_dict_size_limit > 0
184+ && srv_dict_size_limit < 100*1024*1024) {
185+ fprintf(stderr,
186+ "InnoDB: If set, innodb_dict_size_limit "
187+ "must be >=100MB\n");
188+ fprintf(stderr,
189+ "InnoDB: Setting innodb_dict_size_limit "
190+ "to 100MB\n");
191+ srv_dict_size_limit= 100*1024*1024;
192+ }
193+
194 #ifndef MYSQL_SERVER
195 innodb_overwrite_relay_log_info = FALSE;
196 #endif
197@@ -11860,7 +11873,8 @@
198
199 static MYSQL_SYSVAR_ULONG(dict_size_limit, srv_dict_size_limit,
200 PLUGIN_VAR_RQCMDARG,
201- "Limit the allocated memory for dictionary cache. (0: unlimited)",
202+ "Limit the allocated memory for dictionary cache. (0: unlimited, if set,"
203+ " minimum of 100MB)",
204 NULL, NULL, 0, 0, LONG_MAX, 0);
205
206 static MYSQL_SYSVAR_UINT(auto_lru_dump, srv_auto_lru_dump,
207
208=== modified file 'Percona-Server/storage/innodb_plugin/include/dict0dict.h'
209--- Percona-Server/storage/innodb_plugin/include/dict0dict.h 2011-11-24 02:00:40 +0000
210+++ Percona-Server/storage/innodb_plugin/include/dict0dict.h 2011-12-02 00:30:31 +0000
211@@ -1179,6 +1179,7 @@
212 ulint size; /*!< varying space in bytes occupied
213 by the data dictionary table and
214 index objects */
215+ ulint nr_evictions; /*!< nr evictions from dict cache */
216 dict_table_t* sys_tables; /*!< SYS_TABLES table */
217 dict_table_t* sys_columns; /*!< SYS_COLUMNS table */
218 dict_table_t* sys_indexes; /*!< SYS_INDEXES table */
219
220=== modified file 'Percona-Server/storage/innodb_plugin/include/srv0srv.h'
221--- Percona-Server/storage/innodb_plugin/include/srv0srv.h 2011-11-24 02:01:25 +0000
222+++ Percona-Server/storage/innodb_plugin/include/srv0srv.h 2011-12-02 00:30:31 +0000
223@@ -682,6 +682,7 @@
224 ulint innodb_data_written; /*!< Data bytes written */
225 ulint innodb_data_reads; /*!< I/O read requests */
226 ulint innodb_dict_tables;
227+ ulint innodb_dict_evictions; /*!< Nr dict cache evictions */
228 ulint innodb_buffer_pool_pages_total; /*!< Buffer pool size */
229 ulint innodb_buffer_pool_pages_data; /*!< Data pages */
230 ulint innodb_buffer_pool_pages_dirty; /*!< Dirty data pages */
231
232=== modified file 'Percona-Server/storage/innodb_plugin/row/row0ins.c'
233--- Percona-Server/storage/innodb_plugin/row/row0ins.c 2011-11-24 16:33:30 +0000
234+++ Percona-Server/storage/innodb_plugin/row/row0ins.c 2011-12-02 00:30:31 +0000
235@@ -1550,25 +1550,39 @@
236
237 while (foreign) {
238 if (foreign->foreign_index == index) {
239-
240+ dict_table_t* referenced_table;
241+
242+ /* FIRST-AID for for http://bugs.mysql.com/62221. freeze before foreign->referenced_table */
243+ if (0 == trx->dict_operation_lock_mode) {
244+ got_s_lock = TRUE;
245+
246+ row_mysql_freeze_data_dictionary(trx);
247+ }
248+retry:
249+ /* access to foreign->referenced_table might be optimistic still */
250 if (foreign->referenced_table == NULL) {
251+ referenced_table =
252 dict_table_get(foreign->referenced_table_name,
253 FALSE);
254- }
255-
256- if (0 == trx->dict_operation_lock_mode) {
257- got_s_lock = TRUE;
258-
259- row_mysql_freeze_data_dictionary(trx);
260+ } else {
261+ referenced_table = foreign->referenced_table;
262 }
263
264 if (foreign->referenced_table) {
265 mutex_enter(&(dict_sys->mutex));
266
267+ if (!foreign->referenced_table) {
268+ mutex_exit(&(dict_sys->mutex));
269+ goto retry;
270+ }
271+
272 (foreign->referenced_table
273 ->n_foreign_key_checks_running)++;
274
275 mutex_exit(&(dict_sys->mutex));
276+ } else if (referenced_table) {
277+ /* retry only when the previous dict_table_get() seems exist */
278+ goto retry;
279 }
280
281 /* NOTE that if the thread ends up waiting for a lock
282
283=== modified file 'Percona-Server/storage/innodb_plugin/srv/srv0srv.c'
284--- Percona-Server/storage/innodb_plugin/srv/srv0srv.c 2011-11-24 02:01:25 +0000
285+++ Percona-Server/storage/innodb_plugin/srv/srv0srv.c 2011-12-02 00:30:31 +0000
286@@ -2156,6 +2156,7 @@
287 export_vars.innodb_data_writes = os_n_file_writes;
288 export_vars.innodb_data_written = srv_data_written;
289 export_vars.innodb_dict_tables= (dict_sys ? UT_LIST_GET_LEN(dict_sys->table_LRU) : 0);
290+ export_vars.innodb_dict_evictions= (dict_sys ? dict_sys->nr_evictions : 0);
291 export_vars.innodb_buffer_pool_read_requests = buf_pool->stat.n_page_gets;
292 export_vars.innodb_buffer_pool_write_requests
293 = srv_buf_pool_write_requests;
294
295=== modified file 'Percona-Server/storage/innodb_plugin/sync/sync0rw.c'
296--- Percona-Server/storage/innodb_plugin/sync/sync0rw.c 2011-11-24 16:33:30 +0000
297+++ Percona-Server/storage/innodb_plugin/sync/sync0rw.c 2011-12-02 00:30:31 +0000
298@@ -304,7 +304,7 @@
299 rw_lock_t* lock) /*!< in: rw-lock */
300 {
301 ut_ad(rw_lock_validate(lock));
302- ut_a(lock->lock_word == X_LOCK_DECR);
303+ ut_a(rw_lock_lock_word_decr(lock, X_LOCK_DECR));
304
305 #ifndef INNODB_RW_LOCKS_USE_ATOMICS
306 mutex_free(rw_lock_get_mutex(lock));

Subscribers

People subscribed via source and target branches