views:

1674

answers:

3

Hi,

today I have coded a test case for my application, to see how transactions behave. And I have found that nothing works the way I thought it does.

I have a Spring-based application using Hibernate as JPA provider, backed by MySQL. I have DAO objects, extending Spring's JpaDaoSupport. These are covered by Spring's transaction management.

The test case I've created works like this: 1) An entity is created, with some counter set to 0. 2) Then two threads are created, which both call a DAO method incrementCounter() in a loop.

I thought that when DAO methods are covered with transaction, then only one thread will be inside it (i.e., Spring will take care of synchronization). But this has proven to be false assumption.

After (temporary) solving this by adding synchronized to the DAO method, I found out that Hibernate does not store the changes made by the DAO method, and the other thread, when find()ing the entity, has old data. Only explicit call of this.getJpaTemplate().flush(); helped.

I also thought that entity manager would give me the same entity instance from the cache of the persistence context, but this is also false. I have checked hashCode() and equals(), and they are fine - based on entity's bussines key.

Any comments welcome, as it seems I miss some basic concepts of how JPA / Spring works with transactions.

  1. Should DAO methods be synchronized?
  2. Should I call flush() at the end of every DAO method?
  3. Is spring responsible for making the calls to DAO methods behave transactionally? (i.e. not letting two threads work with the same entity at the same time)
  4. If not, how do I achieve this?

Note that in my test case, I use a single DAO object, but that should be OK as Spring's beans are singletons - right?

Thanks for any help.

public class EntityDaoImpl extends JpaDaoSupport implements EntityDao {

public synchronized void incrementCounter( String znacka ) 
{

  String threadName = Thread.currentThread().getName();
  log.info(threadName + " entering do incrementCounter().");

  Entity ent = this.getJpaTemplate().find( Entity.class, znacka );
  log.info("Found an entity "+ent.getZnacka()+"/"+ent.hashCode()+" - " + ObjectUtils.identityToString( ent ) );
  log.info(threadName + ": Actual count: "+ent.getCount() );

  ent.setCount( ent.getCount() + 5 );

  int sleepTime = threadName.endsWith("A") ? 700 : 50;
  try { Thread.sleep( sleepTime ); }
  catch( InterruptedException ex ) {  }

  ent.setCount( ent.getCount() + 5 );
  this.getJpaTemplate().flush();

  log.info(threadName + " leaving incrementCounter().");
}
}


Without synchronized and flush(), this was giving me output like

Thread A: Actual count: 220
...
Thread B: Actual count: 220
...
Thread A: Actual count: 240
...
Thread B: Actual count: 250
...
Thread A: Actual count: 250

...etc, meaning that one thread has overwriten the changes from the other.

+1  A: 
  1. Should DAO methods be synchronized? Mine usually are not, because they don't have any state. No need.
  2. Should I call flush() at the end of every DAO method? No, I don't do that.
  3. Is Spring responsible for making the calls to DAO methods behave transactionally? (i.e. not letting two threads work with the same entity at the same time). I think you're confusing transactions, isolation, and synchronization.
  4. If not, how do I achieve this? I think you have to worry about synchronization and isolation.

Your example is not what I would think of as a DAO. I think your test really isn't the proper idiom. If I had an object Foo, I'd have a FooDao interface that would declare the CRUD methods for that object:

public interface FooDao
{
    List<Foo> find();
    Foo find(Serializable id);
    void saveOrUpdate(Foo foo);
    void delete(Foo foo);
}

It's possible to write a generic DAO, as you'd guess from the example.

Spring usually has a service layer that uses domain and persistence objects to implement use cases. That's where transactions need to be declared, because the unit of work is associated with a use case and not a DAO. A DAO has no way of knowing whether or not it's part of a larger transaction. That's why transactions are usually declared on the service.

duffymo
Thanks for the answer.I started to put the use cases to DAO, because with JpaDaoSupport, DAO objects were only a thin shell serving to cast the references to proper types. If I understand it well, with another layer for use cases, I would end up with other Spring-managed beans, implementing the use cases, AOP'ed with spring transactions instead of DAOs; so it would technically be the same, wouldn't it?What approach would you recommend to take care of synchronization?Would you recommend some Spring 2.5 + JPA tutorial app? Thanks
Ondra Žižka
I've quoted the Spring idiom to you - I'd recommend following it, especially if this is your first time with Spring. Usually I run Spring on a Java EE app server like Tomcat or JBOSS or WebLogic, so each request runs in its own thread. The app server queues up the requests for processing. I'd recommend the Spring reference docs.
duffymo
Well, what I do is a backend for a web app running on Tomcat, too.Spring reference is good and vast, but still does not contain the tips on synchronization - at least I haven't found.I guess EntityManager#lock(Object entity, LockModeType lockMode) is what I should take look at, right?
Ondra Žižka
No, have a look at isolation in the transaction manager. If you want to understand threading, read Brian Goetz's book. I believe you're off base on synchronization. Your example isn't representative of the way a Spring app should work.
duffymo
I quite understand threading, transactions and isolation levels - from other (non-Java) technologies. What I do not know is how to combine/configure all this together, using Spring.Thanks for pointing me, I found few pages to read. If you had some link handy, please share it.
Ondra Žižka
+1  A: 

As an aside in most apps I have worked on, transactions are usually declared at a service layer above DAOs. This means if you're doing a few inserts across different entities it can have transactional semantics.

The answer depends on your application:

  • If you're expecting a lot of contention over the same entity, you can use pessimistic locking which will obtain a database lock so that another thread can't update over the top. Similar to your synchronized DAO this will restrict the overall performance of the application and you'll need to take care to avoid deadlocks
  • If you're expecting this to occur infrequently use optimistic locking which will maintain a version column and if an update is lost an exception is thrown to be handled in application code.
Stephen
+1  A: 

I thought that when DAO methods are covered with transaction, then only one thread will be inside it

Based on your description, each call to incrementCounter should be running in its own transaction, but the threads are running concurrently so you would have two transactions running concurrently.

(i.e., Spring will take care of synchronization).

It sounds like you're confusing transactions with synchronization. Just because two threads are running their own transaction does not mean that the transactions will be serialized with respect to each other, unless your transaction isolation level is set to SERIALIZABLE.

I also thought that entity manager would give me the same entity instance from the cache of the persistence context

Yes, within the scope of the persistence context. If your persistence context has transaction scope, then you're only guaranteed to get the same entity instance within the same transaction, i.e. within one invocation of incrementCounter.

Should DAO methods be synchronized?

As mentioned in other posts, typically DAO methods are CRUD methods, and you don't usually synchronize since the DAO is simply selecting/updating/etc. and doesn't know the larger context of what you're doing.

Is spring responsible for making the calls to DAO methods behave transactionally? (i.e. not letting two threads work with the same entity at the same time)

Again, transaction != synchronization.

If not, how do I achieve this?

As mentioned in other posts, you can do synchronization at the Java level or set your transaction isolation level to SERIALIZABLE. Another option would be to use SELECT FOR UPDATE syntax (e.g. LockMode.UPGRADE in Hibernate) to inform the database that you intend to make a change and the database should lock the rows.

If you're not tied to pessimistic locking, you can implement optimistic locking, e.g. using Hibernate versioning. You would get a lot of optimistic lock failures with your specific incrementCounter example, but one would assume a real application would not behave like that.

Allan