FCO / Red

A WiP ORM for Raku
Artistic License 2.0
70 stars 27 forks source link

Nested transactions. #506

Open jonathanstowe opened 3 years ago

jonathanstowe commented 3 years ago

The SQL Standards are a bit handwavey about nested transactions and different DB backends handle it different, but for our current purposes both Pg and SQLite basically ignore any subsequent BEGIN before a COMMIT or ROLLBACK :

WARNING:  there is no transaction in progress
COMMIT
hes_test=> BEGIN;
BEGIN
hes_test=> BEGIN;
WARNING:  there is already a transaction in progress
BEGIN
hes_test=> COMMIT;
COMMIT
hes_test=> ROLLBACK;
WARNING:  there is no transaction in progress
ROLLBACK
hes_test=> \q
sqlite> create table bar (zub text);
sqlite> begin;
sqlite> insert into bar value ("foo");
SQL error: near "value": syntax error
sqlite> insert into bar values ("foo");
sqlite> begin;
SQL error: cannot start a transaction within a transaction
sqlite> commit;
sqlite> select * from bar;
foo
sqlite> rollback;
SQL error: cannot rollback - no transaction is active

This has implications for code like:

red-do {
   my $foo = Foo.^create: @bars => [{ name => 'bumble' });
   Zub.^create: foo-id => $foo.id;
   True;
}, :transaction;

Where the implicit transaction formed ( after the #505 ,) by the first statement in the red-do will create a warning about the nested transaction, but crucially the COMMIT of that implicit transaction will silently end the transaction formed by the red-do so a failure in the second statement will not rollback any changes from the first, the subsequent ROLLBACK emitting a warning.

An inadvertent side effect of the #504 is that a failure in the nested transaction will lead to a rollback of the outer transaction, however because they are in effect separate transactions (on different client connections,) a failure in the second transaction will have no effect on the first. Prior to #504 the behaviour would have depended on the number of concurrent clients of the driver (so essentially non-deterministic.)

So what I think needs to happen is that when the new-connection is called to get a new driver for begin then an e.g. transaction-depth should be set (on the driver,) then any subsequent begin should check this and if it is set should increment the value and do nothing else and for any commit or rollback should check the value and if it is not the original value then should should do nothing but decrement the value, if it is the original value set by the first begin then the commit or rollback should be performed (and the transaction-depth unset though it is assumed that this instance of the driver will go out of scope at this point.)

FCO commented 3 years ago

Maybe the rollback should work independently of the depth?

jonathanstowe commented 3 years ago

That only works if any inner transaction block propagates an exception that will skip any remaining outer calls. If an inner block signals that it wants a rollback by returning False (possibly determined by some non DB activity,) then calling the inner rollback will cause any remaining statements outside the block to not effectively be in a transaction any longer.

So something like:

redo-do {
   # do something to DB
   red-do {
       # do something to DB which succeeds
       ok-to-commit();  #  returns Bool,  False triggers a rollback
   }, :transaction;
   # do something to DB
}. :transaction;

Where the ok-to-commit may, for example, depend on the prior statements.

Now it could be that the False return from the inner block is upgraded to a 'Control Exception' in the nested case, which is caught by the most outward red-do, but the complexity does begin to stack up.

An alternative might be to maintain a stack of results (whether commit or rollback,) which is processed by the outer red-do and if any are rollbacks then a rollback is performed, else a commit.

I'm beginning to wonder whether a separate 'transaction manager' is the way to go.

FCO commented 3 years ago

I’m starting to agree that the transaction manager may be the way to go.

jonathanstowe commented 3 years ago

Another advantage of having a transaction manager is that if someone disagrees with the strategy chosen, or has differing requirements then they can implement their own. It also opens the possibility to distributed transaction management which is something I've been thinking about for a while.

FCO commented 3 years ago

I think that sounds great! Any suggestion on how that should work?

jonathanstowe commented 3 years ago

In a sort of handwavey way, I'd see there being a single transaction manager within the dynamic scope of an outer transaction (i.e the first red-do creates one,) which will create its own connection. It will have a begin method that may issue a BEGIN to the DB if necessary and will return a transaction object against which any 'rollback' or 'commit' which will have a promise held by the transaction manager which will choose if and when to issue the rollback or commit. For some drivers this may be more or less complicated due to their individual requirements.

For a hypothetical "distributed" or "multi-service" transaction manager, I'd imagine the manager would allow the registration of service drivers (which presumably implement some interface,) which could also take part in the transaction, so, for instance some future STOMP client could be registered and have the 'transaction' managed for them.

jonathanstowe commented 3 years ago

I'm beginning to think that a general purpose "Transaction Manager" could be developed separately, to work out what the interface should look like and how it would work for a variety of different services.

FCO commented 1 year ago

A simple solution for this problem (the .^create inside a red-do :transaction) could be .^create be aware it's inside a transaction already and don't create a transaction for itself. A dynamic variable on Red-do would easily allow that. We still need a transaction manager, but for this specific case, I think that would be a good enough solution.

FCO commented 1 year ago

Maybe generalising that a bit would also be good... maybe we could make all transaction calls set a dynamic variable and all transaction calls be ignored if that dynamic variable is set. (I don't know why, but this idea is sounding very familiar... have I already tried that?)