database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-07-27 18:02:13 +03:00
Code Activity

A prerequisite patch for the fix for Bug#46224

Browse Source 
"HANDLER statements within a transaction might lead to deadlocks".
Introduce a notion of a sentinel to MDL_context. A sentinel
is a ticket that separates all tickets in the context into two
groups: before and after it. Currently we can have (and need) only
one designated sentinel -- it separates all locks taken by LOCK
TABLE or HANDLER statement, which must survive COMMIT and ROLLBACK
and all other locks, which must be released at COMMIT or ROLLBACK.
The tricky part is maintaining the sentinel up to date when
someone release its corresponding ticket. This can happen, e.g.
if someone issues DROP TABLE under LOCK TABLES (generally,
see all calls to release_all_locks_for_name()).
MDL_context::release_ticket() is modified to take care of it.

******
A fix and a test case for Bug#46224 "HANDLER statements within a
transaction might lead to deadlocks".

An attempt to mix HANDLER SQL statements, which are transaction-
agnostic, an open multi-statement transaction,
and DDL against the involved tables (in a concurrent connection) 
could lead to a deadlock. The deadlock would occur when
HANDLER OPEN or HANDLER READ would have to wait on a conflicting
metadata lock. If the connection that issued HANDLER statement
also had other metadata locks (say, acquired in scope of a 
transaction), a classical deadlock situation of mutual wait
could occur.

Incompatible change: entering LOCK TABLES mode automatically
closes all open HANDLERs in the current connection.

Incompatible change: previously an attempt to wait on a lock
in a connection that has an open HANDLER statement could wait
indefinitely/deadlock. After this patch, an error ER_LOCK_DEADLOCK
is produced.

The idea of the fix is to merge thd->handler_mdl_context
with the main mdl_context of the connection, used for transactional
locks. This makes deadlock detection possible, since all waits
with locks are "visible" and available to analysis in a single
MDL context of the connection.

Since HANDLER locks and transactional locks have a different life
cycle -- HANDLERs are explicitly open and closed, and so
are HANDLER locks, explicitly acquired and released, whereas
transactional locks "accumulate" till the end of a transaction
and are released only with COMMIT, ROLLBACK and ROLLBACK TO SAVEPOINT,
a concept of "sentinel" was introduced to MDL_context.
All locks, HANDLER and others, reside in the same linked list.
However, a selected element of the list separates locks with
different life cycle. HANDLER locks always reside at the
end of the list, after the sentinel. Transactional locks are
prepended to the beginning of the list, before the sentinel.
Thus, ROLLBACK, COMMIT or ROLLBACK TO SAVEPOINT, only
release those locks that reside before the sentinel. HANDLER locks
must be released explicitly as part of HANDLER CLOSE statement,
or an implicit close. 
The same approach with sentinel
is also employed for LOCK TABLES locks. Since HANDLER and LOCK TABLES
statement has never worked together, the implementation is
made simple and only maintains one sentinel, which is used either
for HANDLER locks, or for LOCK TABLES locks.
This commit is contained in:
Konstantin Osipov
2009-12-22 19:09:15 +03:00
parent f032795ccc
commit 39a1a50dfb
24 changed files with 2335 additions and 391 deletions
Download Patch File Download Diff File Expand all files Collapse all files

207
sql/sql_handler.cc
View File

@ -33,27 +33,21 @@
*/
/*
There are two containers holding information about open handler tables.
The first is 'thd->handler_tables'. It is a linked list of TABLE objects.
It is used like 'thd->open_tables' in the table cache. The trick is to
exchange these two lists during open and lock of tables. Thus the normal
table cache code can be used.
The second container is a HASH. It holds objects of the type TABLE_LIST.
Despite its name, no lists of tables but only single structs are hashed
(the 'next' pointer is always NULL). The reason for theis second container
is, that we want handler tables to survive FLUSH TABLE commands. A table
affected by FLUSH TABLE must be closed so that other threads are not
blocked by handler tables still in use. Since we use the normal table cache
functions with 'thd->handler_tables', the closed tables are removed from
this list. Hence we need the original open information for the handler
table in the case that it is used again. This information is handed over
to mysql_ha_open() as a TABLE_LIST. So we store this information in the
second container, where it is not affected by FLUSH TABLE. The second
container is implemented as a hash for performance reasons. Consequently,
we use it not only for re-opening a handler table, but also for the
HANDLER ... READ commands. For this purpose, we store a pointer to the
TABLE structure (in the first container) in the TBALE_LIST object in the
second container. When the table is flushed, the pointer is cleared.
The information about open HANDLER objects is stored in a HASH.
It holds objects of type TABLE_LIST, which are indexed by table
name/alias, and allows us to quickly find a HANDLER table for any
operation at hand - be it HANDLER READ or HANDLER CLOSE.
It also allows us to maintain an "open" HANDLER even in cases
when there is no physically open cursor. E.g. a FLUSH TABLE
statement in this or some other connection demands that all open
HANDLERs against the flushed table are closed. In order to
preserve the information about an open HANDLER, we don't perform
a complete HANDLER CLOSE, but only close the TABLE object. The
corresponding TABLE_LIST is kept in the cache with 'table'
pointer set to NULL. The table will be reopened on next access
(this, however, leads to loss of cursor position, unless the
cursor points at the first record).
*/
#include "mysql_priv.h"
@ -124,32 +118,19 @@ static void mysql_ha_hash_free(TABLE_LIST *tables)
static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables)
{
TABLE **table_ptr;
MDL_ticket *mdl_ticket;
/*
Though we could take the table pointer from hash_tables->table,
we must follow the thd->handler_tables chain anyway, as we need the
address of the 'next' pointer referencing this table
for close_thread_table().
*/
for (table_ptr= &(thd->handler_tables);
*table_ptr && (*table_ptr != tables->table);
table_ptr= &(*table_ptr)->next)
;
if (*table_ptr)
if (tables->table && !tables->table->s->tmp_table)
{
(*table_ptr)->file->ha_index_or_rnd_end();
mdl_ticket= (*table_ptr)->mdl_ticket;
/* Non temporary table. */
tables->table->file->ha_index_or_rnd_end();
pthread_mutex_lock(&LOCK_open);
if (close_thread_table(thd, table_ptr))
if (close_thread_table(thd, &tables->table))
{
/* Tell threads waiting for refresh that something has happened */
broadcast_refresh();
}
pthread_mutex_unlock(&LOCK_open);
thd->handler_mdl_context.release_lock(mdl_ticket);
thd->mdl_context.release_lock(tables->mdl_request.ticket);
}
else if (tables->table)
{
@ -158,6 +139,7 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables)
table->file->ha_index_or_rnd_end();
table->query_id= thd->query_id;
table->open_by_handler= 0;
mark_tmp_table_for_reuse(table);
}
/* Mark table as closed, ready for re-open if necessary. */
@ -195,7 +177,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
uint dblen, namelen, aliaslen, counter;
bool error;
TABLE *backup_open_tables;
MDL_context backup_mdl_context;
MDL_ticket *mdl_savepoint;
DBUG_ENTER("mysql_ha_open");
DBUG_PRINT("enter",("'%s'.'%s' as '%s' reopen: %d",
tables->db, tables->table_name, tables->alias,
@ -265,6 +247,8 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
memcpy(hash_tables->table_name, tables->table_name, namelen);
memcpy(hash_tables->alias, tables->alias, aliaslen);
hash_tables->mdl_request.init(MDL_key::TABLE, db, name, MDL_SHARED);
/* for now HANDLER can be used only for real TABLES */
hash_tables->required_type= FRMTYPE_TABLE;
/* add to hash */
if (my_hash_insert(&thd->handler_tables_hash, (uchar*) hash_tables))
@ -283,16 +267,11 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
from open_tables(), thd->open_tables will contain only the opened
table.
The thd->handler_tables list is kept as-is to avoid deadlocks if
open_table(), called by open_tables(), needs to back-off because
of a pending exclusive metadata lock or flush for the table being
opened.
See open_table() back-off comments for more details.
*/
backup_open_tables= thd->open_tables;
thd->open_tables= NULL;
thd->mdl_context.backup_and_reset(&backup_mdl_context);
mdl_savepoint= thd->mdl_context.mdl_savepoint();
/*
open_tables() will set 'hash_tables->table' if successful.
@ -300,53 +279,47 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
*/
DBUG_ASSERT(! hash_tables->table);
/* for now HANDLER can be used only for real TABLES */
hash_tables->required_type= FRMTYPE_TABLE;
/*
We use open_tables() here, rather than, say,
open_ltable() or open_table() because we would like to be able
to open a temporary table.
*/
error= open_tables(thd, &hash_tables, &counter, 0);
if (thd->open_tables)
{
if (thd->open_tables->next)
{
/*
We opened something that is more than a single table.
This happens with MERGE engine. Don't try to link
this mess into thd->handler_tables list, close it
and report an error. We must do it right away
because mysql_ha_close_table(), called down the road,
can close a single table only.
*/
close_thread_tables(thd);
thd->mdl_context.release_all_locks();
my_error(ER_ILLEGAL_HA, MYF(0), hash_tables->alias);
error= TRUE;
}
else
{
/* Merge the opened table into handler_tables list. */
thd->open_tables->next= thd->handler_tables;
thd->handler_tables= thd->open_tables;
}
}
thd->handler_mdl_context.merge(&thd->mdl_context);
thd->open_tables= backup_open_tables;
thd->mdl_context.restore_from_backup(&backup_mdl_context);
if (error)
goto err;
/* There can be only one table in '*tables'. */
if (! (hash_tables->table->file->ha_table_flags() & HA_CAN_SQL_HANDLER))
if (! error &&
! (hash_tables->table->file->ha_table_flags() & HA_CAN_SQL_HANDLER))
{
my_error(ER_ILLEGAL_HA, MYF(0), tables->alias);
goto err;
error= TRUE;
}
if (!error &&
hash_tables->mdl_request.ticket &&
thd->mdl_context.has_lock(mdl_savepoint,
hash_tables->mdl_request.ticket))
{
/* The ticket returned is within a savepoint. Make a copy. */
error= thd->mdl_context.clone_ticket(&hash_tables->mdl_request);
hash_tables->table->mdl_ticket= hash_tables->mdl_request.ticket;
}
if (error)
{
close_thread_tables(thd);
thd->open_tables= backup_open_tables;
thd->mdl_context.rollback_to_savepoint(mdl_savepoint);
if (!reopen)
my_hash_delete(&thd->handler_tables_hash, (uchar*) hash_tables);
else
hash_tables->table= NULL;
DBUG_PRINT("exit",("ERROR"));
DBUG_RETURN(TRUE);
}
thd->open_tables= backup_open_tables;
if (hash_tables->mdl_request.ticket)
thd->mdl_context.
move_ticket_after_lt_or_ha_sentinel(hash_tables->mdl_request.ticket);
/* Assert that the above check prevent opening of views and merge tables. */
DBUG_ASSERT(hash_tables->table->next == NULL);
/*
If it's a temp table, don't reset table->query_id as the table is
being used by this handler. Otherwise, no meaning at all.
@ -357,14 +330,6 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
my_ok(thd);
DBUG_PRINT("exit",("OK"));
DBUG_RETURN(FALSE);
err:
if (hash_tables->table)
mysql_ha_close_table(thd, hash_tables);
if (!reopen)
my_hash_delete(&thd->handler_tables_hash, (uchar*) hash_tables);
DBUG_PRINT("exit",("ERROR"));
DBUG_RETURN(TRUE);
}
@ -496,59 +461,41 @@ retry:
hash_tables->db, hash_tables->table_name,
hash_tables->alias, table));
}
table->pos_in_table_list= tables;
#if MYSQL_VERSION_ID < 40100
if (*tables->db && strcmp(table->table_cache_key, tables->db))
{
DBUG_PRINT("info",("wrong db"));
table= NULL;
}
#endif
}
else
table= NULL;
if (!table)
{
#if MYSQL_VERSION_ID < 40100
char buff[MAX_DBKEY_LENGTH];
if (*tables->db)
strxnmov(buff, sizeof(buff)-1, tables->db, ".", tables->table_name,
NullS);
else
strncpy(buff, tables->alias, sizeof(buff));
my_error(ER_UNKNOWN_TABLE, MYF(0), buff, "HANDLER");
#else
my_error(ER_UNKNOWN_TABLE, MYF(0), tables->alias, "HANDLER");
#endif
goto err0;
}
tables->table=table;
/* save open_tables state */
backup_open_tables= thd->open_tables;
/* Always a one-element list, see mysql_ha_open(). */
DBUG_ASSERT(hash_tables->table->next == NULL);
/*
mysql_lock_tables() needs thd->open_tables to be set correctly to
be able to handle aborts properly. When the abort happens, it's
safe to not protect thd->handler_tables because it won't close any
tables.
be able to handle aborts properly.
*/
thd->open_tables= thd->handler_tables;
thd->open_tables= hash_tables->table;
lock= mysql_lock_tables(thd, &tables->table, 1, 0, &need_reopen);
/* restore previous context */
lock= mysql_lock_tables(thd, &thd->open_tables, 1, 0, &need_reopen);
/*
In 5.1 and earlier, mysql_lock_tables() could replace the TABLE
object with another one (reopen it). This is no longer the case
with new MDL.
*/
DBUG_ASSERT(hash_tables->table == thd->open_tables);
/* Restore previous context. */
thd->open_tables= backup_open_tables;
if (need_reopen)
{
mysql_ha_close_table(thd, hash_tables);
/*
The lock might have been aborted, we need to manually reset
thd->some_tables_deleted because handler's tables are closed
in a non-standard way. Otherwise we might loop indefinitely.
*/
thd->some_tables_deleted= 0;
goto retry;
}
@ -556,7 +503,8 @@ retry:
goto err0; // mysql_lock_tables() printed error message already
// Always read all columns
tables->table->read_set= &tables->table->s->all_set;
hash_tables->table->read_set= &hash_tables->table->s->all_set;
tables->table= hash_tables->table;
if (cond)
{
@ -811,6 +759,14 @@ void mysql_ha_flush(THD *thd)
safe_mutex_assert_not_owner(&LOCK_open);
/*
Don't try to flush open HANDLERs when we're working with
system tables. The main MDL context is backed up and we can't
properly release HANDLER locks stored there.
*/
if (thd->state_flags & Open_tables_state::BACKUPS_AVAIL)
DBUG_VOID_RETURN;
for (uint i= 0; i < thd->handler_tables_hash.records; i++)
{
hash_tables= (TABLE_LIST*) my_hash_element(&thd->handler_tables_hash, i);
@ -818,9 +774,10 @@ void mysql_ha_flush(THD *thd)
TABLE::mdl_ticket is 0 for temporary tables so we need extra check.
*/
if (hash_tables->table &&
(hash_tables->table->mdl_ticket &&
hash_tables->table->mdl_ticket->has_pending_conflicting_lock() ||
hash_tables->table->s->needs_reopen()))
((hash_tables->table->mdl_ticket &&
hash_tables->table->mdl_ticket->has_pending_conflicting_lock()) ||
(!hash_tables->table->s->tmp_table &&
hash_tables->table->s->needs_reopen())))
mysql_ha_close_table(thd, hash_tables);
}