Ayende @ Rahien

Refunds available at head office

Refactoring toward frictionless & odorless code: What about transactions?

Originally posted at 3/30/2011

One thing that we haven’t done so far is manage transactions. I am strongly against having automatic transactions that wrap the entire request. However, I do like automatic transactions that wrap a single action. We can implement this as:

 

public class NHibernateActionFilter : ActionFilterAttribute
{
    private static readonly ISessionFactory sessionFactory = BuildSessionFactory();

    private static ISessionFactory BuildSessionFactory()
    {
        return new Configuration()
            .Configure()
            .BuildSessionFactory();
    }

    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        var sessionController = filterContext.Controller as SessionController;

        if (sessionController == null)
            return;

        sessionController.Session = sessionFactory.OpenSession();
        sessionController.Session.BeginTransaction();
    }

    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        var sessionController = filterContext.Controller as SessionController;

        if (sessionController == null)
            return;

        using (var session = sessionController.Session)
        {
            if (session == null)
                return;

            if (!session.Transaction.IsActive) 
                return;

            if (filterContext.Exception != null)
                session.Transaction.Rollback();
            else
                session.Transaction.Commit();
        }
    }
}

This one is pretty simple, except that you should note that we are checking if the transaction is active before trying something. That is important because we might have got here because of an error in the opening the transaction, we would get a second error which would mask the first.

Comments

hazzik
04/12/2011 10:45 AM by
hazzik

And what about child actions?

mag20
04/12/2011 11:40 AM by
mag20

A couple of questions:

  1. Is there a performance penalty for using transaction for all actions including the ones that are read-only.

  2. If I use a BLL layer, how can I share the session used in UI and BLL?

Ayende Rahien
04/12/2011 11:46 AM by
Ayende Rahien

Mag2,

1) you always need to use transactions with nhibernate

2) ambient session

Bogdan Marian
04/12/2011 12:00 PM by
Bogdan Marian

@mag20 Since we are talking about a NHibernateActionFilter, this means the developer knows that the action manipulates a RDBMS, so even if it will run only one query, a transaction is needed (isolation from ACID is enforced by transactions).

@ayende How about using a TransactionScope ( msdn.microsoft.com/.../...ns.transactionscope.aspx)? If we have an action which spawns several child actions, each of these will just call the TransactionScope Complete in case everything is OK. Of course, this TransactionScope would have to be stored somewhere in order to make it available to all involved actions - maybe inside the HttpContext.Current ...

Be using a TransactionScope, we can enlist other types of resourcs, eg MSQM, etc.

Scooletz
04/12/2011 12:01 PM by
Scooletz

@hazzik, child actions will run in their own transactions. Filters are applied on per action basis and child actions are run during result execution. If you got all the needed data in your session context, and you don't alter db in a child action, then everything is ok. Otherwise you have two (or more), separate transactions per request.

Ayende Rahien
04/12/2011 12:02 PM by
Ayende Rahien

Bodgan,

If possible, avoid TransactionScope, it can lead to very bad perf.

Scooletz
04/12/2011 12:03 PM by
Scooletz

@Bogdan Marian: look at what @hazzik pointed out. Where would you commit your TransactionScope?

Bogdan Marian
04/12/2011 12:08 PM by
Bogdan Marian

@Scooletz The TransactionScope has a a method, Complete, which is called if the underlying tx should be commited. If a rollback is needed, no method is called, thus will make the underlying tx to be rolled back - more info can be found on MSDN (see the link of my first comment). The Complete method will be called inside the OnActionExecuted method.

Leonardo
04/12/2011 12:55 PM by
Leonardo

What about actions that do not require db access at all? You are opening a session and a transaction ALWAYS. Perhaps it's better to make the Session property "lazy" and when first issued open the session and the transaction.

Then corresponding checks at ActionExecuted should be taken when that happens.

It still feels a bit awkward to have different sessions and transactions for EACH child action. I think it's better to have a wider session (i.e. close it at ResultExecuted) in the parent action and use the same session and transaction for the child actions (since generally those actions will be read only), just to present data in the view.

Scooletz
04/12/2011 12:55 PM by
Scooletz

@Bogdan Marian, if so, the "child action problem" occur in the very same way, cause child action are run after OnActionExecuted method. They are run during ActionResult.ExecuteResult. Using TransactionScope adds nothing the Ayende's solution, but perf problems mentioned by Ayende.

NC
04/12/2011 01:13 PM by
NC

@Leonardo - Creating a session isn't expensive, it's cheap, it's not going to hurt performance by creating a session thats never used for an action that does not do any database access.

However any action that does not require database access is probably a controller that does not require database access, so you wouldn't inherit the NH controller to begin with.

NC
04/12/2011 01:19 PM by
NC

@Leonardo - Also it may feel awkward, but it's more logical, your session is a UoW, not a Collection of Work. If 1 action is saving a product to your shopping cart, and another action is querying the shopping cart. They are two completely separate things, and should be run under two separate units of work and two separate transactions.

Leonardo
04/12/2011 01:20 PM by
Leonardo

@NC

I agree that session openings aren't expensive, but what about transaction opening?

I don't think that if I have an action that does not access the db then the whole controller isn't going to access the db.

Think of the case of a simple crud: The New action does not access the db (given the form is simple enough), but the Create action does.

In this scenario, you would be opening a transaction in the New action and that is not needed.

Ayende Rahien
04/12/2011 01:26 PM by
Ayende Rahien

Leonardo,

No one solution is applicable for all scenarios.

I would point out that usually for New action, you need some lookup data, but that is mostly beside the point

Roger
04/12/2011 01:28 PM by
Roger

IMHO, checking if transaction is active isn't really reliable in case of zombied transactions. The client may think the transaction is active when it's actually not -> you'll loose the first exception.

Leonardo
04/12/2011 01:41 PM by
Leonardo

@Ayende

But if you make the Session property lazy, so that it opens when first issued (and opens a transaction also), you have a solution applicable to all cases.

@NC

I agree with your point, you're right that those are 2 separate things. It's just from the performance perspective where you could perhaps gain something. I'm thinking of a view that has several sections and each could be rendered by a separate action, retrieving different data.

For example this blog page. Main view for the post, then a placeholder for the comments, which would be a child action, then in the master of this view would also be different actions for each section in the sidebar from the right.

Do I really need to open that many transactions each time this page is requested?

Alex Simkin
04/12/2011 01:43 PM by
Alex Simkin

@Leonardo, @Ayende

You can create Transaction Opt Out Attribute and inspect it through ActionExecutingContext in the filter above.

Bogdan Marian
04/12/2011 01:53 PM by
Bogdan Marian

@Scooletz If the TransactionScope is sotred inside the HttpContext.Current, the action and all its children will share it. Each of them will call the Complete method to notify the transaction manager that their work is OK to be comitted. There is only one TransactionScope, so only one real transaction shared by all actions.

Leonardo
04/12/2011 02:07 PM by
Leonardo

@Alex

That would bring the problem of explicitly having to say that you don't want a transaction, and you have to remember to put that on every action that does not access to the db.

Also, if you already have an action with that attribute and then you find out that it indeed needs db access (and you forget to remove the attribute), you will be accessing the session outside a transaction, which is not the correct usage.

joe
04/12/2011 02:13 PM by
joe

If that commit() fails, do you still need to rollback()? Or, is that handled implicitly?

Alex Simkin
04/12/2011 02:16 PM by
Alex Simkin

@Leonardo

  1. If you have most Actions accessing db, then you make opt-in attribute global and apply opt-out as optimization technique (that can be postponed until you start optimization phase)

  2. If you have most Actions not accessing db, then you only apply transaction opt-in attribute to them.

And, as usual, you cannot have it both ways

Alex Simkin
04/12/2011 02:17 PM by
Alex Simkin

I meant "then you only apply transaction opt-in attribute to those accessing db"

Leonardo
04/12/2011 02:31 PM by
Leonardo

@Alex

But why would you need to specify to not use transaction if you don't access the db? That's like saying things twice. If the session is not used anytime during the action, then that means you don't access the db and therefore you don't need a transaction.

And still I think that approach can lead to use the session outside a transaction if the attribute is not updated when the action is.

Leonardo
04/12/2011 03:07 PM by
Leonardo

@Alex

Yes, that's what I was talking about. Nice implementation using ICurrentSessionContext.

Eber Irigoyen
04/12/2011 03:55 PM by
Eber Irigoyen

What are your reasons to be "strongly against having automatic transactions that wrap the entire request", I'm interested in this, since I've seen some NHibernate experts suggest you do wrap them

Marco
04/12/2011 04:19 PM by
Marco

This is exactly my transaction boundary starts at the app service layer. My read layer does have this complexity and my command handler begins/commits my transactions.

I'm sure we all can think of some edge cases, but so far, no issues when building app layer from use cases.

Alex Simkin
04/12/2011 04:23 PM by
Alex Simkin

@Eber,

I am with you on using session per request. This way child actions can sometimes benefit from using cached data.

José F. Romaniello
04/13/2011 11:51 AM by
José F. Romaniello

As some guys mentioned here (my article). Instead of using an attribute to explcitly say where the transaction begin and end, i prefer delay the two things (OpenSession/BeginTransaction) to the first usage GetCurrentSession... and in the End of the request, if there is a transaction commit and/or close the session.

NHibernate need transaction for everything, so maintaining an attribute in the request that i will need to use the persistence is something that doesn't worth. I don't want to think how you test all this.

Thanigainathan
04/14/2011 03:56 PM by
Thanigainathan

Hi,

How will this work among the Web Farms ?

Will one transaction be available in another farm ?

Thanks

Bunter
05/05/2011 06:31 PM by
Bunter

As we have only single inheritance in C# and composition can be rather tricky with different base controllers, alternative approach to the "SessionController" would be to go with storing ISession in controllers (well, global) HttpContext and have an controller extension method to get the session from there.

Comments have been closed on this topic.