summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThirunarayanan Balathandayuthapani <thiru@mariadb.com>2022-11-17 17:24:13 +0530
committerThirunarayanan Balathandayuthapani <thiru@mariadb.com>2022-11-22 15:24:44 +0530
commit71c93fb8fd894e39b81b801a67ccb9de535b6ce8 (patch)
tree647e29cb7889df1276aa773d513d5449875edd05
parentf4a1298f245f678badc8a5b55571ca4f460718b1 (diff)
downloadmariadb-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.cc16
-rw-r--r--storage/innobase/btr/btr0defragment.cc2
-rw-r--r--storage/innobase/btr/btr0sea.cc14
-rw-r--r--storage/innobase/buf/buf0buf.cc2
-rw-r--r--storage/innobase/buf/buf0lru.cc4
-rw-r--r--storage/innobase/handler/handler0alter.cc12
-rw-r--r--storage/innobase/include/btr0sea.h2
-rw-r--r--storage/innobase/page/page0zip.cc2
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 */