On Monday, December 3, 2018 at 10:53:30 PM UTC+1, Ben Hoyt wrote:
>
> Hi folks,
>
> We found some subtle bugs in our db transaction code for handling
> commits/rollbacks. Here's the pattern we were using (not real, but shows
> the issue):
>
> func DoTwoThings() error {
> tx, err := db.Begin()
> if err != nil {
> return err
> }
> // commit or rollback the transaction before we return
> defer tx.Close(&err)
>
> err := InsertFoo(tx)
> if err != nil {
> return err
> }
> if _, err := UpdateBar(tx); err != nil {
> return err
> }
> return nil
> }
>
> The problem is there's a subtle but potentially quite bad bug with this
> usage pattern -- if the InsertFoo succeeds but UpdateBar fails, the first
> "err" variable will be nil, so the deferred tx.Close() will COMMIT the
> transaction rather than ROLLBACK, and the database will be in an
> inconsistent state.
>
> The code above is a bit contrived, and you can easily fix it by moving the
> "_, err := UpdateBar()" outside of the if so it's referring to the same
> "err" variable, but it's very easy to miss and get it wrong. So we decided
> it was a bad pattern and started thinking about the best way to fix.
>
>
> [...]
> Another idea is to create a "Transact" function which takes an anonymous
> function and does all the transaction handling:
>
> func (db *DatabaseImpl) Transact(txFunc func() error) (err error) {
> tx, err := db.Begin()
> if err != nil {
> return
> }
> defer func() {
> if p := recover(); p != nil {
> tx.Rollback()
> panic(p) // re-throw panic after Rollback
> } else if err != nil {
> tx.Rollback() // err is non-nil; don't change it
> } else {
> err = tx.Commit() // err is nil; if Commit returns error
> update err
> }
> }()
> err = txFunc(tx)
> return err
> }
>
>
I'm using a similar function but with a different implementation:
https://play.golang.org/p/4t4ls8JH8Ss
Currently does not handle panics.
I'm still experimenting with the database/sql package.
One problem is what happens if the function needs to commit some code.
Should you use nested transaction? Or should you use reference counting to
only commit the last transaction?
> [...]
Manlio
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
