Skip to content

Commit

Permalink
txn: run on_rollback triggers on rollback to savepoint
Browse files Browse the repository at this point in the history
Currently, if a statement is rolled back during rollback to a savepoint, it
does not appear in neither on_commit nor on_rollback triggers. Fix this by
running on_rollback triggers during the rollback to the savepoint.

Closes tarantool#7810

NO_DOC=bugfix
  • Loading branch information
Gumix committed Nov 20, 2023
1 parent cc700f4 commit ed908f1
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## bugfix/core

* Fixed a bug when `on_rollback` triggers were not invoked during a rollback
to a savepoint (gh-7810).
2 changes: 1 addition & 1 deletion src/box/txn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1437,7 +1437,7 @@ box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp)
diag_set(ClientError, ER_NO_SUCH_SAVEPOINT);
return -1;
}
txn_rollback_to_svp(txn, svp->stmt, false);
txn_rollback_to_svp(txn, svp->stmt, true);
/* Discard from list all newer savepoints. */
RLIST_HEAD(discard);
rlist_cut_before(&discard, &txn->savepoints, &svp->link);
Expand Down
52 changes: 52 additions & 0 deletions test/engine-luatest/gh_9340_on_rollback_trigger_args_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,58 @@ g_mvcc_on.test_trigger_args = function(cg)
box.commit()
s2:on_replace(nil, error_in_on_replace)
t.assert_equals(_G.result, {})

-- Check rollback to savepoint (with empty txn).
box.begin()
local svp = box.savepoint()
box.rollback_to_savepoint(svp)
t.assert_equals(_G.result, {})
box.commit()
t.assert_equals(_G.result, {})

-- Check rollback to savepoint (with txn commit).
box.begin()
s1:insert{8}
s2:insert{9}
local svp1 = box.savepoint()
s1:insert{10}
local svp2 = box.savepoint()
s2:insert{11}
box.rollback_to_savepoint(svp2)
t.assert_equals(_G.result, {
{num = 1, space_id = 513, old_tuple = nil, new_tuple = {11}},
})
s1:insert{12}
box.rollback_to_savepoint(svp1)
t.assert_equals(_G.result, {
{num = 1, space_id = 512, old_tuple = nil, new_tuple = {12}},
{num = 2, space_id = 512, old_tuple = nil, new_tuple = {10}},
})
_G.result = {}
box.commit()
t.assert_equals(_G.result, {})

-- Check rollback to savepoint (with txn rollback).
box.begin()
s1:insert{13}
s2:insert{14}
local svp = box.savepoint()
s1:insert{15}
s2:insert{16}
box.rollback_to_savepoint(svp)
t.assert_equals(_G.result, {
{num = 1, space_id = 513, old_tuple = nil, new_tuple = {16}},
{num = 2, space_id = 512, old_tuple = nil, new_tuple = {15}},
})
s1:insert{17}
s2:insert{18}
box.rollback()
t.assert_equals(_G.result, {
{num = 1, space_id = 513, old_tuple = nil, new_tuple = {18}},
{num = 2, space_id = 512, old_tuple = nil, new_tuple = {17}},
{num = 3, space_id = 513, old_tuple = nil, new_tuple = {14}},
{num = 4, space_id = 512, old_tuple = nil, new_tuple = {13}},
})
end, {cg.params.engine})
end

Expand Down
8 changes: 4 additions & 4 deletions test/engine/savepoint.result
Original file line number Diff line number Diff line change
Expand Up @@ -560,12 +560,12 @@ did_rollback = false
box.begin() \
svp = box.savepoint() \
s:replace{4} \
box.on_rollback(function() did_rollback = true assert(false) end) \
box.on_rollback(function() did_rollback = true end) \
box.rollback_to_savepoint(svp) \
box.commit()
---
...
assert(not did_rollback)
assert(did_rollback)
---
- true
...
Expand All @@ -574,12 +574,12 @@ assert(not did_rollback)
box.begin() \
svp = box.savepoint() \
box.schema.create_space('test2', {engine = engine}) \
box.on_rollback(function() did_rollback = true assert(false) end) \
box.on_rollback(function() did_rollback = true end) \
box.rollback_to_savepoint(svp) \
box.commit()
---
...
assert(not did_rollback)
assert(did_rollback)
---
- true
...
Expand Down
8 changes: 4 additions & 4 deletions test/engine/savepoint.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -285,20 +285,20 @@ did_rollback = false
box.begin() \
svp = box.savepoint() \
s:replace{4} \
box.on_rollback(function() did_rollback = true assert(false) end) \
box.on_rollback(function() did_rollback = true end) \
box.rollback_to_savepoint(svp) \
box.commit()
assert(not did_rollback)
assert(did_rollback)

-- Try DDL too. It installs the flag in the statement that it has triggers.
-- This should not change the behaviour.
box.begin() \
svp = box.savepoint() \
box.schema.create_space('test2', {engine = engine}) \
box.on_rollback(function() did_rollback = true assert(false) end) \
box.on_rollback(function() did_rollback = true end) \
box.rollback_to_savepoint(svp) \
box.commit()
assert(not did_rollback)
assert(did_rollback)
assert(not box.schema.space.test2)

s:drop()

0 comments on commit ed908f1

Please sign in to comment.