MDEV-10104 Table lock race condition with replication
Problem was two race condtion in Aria page cache: - find_block() didn't inform free_block() that it had released requests - free_block() didn't handle pinned blocks, which could happen if free_block() was called as part of flush. This is fixed by not freeing blocks that are pinned. This is safe as when maria_close() is called when last thread is using a table, there can be no pinned blocks. For other flush calls it's safe to ignore pinned blocks.
This commit is contained in:
parent
e3521ab904
commit
fc25437aff
@ -493,7 +493,8 @@ error:
|
|||||||
|
|
||||||
#define FLUSH_CACHE 2000 /* sort this many blocks at once */
|
#define FLUSH_CACHE 2000 /* sort this many blocks at once */
|
||||||
|
|
||||||
static void free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block);
|
static my_bool free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block,
|
||||||
|
my_bool abort_if_pinned);
|
||||||
static void unlink_hash(PAGECACHE *pagecache, PAGECACHE_HASH_LINK *hash_link);
|
static void unlink_hash(PAGECACHE *pagecache, PAGECACHE_HASH_LINK *hash_link);
|
||||||
#ifndef DBUG_OFF
|
#ifndef DBUG_OFF
|
||||||
static void test_key_cache(PAGECACHE *pagecache,
|
static void test_key_cache(PAGECACHE *pagecache,
|
||||||
@ -1939,7 +1940,7 @@ restart:
|
|||||||
removed from the cache as we set the PCBLOCK_REASSIGNED
|
removed from the cache as we set the PCBLOCK_REASSIGNED
|
||||||
flag (see the code below that handles reading requests).
|
flag (see the code below that handles reading requests).
|
||||||
*/
|
*/
|
||||||
free_block(pagecache, block);
|
free_block(pagecache, block, 0);
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
/* Wait until the page is flushed on disk */
|
/* Wait until the page is flushed on disk */
|
||||||
@ -1950,7 +1951,7 @@ restart:
|
|||||||
/* Invalidate page in the block if it has not been done yet */
|
/* Invalidate page in the block if it has not been done yet */
|
||||||
DBUG_ASSERT(block->status); /* Should always be true */
|
DBUG_ASSERT(block->status); /* Should always be true */
|
||||||
if (block->status)
|
if (block->status)
|
||||||
free_block(pagecache, block);
|
free_block(pagecache, block, 0);
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1975,8 +1976,13 @@ restart:
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
DBUG_ASSERT(hash_link->requests > 0);
|
/*
|
||||||
hash_link->requests--;
|
When we come here either PCBLOCK_REASSIGNED or PCBLOCK_IN_SWITCH are
|
||||||
|
active. In both cases wqueue_release_queue() is called when the
|
||||||
|
state changes.
|
||||||
|
*/
|
||||||
|
DBUG_ASSERT(block->hash_link == hash_link);
|
||||||
|
remove_reader(block);
|
||||||
KEYCACHE_DBUG_PRINT("find_block",
|
KEYCACHE_DBUG_PRINT("find_block",
|
||||||
("request waiting for old page to be saved"));
|
("request waiting for old page to be saved"));
|
||||||
{
|
{
|
||||||
@ -3635,7 +3641,7 @@ static my_bool pagecache_delete_internal(PAGECACHE *pagecache,
|
|||||||
DBUG_ASSERT(block->hash_link->requests > 0);
|
DBUG_ASSERT(block->hash_link->requests > 0);
|
||||||
page_link->requests--;
|
page_link->requests--;
|
||||||
/* See NOTE for pagecache_unlock() about registering requests. */
|
/* See NOTE for pagecache_unlock() about registering requests. */
|
||||||
free_block(pagecache, block);
|
free_block(pagecache, block, 0);
|
||||||
dec_counter_for_resize_op(pagecache);
|
dec_counter_for_resize_op(pagecache);
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
@ -4227,7 +4233,8 @@ end:
|
|||||||
and add it to the free list.
|
and add it to the free list.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
static void free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block)
|
static my_bool free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block,
|
||||||
|
my_bool abort_if_pinned)
|
||||||
{
|
{
|
||||||
uint status= block->status;
|
uint status= block->status;
|
||||||
KEYCACHE_THREAD_TRACE("free block");
|
KEYCACHE_THREAD_TRACE("free block");
|
||||||
@ -4241,11 +4248,27 @@ static void free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block)
|
|||||||
/*
|
/*
|
||||||
While waiting for readers to finish, new readers might request the
|
While waiting for readers to finish, new readers might request the
|
||||||
block. But since we set block->status|= PCBLOCK_REASSIGNED, they
|
block. But since we set block->status|= PCBLOCK_REASSIGNED, they
|
||||||
will wait on block->wqueue[COND_FOR_SAVED]. They must be signalled
|
will wait on block->wqueue[COND_FOR_SAVED]. They must be signaled
|
||||||
later.
|
later.
|
||||||
*/
|
*/
|
||||||
block->status|= PCBLOCK_REASSIGNED;
|
block->status|= PCBLOCK_REASSIGNED;
|
||||||
wait_for_readers(pagecache, block);
|
wait_for_readers(pagecache, block);
|
||||||
|
if (unlikely(abort_if_pinned) && unlikely(block->pins))
|
||||||
|
{
|
||||||
|
/*
|
||||||
|
Block got pinned while waiting for readers.
|
||||||
|
This can only happens when called from flush_pagecache_blocks_int()
|
||||||
|
when flushing blocks as part of prepare for maria_close() or from
|
||||||
|
flush_cached_blocks()
|
||||||
|
*/
|
||||||
|
block->status&= ~PCBLOCK_REASSIGNED;
|
||||||
|
unreg_request(pagecache, block, 0);
|
||||||
|
|
||||||
|
/* All pending requests for this page must be resubmitted. */
|
||||||
|
if (block->wqueue[COND_FOR_SAVED].last_thread)
|
||||||
|
wqueue_release_queue(&block->wqueue[COND_FOR_SAVED]);
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
unlink_hash(pagecache, block->hash_link);
|
unlink_hash(pagecache, block->hash_link);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -4296,6 +4319,8 @@ static void free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block)
|
|||||||
/* All pending requests for this page must be resubmitted. */
|
/* All pending requests for this page must be resubmitted. */
|
||||||
if (block->wqueue[COND_FOR_SAVED].last_thread)
|
if (block->wqueue[COND_FOR_SAVED].last_thread)
|
||||||
wqueue_release_queue(&block->wqueue[COND_FOR_SAVED]);
|
wqueue_release_queue(&block->wqueue[COND_FOR_SAVED]);
|
||||||
|
|
||||||
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@ -4431,9 +4456,16 @@ static int flush_cached_blocks(PAGECACHE *pagecache,
|
|||||||
if (! (type == FLUSH_KEEP || type == FLUSH_KEEP_LAZY ||
|
if (! (type == FLUSH_KEEP || type == FLUSH_KEEP_LAZY ||
|
||||||
type == FLUSH_FORCE_WRITE))
|
type == FLUSH_FORCE_WRITE))
|
||||||
{
|
{
|
||||||
pagecache->blocks_changed--;
|
if (!free_block(pagecache, block, 1))
|
||||||
pagecache->global_blocks_changed--;
|
{
|
||||||
free_block(pagecache, block);
|
pagecache->blocks_changed--;
|
||||||
|
pagecache->global_blocks_changed--;
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
block->status&= ~PCBLOCK_IN_FLUSH;
|
||||||
|
link_to_file_list(pagecache, block, file, 1);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
@ -4671,7 +4703,7 @@ restart:
|
|||||||
/* It's a temporary file */
|
/* It's a temporary file */
|
||||||
pagecache->blocks_changed--;
|
pagecache->blocks_changed--;
|
||||||
pagecache->global_blocks_changed--;
|
pagecache->global_blocks_changed--;
|
||||||
free_block(pagecache, block);
|
free_block(pagecache, block, 0);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
else if (type != FLUSH_KEEP_LAZY)
|
else if (type != FLUSH_KEEP_LAZY)
|
||||||
@ -4741,11 +4773,12 @@ restart:
|
|||||||
#endif
|
#endif
|
||||||
next= block->next_changed;
|
next= block->next_changed;
|
||||||
if (block->hash_link->file.file == file->file &&
|
if (block->hash_link->file.file == file->file &&
|
||||||
|
!block->pins &&
|
||||||
(! (block->status & PCBLOCK_CHANGED)
|
(! (block->status & PCBLOCK_CHANGED)
|
||||||
|| type == FLUSH_IGNORE_CHANGED))
|
|| type == FLUSH_IGNORE_CHANGED))
|
||||||
{
|
{
|
||||||
reg_requests(pagecache, block, 1);
|
reg_requests(pagecache, block, 1);
|
||||||
free_block(pagecache, block);
|
free_block(pagecache, block, 1);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user