From aa14038f7c823d048df75e92586ba8f8dbbe0f95 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Tue, 3 Nov 2009 14:26:07 +0400 Subject: [PATCH] Applying InnoDB plugin snashot Detailed revision comments: r6110 | marko | 2009-10-29 12:44:57 +0200 (Thu, 29 Oct 2009) | 2 lines branches/zip: Makefile.am (INCLUDES): Merge a change from MySQL: Use $(srcdir)/include instead of $(top_srcdir)/storage/innobase/include. r6111 | marko | 2009-10-29 13:04:11 +0200 (Thu, 29 Oct 2009) | 33 lines branches/zip: Fix corruption of buf_pool->LRU_old and improve debug assertions. This was reported as Issue #381. buf_page_set_old(): Assert that blocks may only be set old if buf_pool->LRU_old is initialized and buf_pool->LRU_old_len is nonzero. Assert that buf_pool->LRU_old points to the block at the old/new boundary. buf_LRU_old_adjust_len(): Invoke buf_page_set_old() after adjusting buf_pool->LRU_old and buf_pool->LRU_old_len, in order not to violate the added assertions. buf_LRU_old_init(): Replace buf_page_set_old() with a direct assignment to bpage->old, because these loops that initialize all the blocks would temporarily violate the assertions about buf_pool->LRU_old. buf_LRU_remove_block(): When setting buf_pool->LRU_old = NULL, also clear all bpage->old flags and set buf_pool->LRU_old_len = 0. buf_LRU_add_block_to_end_low(), buf_LRU_add_block_low(): Move the buf_page_set_old() call later in order not to violate the debug assertions. If buf_pool->LRU_old is NULL, set old=FALSE. buf_LRU_free_block(): Replace the UNIV_LRU_DEBUG assertion with a dummy buf_page_set_old() call that performs more thorough checks. buf_LRU_validate(): Do not tolerate garbage in buf_pool->LRU_old_len even if buf_pool->LRU_old is NULL. Check that bpage->old is monotonic. buf_relocate(): Make the UNIV_LRU_DEBUG checks stricter. buf0buf.h: Revise the documentation of buf_page_t::old and buf_pool_t::LRU_old_len. --- storage/innodb_plugin/ChangeLog | 6 +++ storage/innodb_plugin/buf/buf0buf.c | 5 ++ storage/innodb_plugin/buf/buf0lru.c | 63 ++++++++++++++---------- storage/innodb_plugin/include/buf0buf.h | 5 +- storage/innodb_plugin/include/buf0buf.ic | 5 ++ 5 files changed, 56 insertions(+), 28 deletions(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 5e72b417a21..a6952f27c4d 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2009-10-29 The InnoDB Team + + * buf/buf0lru.c, buf/buf0buf.c, include/buf0buf.h, include/buf0buf.ic: + Fix corruption of the buf_pool->LRU_old list and improve debug + assertions. + 2009-10-26 The InnoDB Team * row/row0ins.c: diff --git a/storage/innodb_plugin/buf/buf0buf.c b/storage/innodb_plugin/buf/buf0buf.c index d87abbd0ed9..ff31457d200 100644 --- a/storage/innodb_plugin/buf/buf0buf.c +++ b/storage/innodb_plugin/buf/buf0buf.c @@ -1163,10 +1163,15 @@ buf_relocate( #ifdef UNIV_LRU_DEBUG /* buf_pool->LRU_old must be the first item in the LRU list whose "old" flag is set. */ + ut_a(buf_pool->LRU_old->old); ut_a(!UT_LIST_GET_PREV(LRU, buf_pool->LRU_old) || !UT_LIST_GET_PREV(LRU, buf_pool->LRU_old)->old); ut_a(!UT_LIST_GET_NEXT(LRU, buf_pool->LRU_old) || UT_LIST_GET_NEXT(LRU, buf_pool->LRU_old)->old); + } else { + /* Check that the "old" flag is consistent in + the block and its neighbours. */ + buf_page_set_old(dpage, buf_page_is_old(dpage)); #endif /* UNIV_LRU_DEBUG */ } diff --git a/storage/innodb_plugin/buf/buf0lru.c b/storage/innodb_plugin/buf/buf0lru.c index 329448a6206..4f19fd13fa5 100644 --- a/storage/innodb_plugin/buf/buf0lru.c +++ b/storage/innodb_plugin/buf/buf0lru.c @@ -978,14 +978,14 @@ buf_LRU_old_adjust_len(void) #ifdef UNIV_LRU_DEBUG ut_a(!LRU_old->old); #endif /* UNIV_LRU_DEBUG */ - buf_page_set_old(LRU_old, TRUE); old_len = ++buf_pool->LRU_old_len; + buf_page_set_old(LRU_old, TRUE); } else if (old_len > new_len + BUF_LRU_OLD_TOLERANCE) { - buf_page_set_old(LRU_old, FALSE); buf_pool->LRU_old = UT_LIST_GET_NEXT(LRU, LRU_old); old_len = --buf_pool->LRU_old_len; + buf_page_set_old(LRU_old, FALSE); } else { return; } @@ -1013,7 +1013,9 @@ buf_LRU_old_init(void) bpage = UT_LIST_GET_PREV(LRU, bpage)) { ut_ad(bpage->in_LRU_list); ut_ad(buf_page_in_file(bpage)); - buf_page_set_old(bpage, TRUE); + /* This loop temporarily violates the + assertions of buf_page_set_old(). */ + bpage->old = TRUE; } buf_pool->LRU_old = UT_LIST_GET_FIRST(buf_pool->LRU); @@ -1089,10 +1091,19 @@ buf_LRU_remove_block( buf_unzip_LRU_remove_block_if_needed(bpage); - /* If the LRU list is so short that LRU_old not defined, return */ + /* If the LRU list is so short that LRU_old is not defined, + clear the "old" flags and return */ if (UT_LIST_GET_LEN(buf_pool->LRU) < BUF_LRU_OLD_MIN_LEN) { + for (bpage = UT_LIST_GET_FIRST(buf_pool->LRU); bpage != NULL; + bpage = UT_LIST_GET_NEXT(LRU, bpage)) { + /* This loop temporarily violates the + assertions of buf_page_set_old(). */ + bpage->old = FALSE; + } + buf_pool->LRU_old = NULL; + buf_pool->LRU_old_len = 0; return; } @@ -1153,14 +1164,13 @@ buf_LRU_add_block_to_end_low( UT_LIST_ADD_LAST(LRU, buf_pool->LRU, bpage); ut_d(bpage->in_LRU_list = TRUE); - buf_page_set_old(bpage, TRUE); - if (UT_LIST_GET_LEN(buf_pool->LRU) > BUF_LRU_OLD_MIN_LEN) { ut_ad(buf_pool->LRU_old); /* Adjust the length of the old block list if necessary */ + buf_page_set_old(bpage, TRUE); buf_pool->LRU_old_len++; buf_LRU_old_adjust_len(); @@ -1169,8 +1179,9 @@ buf_LRU_add_block_to_end_low( /* The LRU list is now long enough for LRU_old to become defined: init it */ - buf_pool->LRU_old_len++; buf_LRU_old_init(); + } else { + buf_page_set_old(bpage, buf_pool->LRU_old != NULL); } /* If this is a zipped block with decompressed frame as well @@ -1221,14 +1232,13 @@ buf_LRU_add_block_low( ut_d(bpage->in_LRU_list = TRUE); - buf_page_set_old(bpage, old); - if (UT_LIST_GET_LEN(buf_pool->LRU) > BUF_LRU_OLD_MIN_LEN) { ut_ad(buf_pool->LRU_old); /* Adjust the length of the old block list if necessary */ + buf_page_set_old(bpage, old); buf_LRU_old_adjust_len(); } else if (UT_LIST_GET_LEN(buf_pool->LRU) == BUF_LRU_OLD_MIN_LEN) { @@ -1237,6 +1247,8 @@ buf_LRU_add_block_low( defined: init it */ buf_LRU_old_init(); + } else { + buf_page_set_old(bpage, buf_pool->LRU_old != NULL); } /* If this is a zipped block with decompressed frame as well @@ -1434,15 +1446,6 @@ alloc: buf_pool->LRU_old = b; } -#ifdef UNIV_LRU_DEBUG - ut_a(prev_b->old - || !UT_LIST_GET_NEXT(LRU, b) - || UT_LIST_GET_NEXT(LRU, b)->old); - } else { - ut_a(!prev_b->old - || !UT_LIST_GET_NEXT(LRU, b) - || !UT_LIST_GET_NEXT(LRU, b)->old); -#endif /* UNIV_LRU_DEBUG */ } lru_len = UT_LIST_GET_LEN(buf_pool->LRU); @@ -1458,6 +1461,11 @@ alloc: defined: init it */ buf_LRU_old_init(); } +#ifdef UNIV_LRU_DEBUG + /* Check that the "old" flag is consistent + in the block and its neighbours. */ + buf_page_set_old(b, buf_page_is_old(b)); +#endif /* UNIV_LRU_DEBUG */ } else { ut_d(b->in_LRU_list = FALSE); buf_LRU_add_block_low(b, buf_page_is_old(b)); @@ -1964,19 +1972,24 @@ buf_LRU_validate(void) } if (buf_page_is_old(bpage)) { - old_len++; - } + const buf_page_t* prev + = UT_LIST_GET_PREV(LRU, bpage); + const buf_page_t* next + = UT_LIST_GET_NEXT(LRU, bpage); - if (buf_pool->LRU_old && (old_len == 1)) { - ut_a(buf_pool->LRU_old == bpage); + if (!old_len++) { + ut_a(buf_pool->LRU_old == bpage); + } else { + ut_a(!prev || buf_page_is_old(prev)); + } + + ut_a(!next || buf_page_is_old(next)); } bpage = UT_LIST_GET_NEXT(LRU, bpage); } - if (buf_pool->LRU_old) { - ut_a(buf_pool->LRU_old_len == old_len); - } + ut_a(buf_pool->LRU_old_len == old_len); UT_LIST_VALIDATE(list, buf_page_t, buf_pool->free, ut_ad(ut_list_node_313->in_free_list)); diff --git a/storage/innodb_plugin/include/buf0buf.h b/storage/innodb_plugin/include/buf0buf.h index e9d95a14f1b..927ff893e39 100644 --- a/storage/innodb_plugin/include/buf0buf.h +++ b/storage/innodb_plugin/include/buf0buf.h @@ -1129,7 +1129,7 @@ struct buf_page_struct{ debugging */ #endif /* UNIV_DEBUG */ unsigned old:1; /*!< TRUE if the block is in the old - blocks in the LRU list */ + blocks in buf_pool->LRU_old */ unsigned freed_page_clock:31;/*!< the value of buf_pool->freed_page_clock when this block was the last @@ -1393,8 +1393,7 @@ struct buf_pool_struct{ the block to which LRU_old points onward, including that block; see buf0lru.c for the restrictions - on this value; not defined if - LRU_old == NULL; + on this value; 0 if LRU_old == NULL; NOTE: LRU_old_len must be adjusted whenever LRU_old shrinks or grows! */ diff --git a/storage/innodb_plugin/include/buf0buf.ic b/storage/innodb_plugin/include/buf0buf.ic index 465fcadc0bb..0f92a59a1c7 100644 --- a/storage/innodb_plugin/include/buf0buf.ic +++ b/storage/innodb_plugin/include/buf0buf.ic @@ -466,6 +466,10 @@ buf_page_set_old( ut_ad(bpage->in_LRU_list); #ifdef UNIV_LRU_DEBUG + ut_a((buf_pool->LRU_old_len == 0) == (buf_pool->LRU_old == NULL)); + /* If a block is flagged "old", the LRU_old list must exist. */ + ut_a(!old || buf_pool->LRU_old); + if (UT_LIST_GET_PREV(LRU, bpage) && UT_LIST_GET_NEXT(LRU, bpage)) { const buf_page_t* prev = UT_LIST_GET_PREV(LRU, bpage); const buf_page_t* next = UT_LIST_GET_NEXT(LRU, bpage); @@ -473,6 +477,7 @@ buf_page_set_old( ut_a(prev->old == old); } else { ut_a(!prev->old); + ut_a(buf_pool->LRU_old == (old ? bpage : next)); } } #endif /* UNIV_LRU_DEBUG */