Grails JIRA

  • Log In Access more options
    • Online Help
    • GreenHopper Help
    • Agile Answers
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What’s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
  • Agile
Grails
  • Grails
  • GRAILS-3396 Top level task: GORM Improvements
  • GRAILS-3156

Grails pesimistic locking does not work

  • Log In
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.0.2
  • Fix Version/s: 1.1-beta1
  • Component/s: Persistence
  • Labels:
    None
  • Environment:
    OSX + Postgres
  • Testcase included:
    yes

Description

Grails Pesismistic Locking doesn't work.

  • domainObject.lock() has no effect
  • DomainObject.lock(id) doesn't take longs or Integers

I've uploaded a test project that demonstrates this. The project uses Postgres and the drives the application through HttpUnit. You'll need to create a db called lockbug to reproduce (or edit DataSource.groovy)

It appears the only way to actually lock objects in grails is to jump through some hibernate hoops or use a SQL statement (as follows).

import org.hibernate.SessionFactory
import org.hibernate.LockMode

class AccountService {

    SessionFactory sessionFactory

    int incrementWithDynamicLockMethod(String name) {
        Account account = Account.findByName(name)
        account.lock()    // Doesn't lock
        account.balance++
        account.save(flush:true)
        return account.balance
    }

    int incrementWithStaticLockMethod(String name) {
        Account account = Account.findByName(name)
        account = Account.lock(account.id) // Throws exception (doesn't take Longs or Strings)
        account.balance++
        account.save(flush:true)
        return account.balance
    }

    int incrementWithHibernateLockMethod(String name) {
        Account account = Account.findByName(name)
        sessionFactory.currentSession.lock(account, LockMode.UPGRADE) // Does nothing because account already in the session
        account.balance++
        account.save(flush:true)
        return account.balance        
    }

    int incrementWithHibernateLoadMethod(String name) {
        Account account = Account.findByName(name)
        sessionFactory.currentSession.clear() // Need to clear the session to force db load (may be able to use evict in some circumstances)
        account = sessionFactory.currentSession.load(Account.class, account.id, LockMode.UPGRADE)
        account.balance++
        account.save(flush:true)
        return account.balance
    }

    int incrementWithPostgresLockStatement(String name) {
        sessionFactory.currentSession.createSQLQuery('LOCK account IN ACCESS EXCLUSIVE MODE').executeUpdate() // Full Table Lock just for example
        Account account = Account.findByName(name)
        account.balance++
        account.save(flush:true)
        return account.balance
    }
}
  • Options
    • Sort By Name
    • Sort By Date
    • Ascending
    • Descending
    • Download All

Attachments

  1. GZip Archive
    lockbug.tar.gz
    05/Sep/08 7:08 AM
    2.73 MB
    Stephen Cresswell
  2. GZip Archive
    lockbug.tar.gz
    23/Jun/08 5:52 AM
    2.71 MB
    Stephen Cresswell
  3. GZip Archive
    postgreslogs.tar.gz
    05/Sep/08 7:28 AM
    4 kB
    Stephen Cresswell

Issue Links

depends on

Sub-task - The sub-task of the issue GRAILS-3402 Add lock:true parameter for queries and criteria

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed can be reopened.

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • Work Log
  • History
  • Activity
  • Git Commits
Hide
Permalink
Stephen Cresswell added a comment - 23/Jun/08 10:26 AM

Slight correction,

  • DomainObject.lock(id) doesn't take Longs or Strings
Show
Stephen Cresswell added a comment - 23/Jun/08 10:26 AM Slight correction, DomainObject.lock(id) doesn't take Longs or Strings
Hide
Permalink
Graeme Rocher added a comment - 23/Jun/08 11:18 AM

I'm using the static lock method in grails.org, so its strange that it doesn't work for you, nevertheless we will take a look

Show
Graeme Rocher added a comment - 23/Jun/08 11:18 AM I'm using the static lock method in grails.org, so its strange that it doesn't work for you, nevertheless we will take a look
Hide
Permalink
Stephen Cresswell added a comment - 23/Jun/08 11:34 AM

Thanks Graeme,

FYI

Caused by: groovy.lang.MissingMethodException: No signature of method: Account.lock() is applicable for argument types: (java.lang.Long) values: {1}
	at AccountService.incrementWithStaticLockMethod(AccountService.groovy:18)
	at AccountService$$FastClassByCGLIB$$655543e8.invoke(<generated>)
	at net.sf.cglib.proxy.MethodProxy.invoke(MethodProxy.java:149)
	at AccountService$$EnhancerByCGLIB$$b8cf58e.incrementWithStaticLockMethod(<generated>)
	at AccountController$_closure3.doCall(AccountController.groovy:21)
	at AccountController$_closure3.doCall(AccountController.groovy)
Error on HTTP request: 500 Internal Error [http://localhost:8080/lockbug/account/incrementWithStaticLockMethod]
Show
Stephen Cresswell added a comment - 23/Jun/08 11:34 AM Thanks Graeme, FYI Caused by: groovy.lang.MissingMethodException: No signature of method: Account.lock() is applicable for argument types: (java.lang.Long) values: {1} at AccountService.incrementWithStaticLockMethod(AccountService.groovy:18) at AccountService$$FastClassByCGLIB$$655543e8.invoke(<generated>) at net.sf.cglib.proxy.MethodProxy.invoke(MethodProxy.java:149) at AccountService$$EnhancerByCGLIB$$b8cf58e.incrementWithStaticLockMethod(<generated>) at AccountController$_closure3.doCall(AccountController.groovy:21) at AccountController$_closure3.doCall(AccountController.groovy) Error on HTTP request: 500 Internal Error [http://localhost:8080/lockbug/account/incrementWithStaticLockMethod]
Hide
Permalink
Graeme Rocher added a comment - 24/Jun/08 3:28 AM

I don't get the above error locally, additional we have a unit test that asssert this behaviour (see http://svn.grails.codehaus.org/browse/grails/trunk/grails/test/groovy/org/codehaus/groovy/grails/orm/hibernate/PessimisticLockingTests.groovy?r=6871)

When running your http unit test suite locally testAccountConcurrencyWithDynamicLockMethod() fails, but this is normal because there is a gap in between when you retrieve the object and when you execute lock() on the instance. It is only truely safe to use the static lock() method

The only other tests that fail are those that use the postgres specific syntax as I was testing against MySQL, so as it stands I cannot reproduce. I will see if Peter can reproduce against PostgresSQL

Show
Graeme Rocher added a comment - 24/Jun/08 3:28 AM I don't get the above error locally, additional we have a unit test that asssert this behaviour (see http://svn.grails.codehaus.org/browse/grails/trunk/grails/test/groovy/org/codehaus/groovy/grails/orm/hibernate/PessimisticLockingTests.groovy?r=6871 ) When running your http unit test suite locally testAccountConcurrencyWithDynamicLockMethod() fails, but this is normal because there is a gap in between when you retrieve the object and when you execute lock() on the instance. It is only truely safe to use the static lock() method The only other tests that fail are those that use the postgres specific syntax as I was testing against MySQL, so as it stands I cannot reproduce. I will see if Peter can reproduce against PostgresSQL
Hide
Permalink
Graeme Rocher added a comment - 21/Jul/08 8:56 AM

Cannot reproduce this. Sorry.

Show
Graeme Rocher added a comment - 21/Jul/08 8:56 AM Cannot reproduce this. Sorry.
Hide
Permalink
Stephen Cresswell added a comment - 05/Sep/08 7:04 AM

We've now upgraded to 1.03 and after some futher investigation I can hopefully explain the problem a little better. I'll also upload an updated test case including http unit reports and postgres logs.

The error occurs when multiple threads concurrently access the following method.

 
    int incrementWithStaticLockMethod(String name) {
        Account account = Account.findByName(name)
        account = Account.lock(account.id) // Doesn't re-read the entity so although we have a lock we could be working on stale data
        account.balance++
        account.save(flush:true)
        return account.balance
    }

The first line retrieves and account entity by name, generating the following SQL

select account0_.id as id0_0_, account0_.balance as balance0_0_, account0_.name as name0_0_ from account account0_ where account0_.id=$1

Since the account entity is already in the hibernate session calling Account.lock(account.id) doesn't re-read the object, it just locks the row with

select id from account where id =$1 for update

Since the account entity was not re-read after the lock was obtained there's a possibility of updating stale data.

The following methods demonstate potential workarounds

    int incrementWithStaticLockMethodAndEvict(String name) {
        Account account = Account.findByName(name)
        sessionFactory.currentSession.evict(account)  // Force Account.lock() to re-read the entity
        account = Account.lock(account.id)
        account.balance++
        account.save(flush:true)
        return account.balance
    }

    int incrementWithStaticLockMethodAndRefresh(String name) {
        Account account = Account.findByName(name)
        account = Account.lock(account.id)
        account.refresh() // Explicitly re-read the entity
        account.balance++
        account.save(flush:true)
        return account.balance
    }


    int incrementWithHibernateRefreshAndLock(String name) {
        Account account = Account.findByName(name)
        sessionFactory.currentSession.refresh(account, LockMode.UPGRADE)
        account.balance++
        account.save(flush:true)
        return account.balance
    }
Show
Stephen Cresswell added a comment - 05/Sep/08 7:04 AM We've now upgraded to 1.03 and after some futher investigation I can hopefully explain the problem a little better. I'll also upload an updated test case including http unit reports and postgres logs. The error occurs when multiple threads concurrently access the following method. int incrementWithStaticLockMethod(String name) { Account account = Account.findByName(name) account = Account.lock(account.id) // Doesn't re-read the entity so although we have a lock we could be working on stale data account.balance++ account.save(flush:true) return account.balance } The first line retrieves and account entity by name, generating the following SQL select account0_.id as id0_0_, account0_.balance as balance0_0_, account0_.name as name0_0_ from account account0_ where account0_.id=$1 Since the account entity is already in the hibernate session calling Account.lock(account.id) doesn't re-read the object, it just locks the row with select id from account where id =$1 for update Since the account entity was not re-read after the lock was obtained there's a possibility of updating stale data. The following methods demonstate potential workarounds int incrementWithStaticLockMethodAndEvict(String name) { Account account = Account.findByName(name) sessionFactory.currentSession.evict(account) // Force Account.lock() to re-read the entity account = Account.lock(account.id) account.balance++ account.save(flush:true) return account.balance } int incrementWithStaticLockMethodAndRefresh(String name) { Account account = Account.findByName(name) account = Account.lock(account.id) account.refresh() // Explicitly re-read the entity account.balance++ account.save(flush:true) return account.balance } int incrementWithHibernateRefreshAndLock(String name) { Account account = Account.findByName(name) sessionFactory.currentSession.refresh(account, LockMode.UPGRADE) account.balance++ account.save(flush:true) return account.balance }
Hide
Permalink
Stephen Cresswell added a comment - 05/Sep/08 7:08 AM

To reproduce

1. Unzip the tar.gz
2. Create a database called "lockbug" (we're running against postgres 8.3)
3. grails test run-app
4. grails run-httpunit

Show
Stephen Cresswell added a comment - 05/Sep/08 7:08 AM To reproduce 1. Unzip the tar.gz 2. Create a database called "lockbug" (we're running against postgres 8.3) 3. grails test run-app 4. grails run-httpunit
Hide
Permalink
Stephen Cresswell added a comment - 05/Sep/08 7:28 AM

Attached separate postgres logs for each of the above examples. incrementWithStaticLockMethod.log demonstrates the stale data / concurrency problem.

Show
Stephen Cresswell added a comment - 05/Sep/08 7:28 AM Attached separate postgres logs for each of the above examples. incrementWithStaticLockMethod.log demonstrates the stale data / concurrency problem.
Hide
Permalink
Graeme Rocher added a comment - 08/Sep/08 4:15 PM

Call account.discard() before calling Account.lock(..)

Show
Graeme Rocher added a comment - 08/Sep/08 4:15 PM Call account.discard() before calling Account.lock(..)
Hide
Permalink
Stephen Cresswell added a comment - 09/Sep/08 4:32 AM

At least update the grails docs. There's no hint of using grails.discard() prior to obtaining a lock.
Would also be nice if there were findByNameForUpdate() type methods to prevent an uncessary refresh.

Show
Stephen Cresswell added a comment - 09/Sep/08 4:32 AM At least update the grails docs. There's no hint of using grails.discard() prior to obtaining a lock. Would also be nice if there were findByNameForUpdate() type methods to prevent an uncessary refresh.
Hide
Permalink
Graeme Rocher added a comment - 09/Sep/08 6:08 PM

schedulling to 1.1 so that we can add the mentioned feature

Show
Graeme Rocher added a comment - 09/Sep/08 6:08 PM schedulling to 1.1 so that we can add the mentioned feature
Hide
Permalink
Graeme Rocher added a comment - 19/Sep/08 4:50 AM

Reduced priority of non critical issues which have current workarounds

Show
Graeme Rocher added a comment - 19/Sep/08 4:50 AM Reduced priority of non critical issues which have current workarounds
Hide
Permalink
Graeme Rocher added a comment - 28/Oct/08 7:22 AM

This issue is fixed by GRAILS-3402

You can now do:

Account account = Account.findByName(name, [lock:true] )

Show
Graeme Rocher added a comment - 28/Oct/08 7:22 AM This issue is fixed by GRAILS-3402 You can now do: Account account = Account.findByName(name, [lock: true ] )

People

  • Assignee:
    Graeme Rocher
    Reporter:
    Stephen Cresswell
Vote (2)
Watch (4)

Dates

  • Created:
    23/Jun/08 5:52 AM
    Updated:
    28/Oct/08 7:22 AM
    Resolved:
    28/Oct/08 7:22 AM

Agile

  • View on Board
  • Atlassian JIRA (v5.2.1#813-sha1:277a546)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for Grails project. Try JIRA - bug tracking software for your team.