Skip to content

Commit

Permalink
Fix deadlocks in STMT dep repl caused due to innodb gap locks and com…
Browse files Browse the repository at this point in the history
…mit ordering

Summary:
InnoDB can sometimes take gap locks on unique secondary indices
even when the isolation level is READ-COMMITTED. This doesn't happen all
the time but only during some very special circumstances (possibly
during an ongoing page split).

Since dependency replication only looks at keys in the row images to
figure out conflicts in transactions, gap locking can cause deadlocks
with commit ordering is enabled. This is best explained using an
example:

Say trx1 and trx2 are two independent transactions (i.e. their read and
write sets do not intersect), so dep repl will schedule them to run in
pararllel. However they will conflict on a gap lock.  Now, say trx2
aquires the conflicting gap lock 1st, finishes execution and then waits
for trx1 to commit before flushing to binlog in ordered commit because
commit ordering is enabled. This leads to a deadlock, since trx1 is
waiting for trx2 to release the gap lock but trx2 is waiting for trx1 to
commit while holding the gap lock because commit ordering is enabled.

Vanilla appliers since 5.7+ that allow parallism within a table also
faced this issue and solved it by implementing a callback from the
storage engine that tells the applier when a lock conflict is detected.
The applier then rolls back the later trx (releasing the gap lock) and
allows the earlier trx to move forward.

This patch is basically a port of this patch from vanilla MySQL:
mysql/mysql-server@61e2172a188

Reviewed By: hermanlee

Differential Revision: D28619160

fbshipit-source-id: 9e066576f0c
  • Loading branch information
abhinav04sharma authored and facebook-github-bot committed May 26, 2021
1 parent 6e53ec0 commit ce8c474
Show file tree
Hide file tree
Showing 25 changed files with 528 additions and 17 deletions.
44 changes: 44 additions & 0 deletions include/mysql/service_thd_engine_lock.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@

/* Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; version 2 of the License.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */

#ifndef MYSQL_SERVICE_THD_EGINE_LOCK_INCLUDED
#define MYSQL_SERVICE_THD_EGINE_LOCK_INCLUDED

/**
@file include/mysql/service_thd_engine_lock.h
This service provides functions for storage engines to report
lock related activities.
SYNOPSIS
thd_row_lock_wait() - call it just when the engine find a transaction should
wait another transaction to realease a row lock
thd The session which is waiting for the row lock to release
thd_wait_for The session which is holding the row lock.
*/

#ifdef __cplusplus
class THD;
#else
#define THD void
#endif

#ifdef __cplusplus
extern "C" {
#endif

void thd_report_row_lock_wait(THD* self, THD *wait_for);

#ifdef __cplusplus
}
#endif

#endif
4 changes: 4 additions & 0 deletions mysql-test/include/have_mts_dependency_replication_stmt.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
if (`SELECT @@GLOBAL.mts_dependency_replication != 'STMT'`)
{
skip Test needs to run with STMT dependency replication;
}
21 changes: 21 additions & 0 deletions mysql-test/suite/innodb/r/innodb_row_lock_wait_callback.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
create table t1 (a int primary key, b int unique, c int) engine = innodb;
insert into t1 values(1, 1, 1);
insert into t1 values(10, 10, 10);
set @@global.debug = "+d,report_row_lock_wait";
select @@tx_isolation;
@@tx_isolation
REPEATABLE-READ
begin;
delete from t1 where a > 5;
begin;
insert into t1 values(6, 6, 6);
set debug_sync="now wait_for signal.reached";
set debug_sync="now signal signal.done";
set @@global.debug = "-d,report_row_lock_wait";
rollback;
select * from t1;
a b c
1 1 1
6 6 6
10 10 10
drop table t1;
30 changes: 30 additions & 0 deletions mysql-test/suite/innodb/t/innodb_row_lock_wait_callback.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
source include/have_debug.inc;
source include/have_innodb.inc;
source include/have_debug_sync.inc;

create table t1 (a int primary key, b int unique, c int) engine = innodb;
insert into t1 values(1, 1, 1);
insert into t1 values(10, 10, 10);
set @@global.debug = "+d,report_row_lock_wait";

select @@tx_isolation;

connect (con1, localhost, root);
begin;
delete from t1 where a > 5; # this will take a gap lock

connection default;
begin;
send insert into t1 values(6, 6, 6); # this will block on gap lock

connection con1;
set debug_sync="now wait_for signal.reached"; # callback was fired
set debug_sync="now signal signal.done";
set @@global.debug = "-d,report_row_lock_wait";
rollback;
disconnect con1;

connection default;
reap;
select * from t1;
drop table t1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
include/master-slave.inc
Warnings:
Note #### Sending passwords in plain text without SSL/TLS is extremely insecure.
Note #### Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
[connection master]
call mtr.add_suppression("Commit order deadlock between");
create table t1 (a int primary key, b int) engine = innodb;
insert into t1 values(3, 3);
include/sync_slave_sql_with_master.inc
stop slave;
set @@global.debug = "+d,dbug.dep_fake_gap_lock_on_insert";
begin;
insert into t1 values(1, 1);
insert into t1 values(1, 1);
insert into t1 values(2, 2);
update t1 set b = 20 where a = 2;
update t1 set b = 200 where a = 2;
update t1 set b = 30 where a = 3;
update t1 set b = 300 where a = 3;
start slave;
rollback;
include/sync_slave_sql_with_master.inc
select * from t1;
a b
1 1
2 200
3 300
stop slave;
set @@global.debug = "-d,dbug.dep_fake_gap_lock_on_insert";
start slave;
drop table t1;
include/sync_slave_sql_with_master.inc
include/rpl_end.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--slave_check_before_image_consistency=ON
--slave_parallel_workers=8
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
source include/master-slave.inc;
source include/have_mts_dependency_replication_stmt.inc;
source include/have_debug.inc;

call mtr.add_suppression("Commit order deadlock between");

connection master;
create table t1 (a int primary key, b int) engine = innodb;
insert into t1 values(3, 3);
source include/sync_slave_sql_with_master.inc;

connection slave;
stop slave;
# We'll take a fake gap lock after execution of every insert event
set @@global.debug = "+d,dbug.dep_fake_gap_lock_on_insert";

# Start a trx to block the 1st insert that primary will send to create a gap in
# commit order
begin;
insert into t1 values(1, 1);

connection master;
insert into t1 values(1, 1); # this will be blocked by trx above
insert into t1 values(2, 2); # this will take the fake gap lock and wait for commit order
update t1 set b = 20 where a = 2; # this will wait for above trx due to deps
update t1 set b = 200 where a = 2; # this will wait for above trx due to deps
update t1 set b = 30 where a = 3; # this will wait for commit order
update t1 set b = 300 where a = 3; # this will wait for above trx due to deps

connection slave1;
start slave;
# Wait for the 2nd and 5th trx to start waiting for commit ordering
let $wait_condition= SELECT COUNT(*) = 2 FROM INFORMATION_SCHEMA.PROCESSLIST
WHERE STATE LIKE "%Waiting for preceding transaction to commit%";
let $wait_timeout= 120;
source include/wait_condition.inc;

# Wait for the 3rd, 4th and 6th trx to wait for dependencies
let $wait_condition= SELECT COUNT(*) = 3 FROM INFORMATION_SCHEMA.PROCESSLIST
WHERE STATE LIKE "%Waiting for dependencies to be satisfied%";
let $wait_timeout= 120;
source include/wait_condition.inc;

connection slave;
# unblock 1st insert trx, it'll try to lock fake gap and fire the callback
rollback;

# Wait for 2nd transcation to be retried after receiving the commit order
# deadlock signal
let $wait_condition= SELECT VARIABLE_VALUE = 1
FROM INFORMATION_SCHEMA.GLOBAL_STATUS
WHERE VARIABLE_NAME = 'Slave_commit_order_deadlocks';
let $wait_timeout= 120;
source include/wait_condition.inc;

connection master;
source include/sync_slave_sql_with_master.inc;

connection slave;
select * from t1;

stop slave;
set @@global.debug = "-d,dbug.dep_fake_gap_lock_on_insert";
start slave;

connection master;
drop table t1;
source include/sync_slave_sql_with_master.inc;

source include/rpl_end.inc;
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
Valid values are 'ON' and 'OFF'
select @@global.innodb_enable_row_lock_wait_callback;
@@global.innodb_enable_row_lock_wait_callback
1
select @@session.innodb_enable_row_lock_wait_callback;
ERROR HY000: Variable 'innodb_enable_row_lock_wait_callback' is a GLOBAL variable
show global variables like 'innodb_enable_row_lock_wait_callback';
Variable_name Value
innodb_enable_row_lock_wait_callback ON
show session variables like 'innodb_enable_row_lock_wait_callback';
Variable_name Value
innodb_enable_row_lock_wait_callback ON
select * from information_schema.global_variables where variable_name='innodb_enable_row_lock_wait_callback';
VARIABLE_NAME VARIABLE_VALUE
INNODB_ENABLE_ROW_LOCK_WAIT_CALLBACK ON
select * from information_schema.session_variables where variable_name='innodb_enable_row_lock_wait_callback';
VARIABLE_NAME VARIABLE_VALUE
INNODB_ENABLE_ROW_LOCK_WAIT_CALLBACK ON
set session innodb_enable_row_lock_wait_callback=1;
ERROR HY000: Variable 'innodb_enable_row_lock_wait_callback' is a GLOBAL variable and should be set with SET GLOBAL
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--source include/have_innodb.inc

# Can only be set from the command line.
# show the global and session values;

--echo Valid values are 'ON' and 'OFF'
select @@global.innodb_enable_row_lock_wait_callback;
--error ER_INCORRECT_GLOBAL_LOCAL_VAR
select @@session.innodb_enable_row_lock_wait_callback;
show global variables like 'innodb_enable_row_lock_wait_callback';
show session variables like 'innodb_enable_row_lock_wait_callback';
select * from information_schema.global_variables where variable_name='innodb_enable_row_lock_wait_callback';
select * from information_schema.session_variables where variable_name='innodb_enable_row_lock_wait_callback';

--error 1229
set session innodb_enable_row_lock_wait_callback=1;

35 changes: 35 additions & 0 deletions sql/dependency_slave_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "dependency_slave_worker.h"
#include "log_event_wrapper.h"
#include "rpl_slave_commit_order_manager.h"
#include <../include/mysql/service_thd_engine_lock.h>


bool append_item_to_jobs(slave_job_item *job_item,
Expand Down Expand Up @@ -89,16 +90,50 @@ bool Dependency_slave_worker::execute_group()
c_rli->dependency_worker_error= true;
break;
}

DBUG_EXECUTE_IF("dbug.dep_fake_gap_lock_on_insert", {
if (!ev->is_end_event && ev->raw_event() &&
ev->raw_event()->get_type_code() == WRITE_ROWS_EVENT)
{
if (!c_rli->dep_fake_gap_lock.try_lock())
{
thd_report_row_lock_wait(
info_thd, c_rli->dep_fake_gap_lock_worker->info_thd);
c_rli->dep_fake_gap_lock.lock();
c_rli->dep_fake_gap_lock_worker= this;
}
else
{
c_rli->dep_fake_gap_lock_worker= this;
}
}
};);

// case: restart trx if temporary error, see @slave_worker_ends_group
if (unlikely(trans_retries && current_event_index == 0))
{
DBUG_EXECUTE_IF("dbug.dep_fake_gap_lock_on_insert", {
if (this == c_rli->dep_fake_gap_lock_worker)
{
c_rli->dep_fake_gap_lock_worker= nullptr;
c_rli->dep_fake_gap_lock.unlock();
}
};);
ev= begin_event;
continue;
}
finalize_event(ev);
ev= ev->next();
}

DBUG_EXECUTE_IF("dbug.dep_fake_gap_lock_on_insert", {
if (this == c_rli->dep_fake_gap_lock_worker)
{
c_rli->dep_fake_gap_lock_worker= nullptr;
c_rli->dep_fake_gap_lock.unlock();
}
};);

// case: in case of error rollback if commit ordering is enabled
if (unlikely(err && commit_order_mngr))
{
Expand Down
Loading

0 comments on commit ce8c474

Please sign in to comment.