From 95247de26028a23e34f12697f62076cf753c6084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 16 Aug 2012 17:31:23 +0300 Subject: [PATCH 1/3] Bug#13523839 ASSERTION FAILURES ON COMPRESSED INNODB TABLES btr_cur_optimistic_insert(): Remove a bogus assertion. The insert may fail after reorganizing the page. btr_cur_optimistic_update(): Do not attempt to reorganize compressed pages, because compression may fail after reorganization. page_copy_rec_list_start(): Use page_rec_get_nth() to restore to the ret_pos, which may also be the page infimum. rb:1221 --- storage/innodb_plugin/ChangeLog | 5 +++++ storage/innodb_plugin/btr/btr0cur.c | 15 ++++++++++++--- storage/innodb_plugin/page/page0page.c | 21 +++++++++++---------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 4eceaeaed0a..2c7828b15a0 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,8 @@ +2012-08-16 The InnoDB Team + + * btr/btr0cur.c, page/page0page.c: + Fix Bug#13523839 ASSERTION FAILURES ON COMPRESSED INNODB TABLES + 2012-08-07 The InnoDB Team * btr/btr0pcur.c, row/row0merge.c: diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index 223b976dea7..0416ce24b2b 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -1220,7 +1220,12 @@ fail_err: if (UNIV_UNLIKELY(reorg)) { ut_a(zip_size); - ut_a(*rec); + /* It's possible for rec to be NULL if the + page is compressed. This is because a + reorganized page may become incompressible. */ + if (!*rec) { + goto fail; + } } } @@ -1973,8 +1978,12 @@ any_extern: goto err_exit; } - max_size = old_rec_size - + page_get_max_insert_size_after_reorganize(page, 1); + /* We do not attempt to reorganize if the page is compressed. + This is because the page may fail to compress after reorganization. */ + max_size = page_zip + ? page_get_max_insert_size(page, 1) + : (old_rec_size + + page_get_max_insert_size_after_reorganize(page, 1)); if (!(((max_size >= BTR_CUR_PAGE_REORGANIZE_LIMIT) && (max_size >= new_rec_size)) diff --git a/storage/innodb_plugin/page/page0page.c b/storage/innodb_plugin/page/page0page.c index 7b72a22fd1c..108c3e0805c 100644 --- a/storage/innodb_plugin/page/page0page.c +++ b/storage/innodb_plugin/page/page0page.c @@ -780,12 +780,18 @@ page_copy_rec_list_start( if (UNIV_LIKELY_NULL(new_page_zip)) { mtr_set_log_mode(mtr, log_mode); + DBUG_EXECUTE_IF("page_copy_rec_list_start_compress_fail", + goto zip_reorganize;); + if (UNIV_UNLIKELY (!page_zip_compress(new_page_zip, new_page, index, mtr))) { + ulint ret_pos; +#ifndef DBUG_OFF +zip_reorganize: +#endif /* DBUG_OFF */ /* Before trying to reorganize the page, store the number of preceding records on the page. */ - ulint ret_pos - = page_rec_get_n_recs_before(ret); + ret_pos = page_rec_get_n_recs_before(ret); /* Before copying, "ret" was the predecessor of the predefined supremum record. If it was the predefined infimum record, then it would @@ -806,15 +812,10 @@ page_copy_rec_list_start( btr_blob_dbg_add(new_page, index, "copy_start_reorg_fail"); return(NULL); - } else { - /* The page was reorganized: - Seek to ret_pos. */ - ret = new_page + PAGE_NEW_INFIMUM; - - do { - ret = rec_get_next_ptr(ret, TRUE); - } while (--ret_pos); } + + /* The page was reorganized: Seek to ret_pos. */ + ret = page_rec_get_nth(new_page, ret_pos); } } From 6d7f6baa22c8e98fc0651c118e715cb27727a6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 16 Aug 2012 17:37:52 +0300 Subject: [PATCH 2/3] Bug#12845774 OPTIMISTIC INSERT/UPDATE USES WRONG HEURISTICS FOR COMPRESSED PAGE SIZE This was submitted as MySQL Bug 61456 and a patch provided by Facebook. This patch follows the same idea, but instead of adding a parameter to btr_cur_pessimistic_insert(), we simply remove the btr_cur_optimistic_insert() call there and add it to the only caller that needs it. btr_cur_pessimistic_insert(): Do not try btr_cur_optimistic_insert(). btr_insert_on_non_leaf_level_func(): Invoke btr_cur_optimistic_insert() before invoking btr_cur_pessimistic_insert(). btr_cur_pessimistic_update(): Clarify in a comment why it is not necessary to invoke btr_cur_optimistic_insert(). btr_root_raise_and_insert(): Assert that the root page is not empty. This could happen if a pessimistic insert (involving a split or merge) is performed without first attempting an optimistic (intra-page) insert. rb:1219 approved by Sunny Bains --- storage/innodb_plugin/ChangeLog | 6 ++++++ storage/innodb_plugin/btr/btr0btr.c | 21 +++++++++++++++------ storage/innodb_plugin/btr/btr0cur.c | 19 +++++-------------- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 2c7828b15a0..cbdefc84e19 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2012-08-16 The InnoDB Team + + * btr/btr0btr.c, btr/btr0cur.c: + Fix Bug#12845774 OPTIMISTIC INSERT/UPDATE USES WRONG HEURISTICS FOR + COMPRESSED PAGE SIZE + 2012-08-16 The InnoDB Team * btr/btr0cur.c, page/page0page.c: diff --git a/storage/innodb_plugin/btr/btr0btr.c b/storage/innodb_plugin/btr/btr0btr.c index 05723c26e2f..c042fb2ff87 100644 --- a/storage/innodb_plugin/btr/btr0btr.c +++ b/storage/innodb_plugin/btr/btr0btr.c @@ -1822,6 +1822,7 @@ btr_root_raise_and_insert( root = btr_cur_get_page(cursor); root_block = btr_cur_get_block(cursor); root_page_zip = buf_block_get_page_zip(root_block); + ut_ad(page_get_n_recs(root) > 0); #ifdef UNIV_ZIP_DEBUG ut_a(!root_page_zip || page_zip_validate(root_page_zip, root)); #endif /* UNIV_ZIP_DEBUG */ @@ -2302,12 +2303,20 @@ btr_insert_on_non_leaf_level_func( BTR_CONT_MODIFY_TREE, &cursor, 0, file, line, mtr); - err = btr_cur_pessimistic_insert(BTR_NO_LOCKING_FLAG - | BTR_KEEP_SYS_FLAG - | BTR_NO_UNDO_LOG_FLAG, - &cursor, tuple, &rec, - &dummy_big_rec, 0, NULL, mtr); - ut_a(err == DB_SUCCESS); + ut_ad(cursor.flag == BTR_CUR_BINARY); + + err = btr_cur_optimistic_insert( + BTR_NO_LOCKING_FLAG | BTR_KEEP_SYS_FLAG + | BTR_NO_UNDO_LOG_FLAG, &cursor, tuple, &rec, + &dummy_big_rec, 0, NULL, mtr); + + if (err == DB_FAIL) { + err = btr_cur_pessimistic_insert( + BTR_NO_LOCKING_FLAG | BTR_KEEP_SYS_FLAG + | BTR_NO_UNDO_LOG_FLAG, + &cursor, tuple, &rec, &dummy_big_rec, 0, NULL, mtr); + ut_a(err == DB_SUCCESS); + } } /**************************************************************//** diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index 0416ce24b2b..91cbba54308 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -1361,20 +1361,9 @@ btr_cur_pessimistic_insert( ut_ad(mtr_memo_contains(mtr, btr_cur_get_block(cursor), MTR_MEMO_PAGE_X_FIX)); - /* Try first an optimistic insert; reset the cursor flag: we do not - assume anything of how it was positioned */ - cursor->flag = BTR_CUR_BINARY; - err = btr_cur_optimistic_insert(flags, cursor, entry, rec, - big_rec, n_ext, thr, mtr); - if (err != DB_FAIL) { - - return(err); - } - - /* Retry with a pessimistic insert. Check locks and write to undo log, - if specified */ + /* Check locks and write to undo log, if specified */ err = btr_cur_ins_lock_and_undo(flags, cursor, entry, thr, mtr, &dummy_inh); @@ -2365,8 +2354,10 @@ make_external: record on its page? */ was_first = page_cur_is_before_first(page_cursor); - /* The first parameter means that no lock checking and undo logging - is made in the insert */ + /* Lock checks and undo logging were already performed by + btr_cur_upd_lock_and_undo(). We do not try + btr_cur_optimistic_insert() because + btr_cur_insert_if_possible() already failed above. */ err = btr_cur_pessimistic_insert(BTR_NO_UNDO_LOG_FLAG | BTR_NO_LOCKING_FLAG From e288e649c5157765f192bde3790e5a863807a112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 16 Aug 2012 17:45:39 +0300 Subject: [PATCH 3/3] Bug#12595091 POSSIBLY INVALID ASSERTION IN BTR_CUR_PESSIMISTIC_UPDATE() Facebook got a case where the page compresses really well so that btr_cur_optimistic_update() returns DB_UNDERFLOW, but when a record gets updated, the compression rate radically changes so that btr_cur_insert_if_possible() can not insert in place despite reorganizing/recompressing the page, leading to the assertion failing. rb:1220 approved by Sunny Bains --- storage/innodb_plugin/ChangeLog | 6 ++++++ storage/innodb_plugin/btr/btr0cur.c | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index cbdefc84e19..576a67a0106 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,9 @@ +2012-08-16 The InnoDB Team + + * btr/btr0cur.c: + Fix Bug#12595091 POSSIBLY INVALID ASSERTION IN + BTR_CUR_PESSIMISTIC_UPDATE() + 2012-08-16 The InnoDB Team * btr/btr0btr.c, btr/btr0cur.c: diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index 91cbba54308..8fb4366d894 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -2326,7 +2326,12 @@ make_external: err = DB_SUCCESS; goto return_after_reservations; } else { - ut_a(optim_err != DB_UNDERFLOW); + /* If the page is compressed and it initially + compresses very well, and there is a subsequent insert + of a badly-compressing record, it is possible for + btr_cur_optimistic_update() to return DB_UNDERFLOW and + btr_cur_insert_if_possible() to return FALSE. */ + ut_a(page_zip || optim_err != DB_UNDERFLOW); /* Out of space: reset the free bits. */ if (!dict_index_is_clust(index)