Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-23998

Race condition between buf_page_optimistic_get() and buf_page_t::init()

    XMLWordPrintable

    Details

      Description

      MDEV-22871 tried to optimize the buf_page_t initialization in buf_page_init_for_read() by initializing everything while the block is in freed state, and only afterwards attaching the block to buf_pool.page_hash.

      In an rr replay trace, we have multiple threads executing in buf_page_optimistic_get() on the same buf_block_t while the block is being freed and reallocated several times in buf_page_init_for_read(). Because also the buf_page_t::id() is changing, the buf_pool.page_hash is being protected by a different rw-lock than the one that buf_page_optimistic_get() are successfully read-locking.

      I think that the following fix should address this:

      diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc
      index 60979884fda..2cdd36a8b60 100644
      --- a/storage/innobase/buf/buf0buf.cc
      +++ b/storage/innobase/buf/buf0buf.cc
      @@ -3603,10 +3603,13 @@ buf_page_optimistic_get(
       		return FALSE;
       	}
       
      -	page_hash_latch *hash_lock = buf_pool.hash_lock_get(block->page.id());
      +	const page_id_t id(block->page.id());
      +
      +	page_hash_latch *hash_lock = buf_pool.hash_lock_get(id);
       	hash_lock->read_lock();
       
      -	if (UNIV_UNLIKELY(block->page.state() != BUF_BLOCK_FILE_PAGE
      +	if (UNIV_UNLIKELY(id != block->page.id()
      +			  || block->page.state() != BUF_BLOCK_FILE_PAGE
       			  || block->page.io_fix() != BUF_IO_NONE)) {
       		hash_lock->read_unlock();
       		return(FALSE);
      @@ -3619,8 +3622,7 @@ buf_page_optimistic_get(
       
       	buf_page_make_young_if_needed(&block->page);
       
      -	ut_ad(!ibuf_inside(mtr)
      -	      || ibuf_page(block->page.id(), block->zip_size(), NULL));
      +	ut_ad(!ibuf_inside(mtr) || ibuf_page(id, block->zip_size(), NULL));
       
       	mtr_memo_type_t	fix_type;
       
      @@ -3633,6 +3635,8 @@ buf_page_optimistic_get(
       			&block->lock, file, line);
       	}
       
      +	ut_ad(id == block->page.id());
      +
       	if (!success) {
       		buf_block_buf_fix_dec(block);
       		return(FALSE);
      

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              marko Marko Mäkelä
              Reporter:
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: