Skip to content

MDEV-39548 Cleanup MDL_request boilerplate with ACQUIRE_LOCK macro #5075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
longjinvan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

MDEV-39548 Cleanup MDL_request boilerplate with ACQUIRE_LOCK macro #5075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions sql/backup.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ bool run_backup_stage(THD *thd, backup_stages stage)

static bool backup_start(THD *thd)
{
MDL_request mdl_request;
DBUG_ENTER("backup_start");

thd->current_backup_stage= BACKUP_FINISHED; // For next test
Expand All @@ -182,20 +181,20 @@ static bool backup_start(THD *thd)
Wait for old backup to finish and block ddl's so that we can start the
ddl logger
*/
MDL_REQUEST_INIT(&mdl_request, MDL_key::BACKUP, "", "", MDL_BACKUP_BLOCK_DDL,
MDL_EXPLICIT);
if (thd->mdl_context.acquire_lock(&mdl_request,
thd->variables.lock_wait_timeout))
MDL_ticket *ticket;
if (!(ticket= thd->mdl_context.MDL_ACQUIRE_LOCK(
MDL_key::BACKUP, "", "", MDL_BACKUP_BLOCK_DDL,
MDL_EXPLICIT, thd->variables.lock_wait_timeout)))
DBUG_RETURN(1);

if (start_ddl_logging())
{
thd->mdl_context.release_lock(mdl_request.ticket);
thd->mdl_context.release_lock(ticket);
DBUG_RETURN(1);
}

DBUG_ASSERT(backup_flush_ticket == 0);
backup_flush_ticket= mdl_request.ticket;
backup_flush_ticket= ticket;

/* Downgrade lock to only block other backups */
backup_flush_ticket->downgrade_lock(MDL_BACKUP_START);
Expand Down Expand Up @@ -541,11 +540,11 @@ bool backup_lock(THD *thd, TABLE_LIST *table)
my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0));
return 1;
}
table->mdl_request.duration= MDL_EXPLICIT;
if (thd->mdl_context.acquire_lock(&table->mdl_request,
thd->variables.lock_wait_timeout))
if (!(thd->mdl_backup_lock= thd->mdl_context.MDL_ACQUIRE_LOCK(
MDL_key::TABLE, table->db.str, table->table_name.str,
MDL_SHARED_HIGH_PRIO, MDL_EXPLICIT,
thd->variables.lock_wait_timeout)))
return 1;
thd->mdl_backup_lock= table->mdl_request.ticket;
return 0;
}

Expand Down
14 changes: 6 additions & 8 deletions sql/ddl_log.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1252,16 +1252,14 @@ static void rename_triggers(THD *thd, DDL_LOG_ENTRY *ddl_log_entry,
We have to create a MDL lock as change_table_names() checks that we
have a mdl locks for the table
*/
MDL_request mdl_request;
MDL_ticket *mdl_ticket;
TRIGGER_RENAME_PARAM trigger_param;
int error __attribute__((unused));
MDL_REQUEST_INIT(&mdl_request, MDL_key::TABLE,
mdl_ticket= thd->mdl_context.MDL_ACQUIRE_LOCK(MDL_key::TABLE,
from_db.str,
from_converted_name.str,
MDL_EXCLUSIVE, MDL_EXPLICIT);
error= thd->mdl_context.acquire_lock(&mdl_request, 1);
/* acquire_locks() should never fail during recovery */
DBUG_ASSERT(error == 0);
MDL_EXCLUSIVE, MDL_EXPLICIT, 1);
/* acquire_lock() should never fail during recovery */
DBUG_ASSERT(mdl_ticket != NULL);
Comment thread
svoj marked this conversation as resolved.

(void) Table_triggers_list::prepare_for_rename(thd,
&trigger_param,
Expand All @@ -1277,7 +1275,7 @@ static void rename_triggers(THD *thd, DDL_LOG_ENTRY *ddl_log_entry,
&from_converted_name,
&to_db,
&to_table);
thd->mdl_context.release_lock(mdl_request.ticket);
thd->mdl_context.release_lock(mdl_ticket);
}
}

Expand Down
8 changes: 3 additions & 5 deletions sql/handler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,13 +1080,11 @@ bool handler::log_not_redoable_operation(const char *operation)
new log entry (and re-copy the table if needed).
*/
THD *thd= table->in_use;
MDL_request mdl_backup;
backup_log_info ddl_log;

MDL_REQUEST_INIT(&mdl_backup, MDL_key::BACKUP, "", "", MDL_BACKUP_DDL,
MDL_STATEMENT);
if (thd->mdl_context.acquire_lock(&mdl_backup,
thd->variables.lock_wait_timeout))
if (!thd->mdl_context.MDL_ACQUIRE_LOCK(MDL_key::BACKUP, "", "", MDL_BACKUP_DDL,
MDL_STATEMENT,
thd->variables.lock_wait_timeout))
DBUG_RETURN(1);

bzero(&ddl_log, sizeof(ddl_log));
Expand Down
17 changes: 8 additions & 9 deletions sql/item_func.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4375,14 +4375,12 @@ longlong Item_func_get_lock::val_int()
DBUG_RETURN(0);
}

MDL_request ull_request;
MDL_REQUEST_INIT(&ull_request, MDL_key::USER_LOCK, res->c_ptr_safe(), "",
MDL_SHARED_NO_WRITE, MDL_EXPLICIT);
MDL_key *ull_key= &ull_request.key;
MDL_key ull_key;
ull_key.mdl_key_init(MDL_key::USER_LOCK, res->c_ptr_safe(), "");


if ((ull= (User_level_lock*)
my_hash_search(&thd->ull_hash, ull_key->ptr(), ull_key->length())))
my_hash_search(&thd->ull_hash, ull_key.ptr(), ull_key.length())))
{
/* Recursive lock */
ull->refs++;
Expand All @@ -4393,9 +4391,10 @@ longlong Item_func_get_lock::val_int()

Lock_wait_timeout_handler lock_wait_timeout_handler;
thd->push_internal_handler(&lock_wait_timeout_handler);
bool error= thd->mdl_context.acquire_lock(&ull_request, timeout);
MDL_ticket *ticket= thd->mdl_context.MDL_ACQUIRE_LOCK_BY_KEY(
&ull_key, MDL_SHARED_NO_WRITE, MDL_EXPLICIT, timeout);
(void) thd->pop_internal_handler();
if (unlikely(error))
if (unlikely(!ticket))
{
if (lock_wait_timeout_handler.m_lock_wait_timeout)
null_value= 0;
Expand All @@ -4407,11 +4406,11 @@ longlong Item_func_get_lock::val_int()
MYF(MY_WME|MY_THREAD_SPECIFIC));
if (ull == NULL)
{
thd->mdl_context.release_lock(ull_request.ticket);
thd->mdl_context.release_lock(ticket);
DBUG_RETURN(0);
}

ull->lock= ull_request.ticket;
ull->lock= ticket;
ull->refs= 1;

if (my_hash_insert(&thd->ull_hash, (uchar*) ull))
Expand Down
12 changes: 4 additions & 8 deletions sql/lock.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,8 +1109,6 @@ bool Global_read_lock::lock_global_read_lock(THD *thd)
if (!m_state)
{
MDL_deadlock_and_lock_abort_error_handler mdl_deadlock_handler;
MDL_request mdl_request;
bool result;

if (thd->current_backup_stage != BACKUP_FINISHED)
{
Expand All @@ -1129,22 +1127,20 @@ bool Global_read_lock::lock_global_read_lock(THD *thd)
MDL_BACKUP_FTWRL1));
DBUG_ASSERT(! thd->mdl_context.is_lock_owner(MDL_key::BACKUP, "", "",
MDL_BACKUP_FTWRL2));
MDL_REQUEST_INIT(&mdl_request, MDL_key::BACKUP, "", "", MDL_BACKUP_FTWRL1,
MDL_EXPLICIT);

do
{
mdl_deadlock_handler.init();
thd->push_internal_handler(&mdl_deadlock_handler);
result= thd->mdl_context.acquire_lock(&mdl_request,
thd->variables.lock_wait_timeout);
m_mdl_global_read_lock= thd->mdl_context.MDL_ACQUIRE_LOCK(
MDL_key::BACKUP, "", "", MDL_BACKUP_FTWRL1,
MDL_EXPLICIT, thd->variables.lock_wait_timeout);
thd->pop_internal_handler();
} while (mdl_deadlock_handler.need_reopen());

if (result)
if (!m_mdl_global_read_lock)
DBUG_RETURN(true);

m_mdl_global_read_lock= mdl_request.ticket;
m_state= GRL_ACQUIRED;
}
/*
Expand Down
48 changes: 48 additions & 0 deletions sql/mdl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2665,6 +2665,21 @@ MDL_context::try_acquire_lock(MDL_request *mdl_request)
}


MDL_ticket *
MDL_context::try_acquire_lock(MDL_key::enum_mdl_namespace mdl_namespace,
const char *db, const char *name,
enum_mdl_type mdl_type,
enum_mdl_duration mdl_duration)
{
MDL_request mdl_request;
MDL_REQUEST_INIT(&mdl_request, mdl_namespace, db, name, mdl_type,
mdl_duration);
if (try_acquire_lock_impl(&mdl_request, nullptr))
return reinterpret_cast<MDL_ticket*>(-1);
return mdl_request.ticket;
}


/**
Auxiliary method for acquiring lock without waiting.

Expand Down Expand Up @@ -3075,6 +3090,39 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
}


MDL_ticket *
MDL_context::acquire_lock(MDL_key::enum_mdl_namespace mdl_namespace,
const char *db, const char *name,
enum_mdl_type mdl_type,
enum_mdl_duration mdl_duration,
double lock_wait_timeout,
const char *src_file, uint src_line)
{
MDL_request mdl_request;
mdl_request.init_with_source(mdl_namespace, db, name, mdl_type, mdl_duration,
src_file, src_line);
if (acquire_lock(&mdl_request, lock_wait_timeout))
return NULL;
return mdl_request.ticket;
}


MDL_ticket *
MDL_context::acquire_lock(const MDL_key *key,
enum_mdl_type mdl_type,
enum_mdl_duration mdl_duration,
double lock_wait_timeout,
const char *src_file, uint src_line)
{
MDL_request mdl_request;
mdl_request.init_by_key_with_source(key, mdl_type, mdl_duration,
src_file, src_line);
if (acquire_lock(&mdl_request, lock_wait_timeout))
return NULL;
return mdl_request.ticket;
}


extern "C" int mdl_request_ptr_cmp(const void* ptr1, const void* ptr2)
{
MDL_request *req1= *(MDL_request**)ptr1;
Expand Down
22 changes: 22 additions & 0 deletions sql/mdl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,12 @@ typedef void (*mdl_cached_object_release_hook)(void *);
#define MDL_REQUEST_INIT_BY_KEY(R, P1, P2, P3) \
(*R).init_by_key_with_source(P1, P2, P3, __FILE__, __LINE__)

#define MDL_ACQUIRE_LOCK(NAMESPACE, DB, NAME, TYPE, DURATION, TIMEOUT) \
acquire_lock(NAMESPACE, DB, NAME, TYPE, DURATION, TIMEOUT, __FILE__, __LINE__)

#define MDL_ACQUIRE_LOCK_BY_KEY(KEY, TYPE, DURATION, TIMEOUT) \
acquire_lock(KEY, TYPE, DURATION, TIMEOUT, __FILE__, __LINE__)


/**
An abstract class for inspection of a connected
Expand Down Expand Up @@ -805,12 +811,28 @@ class MDL_context
void destroy();

bool try_acquire_lock(MDL_request *mdl_request);
MDL_ticket *try_acquire_lock(MDL_key::enum_mdl_namespace mdl_namespace,
const char *db, const char *name,
enum_mdl_type mdl_type,
enum_mdl_duration mdl_duration);
bool acquire_lock(MDL_request *mdl_request, double lock_wait_timeout);
bool acquire_locks(MDL_request_list *requests, double lock_wait_timeout);
bool upgrade_shared_lock(MDL_ticket *mdl_ticket,
enum_mdl_type new_type,
double lock_wait_timeout);

MDL_ticket *acquire_lock(MDL_key::enum_mdl_namespace mdl_namespace,
const char *db, const char *name,
enum_mdl_type mdl_type,
enum_mdl_duration mdl_duration,
double lock_wait_timeout,
const char *src_file, uint src_line);
MDL_ticket *acquire_lock(const MDL_key *key,
enum_mdl_type mdl_type,
enum_mdl_duration mdl_duration,
double lock_wait_timeout,
const char *src_file, uint src_line);

bool clone_ticket(MDL_request *mdl_request);

void release_all_locks_for_name(MDL_ticket *ticket);
Expand Down
Loading
Loading