Skip to content

Commit

Permalink
MDEV-31354 SIGSEGV in log_sort_flush_list() in InnoDB crash recovery
Browse files Browse the repository at this point in the history
log_sort_flush_list(): Wait for any pending page writes to cease
before sorting the buf_pool.flush_list. Starting with
commit 22b62ed (MDEV-25113),
it is possible that some buf_page_t::oldest_modification_
that we will be comparing in std::sort() will be updated from
some value >2 to 1 while we are holding buf_pool.flush_list_mutex.

To catch this type of trouble better in the future, we will clean
garbage (pages that have been written out) from buf_pool.flush_list
while constructing the array for sorting, and check with debug
assertions that all blocks that we are copying from the array to the
list will be dirty (requiring a writeback) while we sort and copy
the array back to buf_pool.flush_list.

This failure was observed by chance exactly once when running the test
innodb.recovery_memory. It was never reproduced in the same form
afterwards. Unrelated to this change, the test does occasionally
reproduce a failure to start up InnoDB due to a corrupted page
being read in recovery. The ticket MDEV-31791 was filed for that.

Tested by: Matthias Leich
  • Loading branch information
dr-m committed Jul 28, 2023
1 parent b102872 commit 0d17596
Showing 1 changed file with 33 additions and 7 deletions.
40 changes: 33 additions & 7 deletions storage/innobase/log/log0recv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3580,25 +3580,51 @@ inline fil_space_t *fil_system_t::find(const char *path) const
/** Thread-safe function which sorts flush_list by oldest_modification */
static void log_sort_flush_list()
{
mysql_mutex_lock(&buf_pool.flush_list_mutex);
/* Ensure that oldest_modification() cannot change during std::sort() */
for (;;)
{
os_aio_wait_until_no_pending_writes(false);
mysql_mutex_lock(&buf_pool.flush_list_mutex);
if (buf_pool.flush_list_active())
my_cond_wait(&buf_pool.done_flush_list,
&buf_pool.flush_list_mutex.m_mutex);
else if (!os_aio_pending_writes())
break;
mysql_mutex_unlock(&buf_pool.flush_list_mutex);
}

const size_t size= UT_LIST_GET_LEN(buf_pool.flush_list);
std::unique_ptr<buf_page_t *[]> list(new buf_page_t *[size]);

/* Copy the dirty blocks from buf_pool.flush_list to an array for sorting. */
size_t idx= 0;
for (buf_page_t *p= UT_LIST_GET_FIRST(buf_pool.flush_list); p;
p= UT_LIST_GET_NEXT(list, p))
list.get()[idx++]= p;
for (buf_page_t *p= UT_LIST_GET_FIRST(buf_pool.flush_list); p; )
{
const lsn_t lsn{p->oldest_modification()};
ut_ad(lsn > 2 || lsn == 1);
buf_page_t *n= UT_LIST_GET_NEXT(list, p);
if (lsn > 1)
list.get()[idx++]= p;
else
buf_pool.delete_from_flush_list(p);
p= n;
}

std::sort(list.get(), list.get() + size,
std::sort(list.get(), list.get() + idx,
[](const buf_page_t *lhs, const buf_page_t *rhs) {
return rhs->oldest_modification() < lhs->oldest_modification();
const lsn_t l{lhs->oldest_modification()};
const lsn_t r{rhs->oldest_modification()};
DBUG_ASSERT(l > 2); DBUG_ASSERT(r > 2);
return r < l;
});

UT_LIST_INIT(buf_pool.flush_list, &buf_page_t::list);

for (size_t i= 0; i < size; i++)
for (size_t i= 0; i < idx; i++)
{
UT_LIST_ADD_LAST(buf_pool.flush_list, list[i]);
DBUG_ASSERT(list[i]->oldest_modification() > 2);
}

mysql_mutex_unlock(&buf_pool.flush_list_mutex);
}
Expand Down

0 comments on commit 0d17596

Please sign in to comment.