Re: [PATCH 4/4] txn: undo commit/rollback triggers when reverting to savepoint

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Fri, Jul 19, 2019 at 09:08:42PM +0300, Vladimir Davydov wrote:
> When reverting to a savepoint inside a DDL transaction, apart from
> undoing changes done by the DDL statements to the system spaces, we also
> have to
> 
>  - Run rollback triggers installed after the savepoint was set, because
>    otherwise changes done to the schema by DDL won't be undone.
>  - Remove commit triggers installed after the savepoint, because they
>    are not relevant anymore, apparently.
> 
> To achieve that, let's remember not only the last transaction statement
> at the time when a savepoint is created, but also the state of commit
> and rollback triggers. Since in contrast to txn statements, commit and
> rollback triggers can actually be deleted from the list, we create dummy
> triggers per each savepoint and use them as markers in the trigger
> lists. We don't however create dummy triggers if no commit/rollback
> triggers is installed, because in that case we would simply need to
> clear the trigger lists.
> 
> While we are at it, let's also make sure that a rollback trigger doesn't
> an already freed tuple. To do that, we release statement tuples after
> running rollback triggers, not before as we used to.
> 
> Closes #4364
> Closes #4365

As discussed f2f, this and the previous patch are also needed to make
savepoints handle user-defined commit/rollback triggers correctly, i.e.
on rollback to a savepoint, remove commit triggers and run rollback
triggers installed after the savepoint. I added a test case to check
this behavior:

diff --git a/test/engine/transaction.result b/test/engine/transaction.result
index 910abb58..0b3fccf1 100644
--- a/test/engine/transaction.result
+++ b/test/engine/transaction.result
@@ -527,3 +527,77 @@ stmts
 s:drop()
 ---
 ...
+--
+-- Check that on rollback to a savepoint we remove all commit
+-- triggers and run all rollback triggers installed after the
+-- savepoint.
+--
+s = box.schema.space.create('test', {engine = engine})
+---
+...
+_ = s:create_index('pk')
+---
+...
+commit = {}
+---
+...
+rollback = {}
+---
+...
+on_commit1 = function() table.insert(commit, 'on_commit_1') end
+---
+...
+on_commit2 = function() table.insert(commit, 'on_commit_2') end
+---
+...
+on_commit3 = function() table.insert(commit, 'on_commit_3') end
+---
+...
+on_rollback1 = function() table.insert(rollback, 'on_rollback_1') end
+---
+...
+on_rollback2 = function() table.insert(rollback, 'on_rollback_2') end
+---
+...
+on_rollback3 = function() table.insert(rollback, 'on_rollback_3') end
+---
+...
+inspector:cmd("setopt delimiter ';'")
+---
+- true
+...
+box.begin()
+box.on_commit(on_commit1)
+box.on_rollback(on_rollback1)
+sv = box.savepoint()
+s:insert{1}
+box.on_commit(on_commit2)
+box.on_rollback(on_rollback2)
+s:insert{2}
+box.on_commit(on_commit3)
+box.on_rollback(on_rollback3)
+box.rollback_to_savepoint(sv)
+s:insert{3}
+box.commit();
+---
+...
+inspector:cmd("setopt delimiter ''");
+---
+- true
+...
+commit -- on_commit_1
+---
+- - on_commit_1
+...
+rollback -- on_rollback_3, on_rollback_2
+---
+- - on_rollback_3
+  - on_rollback_2
+...
+s:select() -- 3
+---
+- - [3]
+...
+s:drop()
+---
+...
diff --git a/test/engine/transaction.test.lua b/test/engine/transaction.test.lua
index 51756f93..75bb5f66 100644
--- a/test/engine/transaction.test.lua
+++ b/test/engine/transaction.test.lua
@@ -228,3 +228,42 @@ box.begin() box.on_commit(on_commit) box.commit()
 stmts
 
 s:drop()
+
+--
+-- Check that on rollback to a savepoint we remove all commit
+-- triggers and run all rollback triggers installed after the
+-- savepoint.
+--
+s = box.schema.space.create('test', {engine = engine})
+_ = s:create_index('pk')
+
+commit = {}
+rollback = {}
+on_commit1 = function() table.insert(commit, 'on_commit_1') end
+on_commit2 = function() table.insert(commit, 'on_commit_2') end
+on_commit3 = function() table.insert(commit, 'on_commit_3') end
+on_rollback1 = function() table.insert(rollback, 'on_rollback_1') end
+on_rollback2 = function() table.insert(rollback, 'on_rollback_2') end
+on_rollback3 = function() table.insert(rollback, 'on_rollback_3') end
+
+inspector:cmd("setopt delimiter ';'")
+box.begin()
+box.on_commit(on_commit1)
+box.on_rollback(on_rollback1)
+sv = box.savepoint()
+s:insert{1}
+box.on_commit(on_commit2)
+box.on_rollback(on_rollback2)
+s:insert{2}
+box.on_commit(on_commit3)
+box.on_rollback(on_rollback3)
+box.rollback_to_savepoint(sv)
+s:insert{3}
+box.commit();
+inspector:cmd("setopt delimiter ''");
+
+commit -- on_commit_1
+rollback -- on_rollback_3, on_rollback_2
+s:select() -- 3
+
+s:drop()