From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 26 Jul 2019 11:56:07 +0300 From: Vladimir Davydov Subject: Re: [PATCH 4/4] txn: undo commit/rollback triggers when reverting to savepoint Message-ID: <20190726085607.GQ24631@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: 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()