Reconsider use of savepoints in transactions

[matt2e]

While fiddling with partner project's transaction code to allow nested "transactions" we noticed that FTL is using savepoints, and it wasn't clear if we actually need to. Do we ever need to rollback a bit but then continue the transaction?

@moe:

it might be worth considering a similar pattern if FTL isn't using SAVEPOINTS for actual nested txns. there's quite a bit of hesitancy around their use wrt performance hits:
https://about.gitlab.com/blog/2021/09/29/why-we-spent-the-last-month-eliminating-postgresql-subtransactions/
https://www.cybertec-postgresql.com/en/subtransactions-and-performance-in-postgresql/
https://postgres.ai/blog/20210831-postgresql-subtransactions-considered-harmful

Used in FTL here: https://github.com/TBD54566975/ftl/blob/edbbf12d57ac2642cf5abf343f2f5a60a4785174/backend/controller/sql/conn.go#L51-L79

[deniseli]

It looks like we only support savepoints currently but don't actually rely on them. The only place where we create a new savepoint is in tx.Begin in the file that you linked. That said, I don't know if there's any significance to that single savepoint's behavior.

[deniseli]

After reading a bit more on savepoints, I can't find a reason we wouldn't be able to get rid of our savepoints usage and just rely on BEGIN;...COMMIT; to wrap the whole transaction. Unless I'm missing something higher up in our SQL setup...

[gak]

pgx internally does a savepoint internally for Begin too!

// Begin starts a pseudo nested transaction implemented with a savepoint.
func (tx *dbTx) Begin(ctx context.Context) (Tx, error) {
	if tx.closed {
		return nil, ErrTxClosed
	}

	tx.savepointNum++
	_, err := tx.conn.Exec(ctx, "savepoint sp_"+strconv.FormatInt(tx.savepointNum, 10))
	if err != nil {
		return nil, err
	}

	return &dbSimulatedNestedTx{tx: tx, savepointNum: tx.savepointNum}, nil
}