Details
-
Bug
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
-
10.2.2, 10.3.0
Description
MySQL 5.7.4 changed the behaviour of INSERT…ON DUPLICATE KEY UPDATE in the InnoDB storage engine. Upon encountering a duplicate key, it would no longer directly fall back to INSERT, but instead it would proceed to acquire an exclusive lock on every index record for the row on which the UPDATE failed.
The extra locking was motivated by a public bug report: MySQL Bug#50413 insert on duplicate key update sometimes writes binlog position incorrectly (Oracle internal BUG#11758237). The fix was followed up by a couple of regression fixes. For one user, reverting these changes significantly reduces the deadlock rate of INSERT…ON DUPLICATE KEY UPDATE.
There also is a related MySQL Bug #52020 InnoDB can still deadlock on just INSERT...ON DUPLICATE KEY. One of the factors was that when the pluggable storage engine interface was created in MySQL 5.1, the function innobase_query_is_update() was removed without replacement, and MySQL Bug #7975 (which lacked a test case) was reintroduced.
In a comment in MySQL Bug #52020 I anticipated that the deadlocks would be caused in a scenario where the INSERT phase fails, then some other transaction locks some of the index records, causing the ON DUPLICATE KEY UPDATE phase to wait for those locks or to deadlock. Acquiring the locks for all index records already in the INSERT phase would make the UPDATE phase wait-free, but it could cause more conflicts with other accesses, as the hold time of the locks is extended.
valerii posted some insightful comments on Bug #52020. I hope he can construct a test case that demonstrates the increased deadlock rate, so that we can see what can be improved here.
Sven Sandberg suggested in MySQL Bug #50413 that the INSERT phase should have acquired a gap lock, so that conflicting INSERT with that key would be prevented. I assume that he meant the PRIMARY key, because his example involves two unique keys: PRIMARY KEY(a), UNIQUE KEY(b). He also filed MySQL Bug #58637 Mark INSERT...ON DUPLICATE KEY UPDATE unsafe when there is more than one key.
Apparently the nondeterminism that the extra locking is trying to prevent is caused by the ambiguity of the ON DUPLICATE KEY syntax. It does not specify the key! An unambiguous syntax would be something like:
INSERT INTO t1 VALUES(1,2,3) ON DUPLICATE KEY(PRIMARY) UPDATE …; |
INSERT INTO t1 VALUES(1,2,3) ON DUPLICATE KEY(u) UPDATE …; |
Statement-based replication is obviously affected by this ambiguity.
I hope that Elkin and seppo can comment on whether row-based replication and parallel forms of replication (including Galera Cluster and MySQL 5.7 group replication) are affected, and how exactly the operations would be logged by the master and applied on the slave.
Note: Comments in MySQL Bug #50413 suggest that innodb_autoinc_lock_mode settings 0 and 1 are equivalent in this respect. I’d also like to know whether this parameter is at all relevant outside statement-based replication (that is, when innodb_autoinc_lock_mode=2 could be safe to use). With the setting 2, InnoDB does not acquire any auto-increment lock within the transaction. With the settings 0 or 1, InnoDB will hold a lock until the end of the current statement. This would suggest that the setting only matters in statement-based replication.
Attachments
Issue Links
- causes
-
-
- Closed
-
- relates to
-
-
- Closed
-
-
-
- Open
-
-
-
- Closed
-
-
-
- Closed
-
-
-
- Closed
-
-
-
- Closed
-
-
-
- In Review
-
Activity
As a simple fix, I would keep the extra locking only when binlog is enabled and statement-based replication is in use. In this way, we will get deterministic locking on master and slave, while not paying penalty for standalone operation or row-based replication:
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
|
index e581ed0af25..71d5b80eaa3 100644
|
--- a/sql/sql_class.cc
|
+++ b/sql/sql_class.cc
|
@@ -4523,6 +4523,11 @@ extern "C" int thd_rpl_is_parallel(const MYSQL_THD thd)
|
return thd->rgi_slave && thd->rgi_slave->is_parallel_exec;
|
}
|
|
+extern "C" int thd_rpl_stmt_based(const MYSQL_THD thd)
|
+{
|
+ return !thd->is_current_stmt_binlog_format_row() &&
|
+ !thd->is_current_stmt_binlog_disabled();
|
+}
|
|
/* Returns high resolution timestamp for the start
|
of the current query. */ |
I pushed a fix to 10.2. With the following patch, it trips a check at the upper level:
diff --git a/mysql-test/suite/innodb/include/innodb_binlog.combinations b/mysql-test/suite/innodb/include/innodb_binlog.combinations
|
index 46d31e733b1..c4286c0a171 100644
|
--- a/mysql-test/suite/innodb/include/innodb_binlog.combinations
|
+++ b/mysql-test/suite/innodb/include/innodb_binlog.combinations
|
@@ -1,3 +1,4 @@
|
[log-bin]
|
log-bin
|
+binlog-format=statement
|
[skip-log-bin] |
innodb.auto_increment_dup 'innodb,log-bin' [ fail ]
|
Test ended at 2018-11-02 15:05:53
|
|
|
CURRENT_TEST: innodb.auto_increment_dup
|
mysqltest: At line 146: query 'INSERT INTO t1(k) VALUES (2), (4), (5) ON DUPLICATE KEY UPDATE c='2'' failed: 1665: Cannot execute statement: impossible to write to binary log since BINLOG_FORMAT = STATEMENT and at least one table uses a storage engine limited to row-based logging. InnoDB is limited to row-logging when transaction isolation level is READ COMMITTED or READ UNCOMMITTED.
|
Maybe a better fix would be to revert the upstream change entirely and improve the check that was originally added in MySQL 5.1.20 in ha_innobase::table_flags() and in 5.1.21 moved to ha_innobase::external_lock().
It turns out that upstream reverted the problematic fix and implemented a cleaner fix in MySQL 5.7.26.
In MariaDB Server 10.2 and later, thanks to MDEV-17614, replication will work correctly (or warnings will be issued to the user) in this scenario. The MDEV-17614 fix thus allows (and its test case requires) us to revise this MDEV-17073 fix. Instead of selectively disabling the problematic change that we inherited from MySQL 5.7, we must remove that problematic change altogether. I did that as part of merging MDEV-17614 from 10.1 to 10.2.
I created
MDEV-17522that has a test case showing deadlocks with INSERT ON DUPLICATE UPDATE and unique keys.