Is it possible to support connection pooling transaction based on current "node-orm2" architecture?

[taoyuan]

node-orm-transaction has an issue Transactions do not work together with connection pooling.

It looks that it is a big problem and difficult to solve.

• Sequelize dose not support in current.
• JugglingDB is discussing and proposed an API .
• node-persist provides comprehensive support for the transaction.

There are too different between node-orm2 and node-persist, and the solution of JugglingDB has taken maybe has performance issues.

Maybe expose connect and add a connection parameter to the functions involved sql execution is a complete and optional solution. But the problem is too much code should be changed.

[taoyuan]

Sequelize transaction issue has been raised two years. Until now not yet been solved.

[dxg]

I'm intrigued by this issue. I'm going to look into it from the postgres side of things.
If it works for pg, shouldn't be hard to port to mysql aswell.

cc dresende/node-orm-transaction#2

[taoyuan]

OK, Thanks. I'll keep paying attention.

[dresende]

The problem is that when having a transaction, pooling should not be used, or... it should be used to get a connection to start transaction and future queries should hold that particular connection. Something has to be done in the driver(s) to maybe hold a possible connection (if asked) and dispose (when asked). This way the plugin could call the start transaction and ask the driver to hold that, and when committing (or rolling back) the plugin could then tell the driver to dispose.

Either we do this or forget the plugin and pass the transaction code to inside the driver.

[dxg]

The problem with that solution is it will affect queries running outside of the transaction at the same time (ie, webserver serving other requests) right until the transaction is complete. If in the middle of the transaction there is a call to a slow third party API, this could be quite noticible.

Things get even worse when whilst one transaction is outstanding [waiting for the API], another request triggers a second transaction block. What do we do then?

Ideally the connection holding behaviour would somehow be scoped to the transaction block only.

@taoyuan 's suggestion of explicitly passing the connection to all queries within the transaction seems like the logical choice at first, however things will break unexpectedly when there are before* hooks which run other queries, or when saving a model results in associations also being saved. We'd have to make sure to pass the connection to all sub queries.

When we call orm.connect a connection is created and passed into an ORM instance which holds a list of models.
Right now I'm wondering if this can somehow be turned on it's head..

[notheotherben]

Another alternative would be to have connection identifiers within ORM, so creating a transaction would return a chainable object which holds some kind of transaction ID. This could then be used by ORM to identify the connection on which to execute the transactions.

I'm thinking something along the lines of

var t = db.transaction();
Model.transaction(t).get(id, function(err, model) { ... });
OtherModel.transaction(t).create({ ... }, function(err, model) { ... });
t.commit();

In this form I think it would be possible to implement it as a simple extension to node-sql-query, which would obviously simplify our lives a lot, since we would only have to link a db.transaction() object to a specific connection instance on which all Model.transaction(t) and Instance.transaction(t) would act. It would also allow (if we use a node-sql-query extension) it to support all future chaining function updates.

[dxg]

Are there any benefit to passing the transaction around over passing the connection?

I'm curious about how we could make it work for hooks.

beforeSave: function (t) {
  otherModel.transaction(t).save( ... );
}

This syntax would be opt in if you want transaction support. This would be confusing though.

Internally, we'd have to pass t around to hooks and things like saveAssociations, which sort of points to including transactions in ORM itself.

[notheotherben]

I guess if we're willing to add a commit() method or something similar to the connection object then it would be possible to do that as well. As far as I could reason, all that a transaction object's public API would need is the following:

{
    commit: function() { },
    rollback: function() { }
}

and then obviously we would need a reference to the connection it would operate on.

As for hooks, I'd assume that it would make more sense to transparently handle it behind the scenes, so any operations within a transaction remain within that transaction. (Not sure how possible it would be to do this, maybe we just set this to model.transaction(t) behind the scenes if the operation is run through a transaction).

[dxg]

Yeah transactions make more sense from that perspective. With connections, the commit would need to be attached to something like ORM itself: ORM.commit(connection). t.commit() is easier since t is already there.

As for handling hooks transparently, I don't think it's possible. Consider this example:

orm.connect("...", function (err, orm) {
  var History = db.define("history", {
    event  : String,
    info   : String,
    userId : Number
  });

  var Person = db.define("person", {
      name    : String,
      surname : String,
      age     : Number
    }, {
      hooks: {
        beforeSave: function (t, next) {
          History.create({
            event: "save",
            info:  "...",
            userId: this.id
          }, function (err, item) {
            next(err);
          });
        }
      }
    }
  );
});

If there is a way to do it that's better than History.transaction(t).create({ then I'd love to hear about it!
Slightly worried that since closures would catch us out here, they might also catch us out somewhere else too..

[notheotherben]

Okay, let's do this then. When you call .transaction(t) on a Model/Instance it wraps it in a transaction compatible version, it should inherit all the properties, methods etc. from the standard one but overload our hook caller to ensure that all hooks operate on the transaction wrapper instead of the original one. We can also have a .transaction property on that wrapper which represents the transaction object.

That would allow us to do the following:

beforeSave: function(next) {
    //Would automatically be part of the current transaction, since it's done within ORM
    this.age++;

    //Would automatically be part of the transaction since 'this' is a transaction aware wrapper
    this.addHistory({
        event: 'save',
        info: '...'
    }, function(err, item) {
        next(item);
    });

    //Would need to be explicitly added to the current transaction
    History.transaction(this.transaction).create({
        event: "save",
        info: "...",
        userId: this.id
    }, function(err, item) {
        next(item);
    });
}

(Excuse the parallel calls, just proving a point - it wouldn't actually work correctly)

It would then be up to the user to 'opt-in' to using transactions in hooks for anything outside of modifications to the current model, which probably isn't a bad idea since it favours explicit requests from the user over transparent behaviour which may be unexpected.

[notheotherben]

Something which we're missing as well is that once a connection has been pooled for a transaction, it should not be available to ORM for general requests until the transaction has been completed. I'm sure that shouldn't be a massive issue, but something to keep in mind.

[dxg]

Yeah, that shouldn't be a huge issue; Once you check a connection out of the pool you have it exclusively.

Even with all the above, we still have an issue with closures; Methods might have calls to History aswell. Other models might also.
This basically means that if you'd like to use transactions, you have to always use:
History.transaction(transaction).create in hooks and methods for all models.

It's becoming clear why sequelize has had the pooled transaction issue open for 2 years.

I've been thinking about a different solution, but have to run some benchmarks to see if it's plausible. It involves reconstructing an entire copy of ORM but with a single connection checked out from the main ORM pool. It would mean other than the cost of reconstruction, we wouldn't really have to change much code, nor pass transactions around. It would effectively "just work". If it takes more than a millisecond or two of CPU time though, it may be off the table.

EDIT
I did some benchmarks, and in my linux VM it takes between 0.9 and 1.9ms.
This is with 6 models and a bunch of associations. It means an absolute maximum of 500 req/s but I don't think anyone would get anywhere near that anyway with a single process talking to a DB server.

[notheotherben]

Only problem I can think of with that approach is that we will no longer have a unified cache, so objects created/retrieved/modified in one transaction will not be updated on the original ORM instance. Other than that, a rather elegant solution (even if it bugs the perfectionist in me).

[dresende]

I'm sorry for my delayed comments but I'm on vacation time and trying to step away from code for a while. Feel free to make decisions about the module.

About the transactions I didn't thought about concurrency, it's another big problem but maybe it can be reduced if we think transactions only make sense for queries that change the database, namely insert, update and remove. About things being done in hooks, they should probably be inside the transaction automatically (if changing database).

So, we should make it as transparently as possible to the user. I'm thinking about a way of having a transaction support in the drivers but being controlled by the plugin. I'm going to make some tests and see if it's possible (making sure multiple/nested transactions should be possible).

[dxg]

It bugs the perfectionist in me too..

Nested transactions you say :-\