diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2023-02-16 08:29:44 +0200 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2023-02-16 08:29:44 +0200 |
commit | 54c0ac72e300acb4405529a827a8c02c4a5e5d15 (patch) | |
tree | 3d347418b334fd36ee437d5064fea1de14c9f079 /storage | |
parent | 9c15799462d6a2673a6158eb1717d222c07cced4 (diff) | |
download | mariadb-git-54c0ac72e300acb4405529a827a8c02c4a5e5d15.tar.gz |
MDEV-30134 Assertion failed in buf_page_t::unfix() in buf_pool_t::watch_unset()
buf_pool_t::watch_set(): Always buffer-fix a block if one was found,
no matter if it is a watch sentinel or a buffer page. The type of
the block descriptor will be rechecked in buf_page_t::watch_unset().
Do not expect the caller to acquire the page hash latch. Starting with
commit bd5a6403cace36c6ed428cde62e35adcd3f7e7d0 it is safe to release
buf_pool.mutex before acquiring a buf_pool.page_hash latch.
buf_page_get_low(): Adjust to the changed buf_pool_t::watch_set().
This simplifies the logic and fixes a bug that was reproduced when
using debug builds and the setting innodb_change_buffering_debug=1.
Diffstat (limited to 'storage')
-rw-r--r-- | storage/innobase/buf/buf0buf.cc | 71 | ||||
-rw-r--r-- | storage/innobase/include/buf0buf.h | 12 |
2 files changed, 33 insertions, 50 deletions
diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index b17c49ab751..47cc915a11d 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -1951,28 +1951,22 @@ static void buf_relocate(buf_page_t *bpage, buf_page_t *dpage) buf_pool.page_hash.replace(chain, bpage, dpage); } -/** Register a watch for a page identifier. The caller must hold an -exclusive page hash latch. The *hash_lock may be released, -relocated, and reacquired. -@param id page identifier -@param chain hash table chain with exclusively held page_hash -@return a buffer pool block corresponding to id -@retval nullptr if the block was not present, and a watch was installed */ -inline buf_page_t *buf_pool_t::watch_set(const page_id_t id, - buf_pool_t::hash_chain &chain) +buf_page_t *buf_pool_t::watch_set(const page_id_t id, + buf_pool_t::hash_chain &chain) { ut_ad(&chain == &page_hash.cell_get(id.fold())); - ut_ad(page_hash.lock_get(chain).is_write_locked()); + page_hash.lock_get(chain).lock(); -retry: - if (buf_page_t *bpage= page_hash.get(id, chain)) + buf_page_t *bpage= page_hash.get(id, chain); + + if (bpage) { - if (!watch_is_sentinel(*bpage)) - /* The page was loaded meanwhile. */ - return bpage; - /* Add to an existing watch. */ +got_block: bpage->fix(); - return nullptr; + if (watch_is_sentinel(*bpage)) + bpage= nullptr; + page_hash.lock_get(chain).unlock(); + return bpage; } page_hash.lock_get(chain).unlock(); @@ -2001,25 +1995,23 @@ retry: w->set_state(buf_page_t::UNFIXED + 1); w->id_= id; - buf_page_t *bpage= page_hash.get(id, chain); + page_hash.lock_get(chain).lock(); + bpage= page_hash.get(id, chain); if (UNIV_LIKELY_NULL(bpage)) { w->set_state(buf_page_t::NOT_USED); - page_hash.lock_get(chain).lock(); mysql_mutex_unlock(&mutex); - goto retry; + goto got_block; } - page_hash.lock_get(chain).lock(); ut_ad(w->state() == buf_page_t::UNFIXED + 1); buf_pool.page_hash.append(chain, w); mysql_mutex_unlock(&mutex); + page_hash.lock_get(chain).unlock(); return nullptr; } ut_error; - mysql_mutex_unlock(&mutex); - return nullptr; } /** Stop watching whether a page has been read in. @@ -2467,16 +2459,13 @@ loop: case BUF_PEEK_IF_IN_POOL: return nullptr; case BUF_GET_IF_IN_POOL_OR_WATCH: - /* We cannot easily use a memory transaction here. */ - hash_lock.lock(); + /* Buffer-fixing inside watch_set() will prevent eviction */ block = reinterpret_cast<buf_block_t*> (buf_pool.watch_set(page_id, chain)); - /* buffer-fixing will prevent eviction */ - state = block ? block->page.fix() : 0; - hash_lock.unlock(); if (block) { - goto got_block; + state = block->page.state(); + goto got_block_fixed; } return nullptr; @@ -2515,6 +2504,7 @@ loop: got_block: ut_ad(!block->page.in_zip_hash); state++; +got_block_fixed: ut_ad(state > buf_page_t::FREED); if (state > buf_page_t::READ_FIX && state < buf_page_t::WRITE_FIX) { @@ -2724,24 +2714,19 @@ re_evict: const bool evicted = buf_LRU_free_page(&block->page, true); space->release(); + if (!evicted) { + block->fix(); + } + + mysql_mutex_unlock(&buf_pool.mutex); + if (evicted) { - page_hash_latch& hash_lock - = buf_pool.page_hash.lock_get(chain); - hash_lock.lock(); - mysql_mutex_unlock(&buf_pool.mutex); - /* We may set the watch, as it would have - been set if the page were not in the - buffer pool in the first place. */ - block= reinterpret_cast<buf_block_t*>( - mode == BUF_GET_IF_IN_POOL_OR_WATCH - ? buf_pool.watch_set(page_id, chain) - : buf_pool.page_hash.get(page_id, chain)); - hash_lock.unlock(); + if (mode == BUF_GET_IF_IN_POOL_OR_WATCH) { + buf_pool.watch_set(page_id, chain); + } return(NULL); } - block->fix(); - mysql_mutex_unlock(&buf_pool.mutex); buf_flush_sync(); state = block->page.state(); diff --git a/storage/innobase/include/buf0buf.h b/storage/innobase/include/buf0buf.h index b4d8c8b76ac..2dccdda8a2f 100644 --- a/storage/innobase/include/buf0buf.h +++ b/storage/innobase/include/buf0buf.h @@ -1470,14 +1470,12 @@ public: return !watch_is_sentinel(*page_hash.get(id, chain)); } - /** Register a watch for a page identifier. The caller must hold an - exclusive page hash latch. The *hash_lock may be released, - relocated, and reacquired. + /** Register a watch for a page identifier. @param id page identifier - @param chain hash table chain with exclusively held page_hash - @return a buffer pool block corresponding to id - @retval nullptr if the block was not present, and a watch was installed */ - inline buf_page_t *watch_set(const page_id_t id, hash_chain &chain); + @param chain page_hash.cell_get(id.fold()) + @return a buffer page corresponding to id + @retval nullptr if the block was not present in page_hash */ + buf_page_t *watch_set(const page_id_t id, hash_chain &chain); /** Stop watching whether a page has been read in. watch_set(id) must have returned nullptr before. |