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

Concurrency *within* transactions. #507

Open
jonathanstowe opened this issue Aug 28, 2021 · 3 comments
Open

Concurrency *within* transactions. #507

jonathanstowe opened this issue Aug 28, 2021 · 3 comments

Comments

@jonathanstowe
Copy link
Contributor

I think that the following may just be a very strong warning in the documentation as I can't actually think of a sane way of fixing it, though your mileage may vary.

Consider some code that does something like:

red-do {

# do something to DB

start {
   # do something to do DB
};

# do something to DB
}, :transaction;

With the Pg driver, the #504 essentially isolates a transaction from other concurrent access outside the transaction by creating a new instance of DB::Pg with its own connection pool such that as long as all access in the transaction is performed sequentially then all will get the same underlying connection from the pool.

However if some code is started asynchronously as above, the async code will get the existing DB::Pg::Database instance (assumed to be the one that the transaction was started on,) at which point it essentially becomes a race between the async code, any DB code after it is started and crucially the COMMIT (or ROLLBACK ) which may get a different connection from the pool such that the code after may not be in the transaction, or the commit is called on a connection without a transaction (leading to an error.) Obviously it becomes less predictable if more threads are spawned.

The only way to prevent this is by waiting on the async code before any further DB access is done in the main block and especially before the end of the red-do block.

A mitigation could be provided if the async code doesn't need to take part in the main transaction by allowing a red-do to over-ride the behaviour described in #506 and force the creation of a further new driver instance with a new connection. But :-(

@FCO
Copy link
Owner

FCO commented Aug 29, 2021

That's a hard problem... I think we'll need to think about that...

@FCO
Copy link
Owner

FCO commented Aug 29, 2021

If it was returning the Promise we could use that to decide to commit or rollback and when. And doc it saying that if you any to use multiple threads inside a transactional red-do block, you should return a "control promise" or maybe a "control awaitable"... or simply await it inside the block.

It would also be good to invalidate the driver after a commit/rollback to give a nice error message if it's used after committed/rolledback. And maybe suggesting returning the promise.

But those were only my first thoughts...

@jonathanstowe
Copy link
Contributor Author

While thinking about some additional complexities in the #506 the idea of a "transaction manager" began to suggest itself. The transaction manager can use any driver specific features that are available.

In this specific case for Pg, a "two phase commit" with prepared transactions might contain a solution, as the commit or rollback can be done on any connection. So each (possibly asynchronous block,) prepares a transaction, the transaction name is added to the transaction manager as well a Promise on the result of the block, at the outer scope the transaction manager will await on all the Promises and if all are kept then issues the commit for the prepared transactions, if any are broken then roll all of them back.

This works for the #506 case nicely too (in my mind.)

FCO added a commit that referenced this issue Aug 29, 2021
If `red-do :transaction`'s block returns a Awaitable, it creates
a new Promise that will handle the commit/rollback and does not
let the sync code to call commit/rollback.

Probable next steps are:
- extract that logic to a transaction manager
- on transaction manager add a transaction stack and methods
	- queue-begin
	- unqueue-commit
	- break-queue-rollback
(or something like that)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants