Details
-
Bug
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
-
10.6.18
-
None
Description
The following code in lock_release_on_prepare_try() is redundant after MDEV-33454 implementation:
if (!supremum_bit && rec_granted_exclusive_not_gap) |
continue; |
Because we should not skip granted X-locks and should invoke lock_rec_unlock_unmodified() for X-locks.
The following test case shows the issue:
--source include/have_innodb.inc
|
--source include/count_sessions.inc
|
|
CREATE TABLE t1 (a int, b int, c int,
|
INDEX i1(a),
|
INDEX i2(b))
|
ENGINE=InnoDB;
|
|
INSERT INTO t1 VALUES
|
(1,1,0), (1,2,0),
|
(2,1,0), (2,2,0);
|
|
SET @old_timeout= @@GLOBAL.innodb_lock_wait_timeout;
|
SET GLOBAL innodb_lock_wait_timeout= 1;
|
|
XA START "t1";
|
UPDATE t1 FORCE INDEX (i2) SET c=c+1 WHERE a=1 AND b=1;
|
XA END "t1";
|
XA PREPARE "t1";
|
|
--connect(con1,localhost,root,,)
|
# (pk, 2, 1, 0) record is X-locked but not modified in clustered index with the
|
# above XA transaction, if the bug is not fixed, the following SELECT will be
|
# blocked by the XA transaction if XA PREPARE does not release the unmodified
|
# record.
|
SELECT * FROM t1 FORCE INDEX (i1) WHERE a=2 AND b=1 FOR UPDATE;
|
--disconnect con1
|
|
--connection default
|
XA COMMIT "t1";
|
DROP TABLE t1;
|
|
SET GLOBAL innodb_lock_wait_timeout= @old_timeout;
|
--source include/wait_until_count_sessions.inc
|
Thanks to Elkin for finding the bug.
Attachments
Issue Links
- is blocked by
-
-
- Closed
-
- is caused by
-
MDEV-33454 release row locks for non-modified rows at XA PREPARE
-
- Closed
-
- relates to
-
-
- Confirmed
-
-
-
- Open
-
Activity
Where do we get an error between send_result() and close_thread_tables() ?
The XA transaction should not be affected.
Calling rollback on the LOAD INDEX INTO CACHE should not be affected by any XA transaction
Why is thd->transaction->xid_state.is_explicit_XA() set for the LOAD INDEX query?
The above bug was already filed in MDEV-19555, see stack trace in the bug descriptions. I agree with monty that the fix could be in calling xa_id_state.set_error() only if is_error() is not set. As the bug does not relate to the current fix and, moreover, does not affect release build, I will leave the correspondent comments in MDEV-19555, and propose to fix it separately from the current issue.
--- a/sql/handler.cc
|
+++ b/sql/handler.cc
|
@@ -2326,7 +2326,8 @@ int ha_rollback_trans(THD *thd, bool all) |
transaction hasn't been started in any transactional storage engine.
|
*/
|
if (thd->transaction_rollback_request && |
- thd->transaction->xid_state.is_explicit_XA())
|
+ thd->transaction->xid_state.is_explicit_XA() &&
|
+ thd->is_error())
|
thd->transaction->xid_state.set_error(thd->get_stmt_da()->sql_errno());
|
|
thd->has_waiter= false; |
|
ralf.gebhardt, this ticket is tightly coupled with MDEV-34690, i.e. there is no sense to push this fix without MDEV-34690 fix. See MDEV-34690 update.
I spent several hours analyzing the rr replay traces for transaction history corruption (MDEV-35227), which was reported by mleich while testing this. It is unrelated to these changes.
The code changes here and in MDEV-34690 look OK to me.
The above bug fix is incorrect.
The original clear_error() was there to avoid any previous errors sent from the engine as the following code is creating a result set with the errors.
Any error after this should be treated as an error.
The code in ha_rollback_trans() does:
if (is_real_trans)
{
/*
Thanks to possibility of MDL deadlock rollback request can come even if
transaction hasn't been started in any transactional storage engine.
*/
if (thd->transaction_rollback_request &&
thd->transaction->xid_state.is_explicit_XA())
thd->transaction->xid_state.set_error(thd->get_stmt_da()->sql_errno());
The above calls sql_errno() without checking if there is an active error, like 'if (thd->is_error()'
Would it be a problem to not call xa_id_state.set_error() if is_error() is not set or set it to 0 or ER_UNKNOWN_ERROR ?