Investigate alternative transaction API

[stuartwdouglas]

The current transaction API has a couple of limitations / issues:

• Transactions can't be shared across DALs, which may become an issue if we want to atomically handle deployment changes. Without using transactions we may end up with some parts of the deployment starting and others failing.
• It is easy to accidentally use the wrong DB handle and invoke operations outside the TX. This is easy to do, hard to spot in review, and can cause problems that are not obvious until they have hit productions. We have already had one production deadlock as a result of this.

I think we should investigate adding the transaction to the Context, and then all subsequent DB accesses using this Context would use the same transaction. This would allow the transaction to be propagated between DALs, and mean't that you can't accidentally do work outside the transaction.

[alecthomas]

The DAL's aren't designed to be shared at all, the current situation is temporary while all the service refactoring is occurring. The correct way to share is to share the underlying queries.

I do agree it's easy to use a DAL method outside a transaction though, but that can be fixed with a relatively straightforward change to the API - make the DAL something like:

type DAL struct {}

func (d *DAL) Begin(ctx context.context) *Tx { ... }

With all of the current DAL methods moved to Tx.

[stuartwdouglas]

So you don't think we will have a situation where we want a TX that encompasses operations for different parts of the code base? I am thinking about things like starting pub/sub or adding cron entries. If we can't do these things transactionally we could end up in a position where a deployment is 'half deployed', with some stuff commited to the database even though other parts have failed.

[alecthomas]

We definitely do, but that's what I was referring to when I was talking about sharing queries, eg. for the cron service we reuse the async SQL queries.

The DAL's shouldn't be treated as part of the public interface of these subsystems, as they're internal implementation details. Also at some point they likely won't be part of the same database (eg. if cron becomes a "system" module), so sharing transactions won't be possible. Keeping them isolated will help with that refactoring.

[stuartwdouglas]

In that case I will close this.