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

Proposed by Laurynas Biveinis on 2012-08-18
Status: Merged
Approved by: Stewart Smith on 2012-08-20
Approved revision: 459
Merged at revision: 465
Proposed branch: lp:~laurynas-biveinis/percona-server/bug905334-5.1
Merge into: lp:percona-server/5.1
Diff against target: 69 lines (+23/-12)
3 files modified
Percona-Server/storage/innodb_plugin/buf/buf0buf.c (+13/-0)
Percona-Server/storage/innodb_plugin/buf/buf0lru.c (+5/-6)
Percona-Server/storage/innodb_plugin/include/buf0lru.h (+5/-6)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bug905334-5.1
Reviewer Review Type Date Requested Status
Stewart Smith (community) 2012-08-18 Approve on 2012-08-20
Review via email: mp+120252@code.launchpad.net

Description of the change

Fix bug 905334 (Intermittent innodb_bug56680 crash).

The cause is that the buffer pool mutex split added a temporary buffer
block mutex unlock to buf_LRU_free_block() but one its call sites in
buf_page_get_gen() failed to account for the possibility of block
pointing to another page after buf_LRU_free_block() returns due to
this unlock.

Fixed by checking that we indeed have the same page as before the
buf_LRU_free_block() call and retrying the read if that's not the
case.

Adjusted the buf_LRU_free_block() header comment to reflect the
reality.

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

To post a comment you must log in.
459. By Laurynas Biveinis on 2012-08-19

Fix bug 905334 (Intermittent innodb_bug56680 crash).

The cause is that the buffer pool mutex split added a temporary buffer
block mutex unlock to buf_LRU_free_block() but one its call sites in
buf_page_get_gen() failed to account for the possibility of block
pointing to another page after buf_LRU_free_block() returns due to
this unlock.

Fixed by checking that we indeed have the same page as before the
buf_LRU_free_block() call and retrying the read if that's not the
case.

Adjusted the buf_LRU_free_block() header comment to reflect the
reality.

Stewart Smith (stewart) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/storage/innodb_plugin/buf/buf0buf.c'
2--- Percona-Server/storage/innodb_plugin/buf/buf0buf.c 2012-05-10 08:11:45 +0000
3+++ Percona-Server/storage/innodb_plugin/buf/buf0buf.c 2012-08-19 15:19:26 +0000
4@@ -2131,6 +2131,19 @@
5 "innodb_change_buffering_debug evict %u %u\n",
6 (unsigned) space, (unsigned) offset);
7 return(NULL);
8+ } else if (UNIV_UNLIKELY(buf_block_get_page_no(block)
9+ != page_no
10+ || buf_block_get_space(block) != space
11+ || (buf_block_get_state(block)
12+ != BUF_BLOCK_FILE_PAGE))) {
13+
14+ /* buf_LRU_free_block temporarily releases the
15+ block mutex, and now block points to something
16+ else. */
17+ mutex_exit(block_mutex);
18+ block = NULL;
19+ goto loop2;
20+
21 } else if (buf_flush_page_try(block)) {
22 fprintf(stderr,
23 "innodb_change_buffering_debug flush %u %u\n",
24
25=== modified file 'Percona-Server/storage/innodb_plugin/buf/buf0lru.c'
26--- Percona-Server/storage/innodb_plugin/buf/buf0lru.c 2012-08-10 18:58:06 +0000
27+++ Percona-Server/storage/innodb_plugin/buf/buf0lru.c 2012-08-19 15:19:26 +0000
28@@ -1429,13 +1429,12 @@
29 Try to free a block. If bpage is a descriptor of a compressed-only
30 page, the descriptor object will be freed as well.
31
32-NOTE: If this function returns TRUE, it will temporarily
33-release buf_pool_mutex. Furthermore, the page frame will no longer be
34-accessible via bpage.
35+NOTE: This will temporarily release buf_pool_mutex. Furthermore, the
36+page frame will no longer be accessible via bpage.
37
38-The caller must hold buf_pool_mutex and buf_page_get_mutex(bpage) and
39-release these two mutexes after the call. No other
40-buf_page_get_mutex() may be held when calling this function.
41+The caller must hold buf_page_get_mutex(bpage) and release this mutex
42+after the call. No other buf_page_get_mutex() may be held when
43+calling this function.
44 @return TRUE if freed, FALSE otherwise. */
45 UNIV_INTERN
46 ibool
47
48=== modified file 'Percona-Server/storage/innodb_plugin/include/buf0lru.h'
49--- Percona-Server/storage/innodb_plugin/include/buf0lru.h 2011-11-24 02:00:47 +0000
50+++ Percona-Server/storage/innodb_plugin/include/buf0lru.h 2012-08-19 15:19:26 +0000
51@@ -93,13 +93,12 @@
52 Try to free a block. If bpage is a descriptor of a compressed-only
53 page, the descriptor object will be freed as well.
54
55-NOTE: If this function returns TRUE, it will temporarily
56-release buf_pool_mutex. Furthermore, the page frame will no longer be
57-accessible via bpage.
58+NOTE: This will temporarily release buf_pool_mutex. Furthermore, the
59+page frame will no longer be accessible via bpage.
60
61-The caller must hold buf_pool_mutex and buf_page_get_mutex(bpage) and
62-release these two mutexes after the call. No other
63-buf_page_get_mutex() may be held when calling this function.
64+The caller must hold buf_page_get_mutex(bpage) and release this mutex
65+after the call. No other buf_page_get_mutex() may be held when
66+calling this function.
67 @return TRUE if freed, FALSE otherwise. */
68 UNIV_INTERN
69 ibool

Subscribers

People subscribed via source and target branches