Skip to content

Commit

Permalink
Fix potential deadlock with LOCK_index between BinlogWrapper and rli_…
Browse files Browse the repository at this point in the history
…relay_log_raft_reset

Summary:
There's a potential deadlock where plugin's BinlogWrapper::IsReadyForAppend acquires smutex -> LOCK_index but in D57198631, we introduced order of LOCK_index->smutex in rli_relay_log_raft_reset()

To fix this, we can unlock the LOCK_index much earlier.

Test Plan:
```
~/mysql/tools/mysqltest-internal.sh --testset=Rpl_raft --raft --workers=4 --parallel=4 rpl_raft_purge_raft_logs_during_promotion
```

Reviewers: abhinavsharma, yichenshen, chili, #mysql_dar

Reviewed By: chili

Differential Revision: https://phabricator.intern.facebook.com/D59862336
  • Loading branch information
alanliang committed Jul 17, 2024
1 parent 20220f2 commit 1d51c17
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions sql/rpl_replica.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1444,6 +1444,7 @@ int rli_relay_log_raft_reset(
char *gtid_set_relay_log = nullptr;
int error = 0;
std::string normalized_log_name;
bool lock_global_log_index = opt_reset_rli_lock_index;

if (disable_raft_log_repointing) DBUG_RETURN(0);

Expand Down Expand Up @@ -1474,7 +1475,6 @@ int rli_relay_log_raft_reset(
original path, but ultimately fail due to the index file already being
recreated by this method.
*/
bool lock_global_log_index = opt_reset_rli_lock_index;
if (lock_global_log_index) mysql_bin_log.lock_index();

/*
Expand Down Expand Up @@ -1546,9 +1546,14 @@ int rli_relay_log_raft_reset(
error = 1;
mi->rli->relay_log.unlock_index();
mysql_mutex_unlock(mi->rli->relay_log.get_log_lock());
if (lock_global_log_index) mysql_bin_log.unlock_index();
goto end;
}

// We need to ensure we unlock before relay_log.register_log_entities is
// called (this is called later in this method) as this could create a deadlock.
if (lock_global_log_index) mysql_bin_log.unlock_index();

all_gtid_set = const_cast<Gtid_set *>(mi->rli->get_gtid_set());
if (all_gtid_set == nullptr) {
error = 1;
Expand Down Expand Up @@ -1620,7 +1625,6 @@ int rli_relay_log_raft_reset(

mysql_mutex_unlock(&mi->rli->data_lock);
mysql_mutex_unlock(&mi->data_lock);
if (lock_global_log_index) mysql_bin_log.unlock_index();
channel_map.unlock();
DBUG_RETURN(error);
}
Expand Down

0 comments on commit 1d51c17

Please sign in to comment.