Transactions

[benjamingr]

Hi, I just noticed (due to an issue on Bluebird) you've migrated to Bluebird promises. This is great. I've used sequelize before and enjoyed it in a node app, so props on the project.

Looking at the transaction syntax from the API reference - I see:

  // we just opened a new connection to the database which is transaction exclusive.
  // also we send some first transaction queries to the database.

  sequelize.transaction(function(t) {
  // do some async stuff ...

  // if everything is ok ... commit the transaction
  t.commit().success(function() {})

  // if something failed ... rollback the transaction
  t.rollback().success(function() {})

  // the commit / rollback will emit events which can be observed via:
  t.done(function() {
    /* we will be here once the transaction
    has been committed / reverted */
  })
})

What do you think about a more promise oriented approach? Something like the Knex discussion here: knex/knex#265 but in a way that doesn't break the current syntax: by returning to commit and rolling back by rejecting.

(Alternatively, supporting a disposer via the new Bluebird 2 .disposer and .using syntax might be helpful for the next time I use sequelize).

[mickhansen]

Hi @benjamingr. Promises are now supported so you can do something like:

return this.sequelize.transaction().then(function (t) {
  return User.create({}, {transaction: t}).then(function () {
    return DoMoreStuff():
  }).then(t.commit.bind(t), t.rollback.bind(t));
});

Automatically rolling back/committing is definitely something we can discuss.
How does disposer and using work?

[benjamingr]

Thanks for the response :)

The problem with }).then(t.commit.bind(t), t.rollback.bind(t)); pattern is that it's usually a sign of using promises in a callback API, which is expected here since you switched (I had a bunch of those myself at a point).

Automatically rolling back/committing would work something like:

return this.sequelize.transaction(function (t) {
  return User.create({}, {transaction: t}).then(DoMoreStuff);
});

This is basically the same as the above code, except without the need for an explicit .then(success,fail), doing something like:

function transaction(fn){
var t = makeTransactionObject();
var p = fn(t);
p.then(function(){ // we got to the then if the returned promise fulfilled
return t.commit();
}).catch(function(){ // we got here if the returned promise rejected
return t.rollback();
});
}

Since promises recursively assimilate - if doMoreStuff itself throws the transaction will rollback and if it all goes smoothly - the transaction will fulfill.

Let's look at .disposer for a raw MySQL connection with a connection pool, and in addition let's use a current sequelize database connection with a database, only let's use them together:

First, the raw MySQL connection:

var mysql = require("mysql");
Promise.promisifyAll(mysql);
var pool  = mysql.createPool({
    connectionLimit: 10,
    host: 'example.org',
    user: 'bob',
    password: 'secret'
});

function getRawSqlConnection() {
    return pool.getConnectionAsync().disposer(function(connection) {
        try {
            connection.release();
        } catch(e) {};
    });
}

Now, for sequelize:

// all you have to do to make this work on your end is call .disposer before .transaction()
// returns, so Promise.using can use it, similarly to: 
function getTransaction(){ 
    return sequelize.transaction().disposer(function(t, promise){
        return promise.isFulfilled() ? t.commit() : tx.rollback();
    });
}

Now for some usage:

Promise.using(getRawSqlConnection(), getTransaction(), function(conn, tx){
        // tx is a sequelize transaction here, conn is a raw mysql connection from the pool
        // I can use more resources like file handles etc.
        return queryAsync("SELECT * FROM...").then(function(){
                // the mySQL connection made a query now let's use the transaction
                return User.create({}, {transaction: tx}); // this is on the transaction
        }).then(function(){
             if(somethingBad) throw new Error(); // all resources will release cleanly
                                                                       // and the transaction will rollback on a rejection
             return true; // if didn't throw, this will fulfill, the resources will again release cleanly
                               // and the transaction will fulfill
        });
});// we are guaranteed that resources will always be cleaned up, and can interop
   // with any other resource that uses disposers here

[mickhansen]

That would require transaction to not have the same resolve API as everything else though, not sure that's great design.

Looking at disposer/using it doesn't really fit how connections are handled right now, they need to be used by queries run by the overall orm functions so should not be exposed to the user. And using finally as we do now seems to work perfectly. (But you might just have been making an example).

return sequelize.transaction().disposer(function(t, promise){ this pattern is interesting though, i suppose we could use that internally in the transaction method before we return the promise?

[benjamingr]

That would require transaction to not have the same resolve API as everything else though, not sure that's great design.

That of course is a design choice. I totally get wanting to keep a consistent API here.

And using finally as we do now seems to work perfectly

The problems are (just like in Java/C#/Python or every other language that implements try(resource)/using/with etc):

• You can forget the .finally and never know that you have forgotten it. With .using the scoping takes care of resource management automatically. You also have to be aware of whether the resource opened correctly.
• It is pretty hard to dispose multiple resources - for example - handling two transactions to different databases and writing to a file at once (for example for copying data over from a staging database to a production database and logging about it). Handling what to close when, in what order and so on when one resource fails to open or fails during this process can be very challenging and not trivial at all.

i suppose we could use that internally in the transaction method before we return the promise?

Yes, you can definitely do that. It sounds like a good idea since it both keeps the current API and enables people to utilize .using which would work for me.

[mickhansen]

It's true that you can forget .finally, but since this is for connection resource cleanup, which the user never sees it's not a huge issue. (However that's internals, not external transactions).

Implementing .disposer inside the transaction method when returning the promise sounds like a good solution.

[mickhansen]

@benjamingr we'll need an option to turn off (or on, depending on what we select as default) the auto resolving, any ideas for the naming? We should also figure out what should happen in case we select on as default and people have existing code which calls commit.

[amaurybrisou]

Hello, I'd like to react to the first mickhansen answer regarding promises, your code block given as example seems wrong. this snippet may work as you would have wanted :

return this.sequelize.transaction({ options }, function (t) {
    return User.create({}, {transaction: t}).then(function () {
      return DoMoreStuff():
    }).then(t.commit.bind(t), t.rollback.bind(t));
});

sequelize.transaction still need at least a callback.

Other point , I put a Sequelize transaction example using promises here http://pastebin.com/DVMAMm56

hope it's good way to use it.

I'll follow with nested transactions.

[mickhansen]

@amaurybrisou transaction() does return a promise on 2.0.0-dev12, not on 1.7.x though.

[mickhansen]

sequelize.transaction() returns a promise on 2.0.0-dev12 as i just stated.
What do you mean by nested transactions? Save points?

[benjamingr]

Sorry, I was afk. I suggest replacing the existing syntax somehow and not having it "double up" and auto resolve. This is since JavaScript functions as you know return undefined by default which is an immediately resolved promise when assimilated and we don't want that.

[mickhansen]

@benjamingr I think we will provide both styles of syntax, if using callback it will automatically clean it up - If using promies it's on the user.