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

+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:

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 */
 	}
 

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));

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! */
 

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 */