Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddl: transactional DDL does not care about savepoints #4364

Closed
Gerold103 opened this issue Jul 17, 2019 · 1 comment
Closed

ddl: transactional DDL does not care about savepoints #4364

Gerold103 opened this issue Jul 17, 2019 · 1 comment
Labels
bug Something isn't working ddl
Milestone

Comments

@Gerold103
Copy link
Collaborator

box.cfg{}
function ddl()
	local check1, check2, check3, check4
	local s1 = box.schema.create_space('test1')
	local save = box.savepoint()
	local s2 = box.schema.create_space('test2')
	check1 = box.space.test1 ~= nil
	check2 = box.space.test2 ~= nil
	box.rollback_to_savepoint(save)
	check3 = box.space.test1 ~= nil
	check4 = box.space.test2 ~= nil
	return check1, check2, check3, check4
end

res = nil
box.begin() res = {ddl()} box.commit()
res

Expected output, is that check4 is false, other checks are true. But reality is disappointing:

tarantool> res
---
- - true
  - true
  - true
  - true
...
@Gerold103 Gerold103 added bug Something isn't working ddl labels Jul 17, 2019
@locker locker self-assigned this Jul 18, 2019
@kyukhin kyukhin added this to the 2.2.0 milestone Jul 18, 2019
@Gerold103
Copy link
Collaborator Author

Gerold103 commented Jul 18, 2019

I thought, that I will fix that, but looks like Vova now is assigned. Nonetheless here are my thoughts on what I had planned to do. Perhaps, it will be useful.

The problem is that we can't set on_rollback triggers on individual statements. But we don't need to, strictly speaking. There are only 2 ways, as I know, how a statement can be rolled back:

  1. rollback the whole transaction;
  2. rollback to a savepoint.

All cases like rollback on yield, rollback on conflict, voluntary rollback, rollback on an error from WAL, are covered by the case one.
It means, that we don't need to have on_rollback triggers per statement - it would be too expensive. It is enough to have them for txn and for savepoint. Alter should set triggers on rollback to the most recent savepoint.

locker added a commit that referenced this issue Jul 19, 2019
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
locker added a commit that referenced this issue Jul 19, 2019
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
access 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
Gerold103 pushed a commit that referenced this issue Jul 19, 2019
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
access 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
locker added a commit that referenced this issue Jul 25, 2019
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
access 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
locker added a commit that referenced this issue Jul 26, 2019
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
access 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
locker added a commit that referenced this issue Jul 26, 2019
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
access 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
locker added a commit that referenced this issue Jul 29, 2019
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
access 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
@locker locker closed this as completed in 7ad7169 Jul 30, 2019
@kyukhin kyukhin added the tmp label Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ddl
Projects
None yet
Development

No branches or pull requests

4 participants