diff options
author | Thirunarayanan Balathandayuthapani <thiru@mariadb.com> | 2022-11-17 17:24:13 +0530 |
---|---|---|
committer | Thirunarayanan Balathandayuthapani <thiru@mariadb.com> | 2022-11-22 15:24:44 +0530 |
commit | 71c93fb8fd894e39b81b801a67ccb9de535b6ce8 (patch) | |
tree | 647e29cb7889df1276aa773d513d5449875edd05 | |
parent | f4a1298f245f678badc8a5b55571ca4f460718b1 (diff) | |
download | mariadb-git-71c93fb8fd894e39b81b801a67ccb9de535b6ce8.tar.gz |
MDEV-28462 Race condition between instant alter and AHI access
- InnoDB AHI tries to access the concurrent instant alter column,
leads to asan failure. Instant alter column should acquire the
clustered index search latch in exclusive mode before changing
the table cache definition.
- Removed the default parameter for the function
btr_search_drop_page_hash_index()
- Addressed the DWITH_INNODB_AHI=0 compilation failure
by passing two parameters from all callers of
btr_search_drop_page_hash_index()
-rw-r--r-- | storage/innobase/btr/btr0btr.cc | 16 | ||||
-rw-r--r-- | storage/innobase/btr/btr0defragment.cc | 2 | ||||
-rw-r--r-- | storage/innobase/btr/btr0sea.cc | 14 | ||||
-rw-r--r-- | storage/innobase/buf/buf0buf.cc | 2 | ||||
-rw-r--r-- | storage/innobase/buf/buf0lru.cc | 4 | ||||
-rw-r--r-- | storage/innobase/handler/handler0alter.cc | 12 | ||||
-rw-r--r-- | storage/innobase/include/btr0sea.h | 2 | ||||
-rw-r--r-- | storage/innobase/page/page0zip.cc | 2 |
8 files changed, 33 insertions, 21 deletions
diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc index 6b575885f81..82619531d5c 100644 --- a/storage/innobase/btr/btr0btr.cc +++ b/storage/innobase/btr/btr0btr.cc @@ -994,7 +994,7 @@ static void btr_free_root(buf_block_t* block, mtr_t* mtr, bool invalidate) | MTR_MEMO_PAGE_SX_FIX)); ut_ad(mtr->is_named_space(block->page.id.space())); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); header = buf_block_get_frame(block) + PAGE_HEADER + PAGE_BTR_SEG_TOP; #ifdef UNIV_BTR_DEBUG @@ -1496,7 +1496,7 @@ btr_page_reorganize_low( buf_frame_copy(temp_page, page); if (!recovery) { - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); } /* Save the cursor position. */ @@ -1785,7 +1785,7 @@ btr_page_empty( ut_a(!page_zip || page_zip_validate(page_zip, page, index)); #endif /* UNIV_ZIP_DEBUG */ - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); /* Recreate the page: note that global data on page (possible segment headers, next page-field, etc.) is preserved intact */ @@ -3410,7 +3410,7 @@ btr_lift_page_up( mem_heap_free(heap); } - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); /* Make the father empty */ btr_page_empty(father_block, father_page_zip, index, page_level, mtr); @@ -3728,7 +3728,7 @@ retry: goto err_exit; } - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); /* Remove the page from the level list */ if (DB_SUCCESS != btr_level_list_remove(index->table->space_id, @@ -3848,7 +3848,7 @@ retry: goto err_exit; } - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); #ifdef UNIV_BTR_DEBUG if (merge_page_zip && left_page_no == FIL_NULL) { @@ -4081,7 +4081,7 @@ btr_discard_only_page_on_level( ut_ad(fil_page_index_page_check(page)); ut_ad(block->page.id.space() == index->table->space->id); ut_ad(mtr_memo_contains(mtr, block, MTR_MEMO_PAGE_X_FIX)); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); if (dict_index_is_spatial(index)) { /* Check any concurrent search having this page */ @@ -4221,7 +4221,7 @@ btr_discard_page( page = buf_block_get_frame(block); ut_a(page_is_comp(merge_page) == page_is_comp(page)); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); if (left_page_no == FIL_NULL && !page_is_leaf(page)) { diff --git a/storage/innobase/btr/btr0defragment.cc b/storage/innobase/btr/btr0defragment.cc index 3fefd77041e..3bfd4d55b0e 100644 --- a/storage/innobase/btr/btr0defragment.cc +++ b/storage/innobase/btr/btr0defragment.cc @@ -486,7 +486,7 @@ btr_defragment_merge_pages( free it. */ lock_update_merge_left(to_block, orig_pred, from_block); - btr_search_drop_page_hash_index(from_block); + btr_search_drop_page_hash_index(from_block, false); ut_a(DB_SUCCESS == btr_level_list_remove( index->table->space_id, diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc index ff5a0e1e737..6171e1a3d25 100644 --- a/storage/innobase/btr/btr0sea.cc +++ b/storage/innobase/btr/btr0sea.cc @@ -682,7 +682,7 @@ btr_search_update_hash_ref( if (cursor->index != index) { ut_ad(cursor->index->id == index->id); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); return; } @@ -1312,7 +1312,7 @@ void btr_search_drop_page_hash_when_freed(const page_id_t page_id) dropping the table (preventing eviction). */ ut_ad(index->table->get_ref_count() > 0 || mutex_own(&dict_sys->mutex)); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); } } @@ -1384,7 +1384,7 @@ btr_search_build_page_hash_index( } if (rebuild) { - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); } /* Check that the values for hash index build are sensible */ @@ -1610,7 +1610,7 @@ btr_search_move_or_delete_hash_entries( if (new_block->index) { drop_exit: - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); return; } @@ -1686,7 +1686,7 @@ void btr_search_update_hash_on_delete(btr_cur_t* cursor) if (index != cursor->index) { ut_ad(index->id == cursor->index->id); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); return; } @@ -1766,7 +1766,7 @@ btr_search_update_hash_node_on_insert(btr_cur_t* cursor, rw_lock_t* ahi_latch) if (cursor->index != index) { ut_ad(cursor->index->id == index->id); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); return; } @@ -1861,7 +1861,7 @@ btr_search_update_hash_on_insert(btr_cur_t* cursor, rw_lock_t* ahi_latch) #endif if (index != cursor->index) { ut_ad(index->id == cursor->index->id); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); return; } diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index fe2c0c6b880..6a9f041deee 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -5414,7 +5414,7 @@ loop: } #ifdef BTR_CUR_HASH_ADAPT if (drop_hash_entry) { - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); } #endif /* BTR_CUR_HASH_ADAPT */ diff --git a/storage/innobase/buf/buf0lru.cc b/storage/innobase/buf/buf0lru.cc index d3bba9a1130..72612ebd9ad 100644 --- a/storage/innobase/buf/buf0lru.cc +++ b/storage/innobase/buf/buf0lru.cc @@ -1608,10 +1608,12 @@ func_exit: order to avoid bogus Valgrind or MSAN warnings.*/ buf_block_t* block = reinterpret_cast<buf_block_t*>(bpage); +#ifdef BTR_CUR_HASH_ADAPT MEM_MAKE_DEFINED(block->frame, srv_page_size); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); MEM_UNDEFINED(block->frame, srv_page_size); +#endif /* BTR_CUR_HASH_ADAPT */ buf_pool_mutex_enter(buf_pool); if (b) { diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 0cba7d953d1..cd40fe3145e 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -4371,8 +4371,18 @@ innobase_add_instant_try( DBUG_ASSERT(ctx->old_table->n_cols == ctx->old_n_cols); dict_table_t* user_table = ctx->old_table; - user_table->instant_add_column(*ctx->instant_table); dict_index_t* index = dict_table_get_first_index(user_table); + +#ifdef BTR_CUR_HASH_ADAPT + /* Acquire the ahi latch to avoid the race condition + between ahi access and instant alter table */ + rw_lock_t* ahi_latch = btr_get_search_latch(index); + rw_lock_x_lock(ahi_latch); +#endif /* BTR_CUR_HASH_ADAPT */ + user_table->instant_add_column(*ctx->instant_table); +#ifdef BTR_CUR_HASH_ADAPT + rw_lock_x_unlock(ahi_latch); +#endif /* BTR_CUR_HASH_ADAPT */ /* The table may have been emptied and may have lost its 'instant-add-ness' during this instant ADD COLUMN. */ diff --git a/storage/innobase/include/btr0sea.h b/storage/innobase/include/btr0sea.h index f217f8213f8..82ae50d6fac 100644 --- a/storage/innobase/include/btr0sea.h +++ b/storage/innobase/include/btr0sea.h @@ -102,7 +102,7 @@ btr_search_move_or_delete_hash_entries( @param[in] garbage_collect drop ahi only if the index is marked as freed */ void btr_search_drop_page_hash_index(buf_block_t* block, - bool garbage_collect= false); + bool garbage_collect); /** Drop possible adaptive hash index entries when a page is evicted from the buffer pool or freed in a file, or the index is being dropped. diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc index 7a86d825aeb..e89e1cf862c 100644 --- a/storage/innobase/page/page0zip.cc +++ b/storage/innobase/page/page0zip.cc @@ -4759,7 +4759,7 @@ page_zip_reorganize( mtr_log_t log_mode = mtr_set_log_mode(mtr, MTR_LOG_NONE); temp_block = buf_block_alloc(buf_pool); - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, false); temp_page = temp_block->frame; /* Copy the old page to temporary space */ |