Details
-
Bug
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Duplicate
-
10.0.11, 10.0, 10.1, 10.2, 10.3, 10.4, 10.5
Description
In MariaDB Server 10.0.11, a change from MySQL 5.6.16 was applied.
As far as I understand, the intention of the ha_rollback_to_savepoint_can_release_mdl() check is to ensure that no other transaction inside InnoDB is waiting for a lock on the table that the ROLLBACK TO SAVEPOINT would release. But, the check seems to be misplaced. We could have the following happen:
- innobase_rollback_to_savepoint_can_release_mdl() returns true, because the transaction only had placed implicit record locks. (Until MDEV-16232 is implemented, this is only possible if the transaction performed nothing else than INSERT operations).
- Another transaction starts to wait for a record lock that our transaction had implicitly locked. The implicit lock will be converted to an explicit one.
- ha_rollback_to_savepoint() will be executed. No explicit locks will be released, because we did not roll back the entire transaction.
- The MDL will be released in thd->mdl_context.rollback_to_savepoint(), even though the other transaction will be blocked inside InnoDB until our transaction has been committed or rolled back.
Note: In innobase_rollback_to_savepoint_can_release_mdl() there is another race that would be fixed by the first hunk of the following untested patch:
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
|
index 7cc4401d0ff..bca47f011a6 100644
|
--- a/storage/innobase/handler/ha_innodb.cc
|
+++ b/storage/innobase/handler/ha_innodb.cc
|
@@ -4999,15 +4999,14 @@ innobase_rollback_to_savepoint_can_release_mdl(
|
DBUG_ASSERT(hton == innodb_hton_ptr);
|
|
trx_t* trx = check_trx_exists(thd);
|
+ trx_mutex_enter(trx);
|
+ ulint n_locks = UT_LIST_GET_LEN(trx->lock.trx_locks) == 0;
|
+ trx_mutex_exit(trx);
|
+
|
|
/* If transaction has not acquired any locks then it is safe
|
to release MDL after rollback to savepoint */
|
- if (UT_LIST_GET_LEN(trx->lock.trx_locks) == 0) {
|
-
|
- DBUG_RETURN(true);
|
- }
|
-
|
- DBUG_RETURN(false);
|
+ DBUG_RETURN(!n_locks);
|
}
|
|
/*****************************************************************//**
|
diff --git a/sql/transaction.cc b/sql/transaction.cc
|
index 543e0b7ad38..66b40d23e80 100644
|
--- a/sql/transaction.cc
|
+++ b/sql/transaction.cc
|
@@ -718,10 +718,6 @@ bool trans_rollback_to_savepoint(THD *thd, LEX_STRING name)
|
For backward-compatibility reasons we always release MDL if binary
|
logging is off.
|
*/
|
- bool mdl_can_safely_rollback_to_savepoint=
|
- (!(mysql_bin_log.is_open() && thd->variables.sql_log_bin) ||
|
- ha_rollback_to_savepoint_can_release_mdl(thd));
|
-
|
if (ha_rollback_to_savepoint(thd, sv))
|
res= TRUE;
|
else if (((thd->variables.option_bits & OPTION_KEEP_LOG) ||
|
@@ -733,7 +729,8 @@ bool trans_rollback_to_savepoint(THD *thd, LEX_STRING name)
|
|
thd->transaction.savepoints= sv;
|
|
- if (!res && mdl_can_safely_rollback_to_savepoint)
|
+ if (!res && (!(mysql_bin_log.is_open() && thd->variables.sql_log_bin) ||
|
+ ha_rollback_to_savepoint_can_release_mdl(thd)))
|
thd->mdl_context.rollback_to_savepoint(sv->mdl_savepoint);
|
|
DBUG_RETURN(MY_TEST(res)); |
Note: I did not review whether this could break something in other implementations of handlerton::savepoint_rollback_can_release_mdl.
Attachments
Issue Links
- duplicates
-
-
- Closed
-
-
-
- Closed
-
- relates to
-
-
- Closed
-
-
-
- Closed
-