How to create a thread safe hit counter in Grails?

Question

I have a concurrency issue in my hit counter implementation.

Here is how I have implemented the hit counter.

The hit counter domain is as follows:

class PageHitCounter {

    String pageIdentifier
    int hits

    static constraints = {
    }
}

In the action I have the code as follows:

def verifyRegistration(Long id){

    if(springSecurityService.isLoggedIn()){

        redirect(controller: "user", action: "index")
        return
    }


    def hitcounter = "${controllerName}/${actionName}/${id}"

    def hc = PageHitCounter.findByPageIdentifier(hitcounter)

    PageHitCounter.withTransaction {

        if(hc){
            hc.hits = hc.hits + 1
            hc.save()
        }
        else{
            hc = new PageHitCounter(pageIdentifier: hitcounter, hits:1)
            hc.save()
        }

    }


    [id: id, hits:hc.hits]
}

The hits variable is then displayed in the view page as follows:

<div class="hitCounter">

    ${hits}

</div>

The error I am getting the logs is:

org.springframework.orm.hibernate5.HibernateOptimisticLockingFailureException: Object of class [com.analytics.PageHitCounter] with identifier [18]: optimistic locking failed; nested exception is org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [com.analytics.PageHitCounter#18]
    at org.springframework.orm.hibernate5.SessionFactoryUtils.convertHibernateAccessException(SessionFactoryUtils.java:284)
    at org.springframework.orm.hibernate5.HibernateTransactionManager.convertHibernateAccessException(HibernateTransactionManager.java:802)
    at org.springframework.orm.hibernate5.HibernateTransactionManager.doCommit(HibernateTransactionManager.java:638)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:746)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:714)
    at org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:152)
    at grails.gorm.transactions.GrailsTransactionTemplate.execute(GrailsTransactionTemplate.groovy:91)
    at org.grails.datastore.gorm.GormStaticApi.with

and

org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [com.analytics.PageHitCounter#18]
    at org.hibernate.persister.entity.AbstractEntityPersister.check(AbstractEntityPersister.java:2604)
    at org.hibernate.persister.entity.AbstractEntityPersister.update(AbstractEntityPersister.java:3448)
    at org.hibernate.persister.entity.AbstractEntityPersister.updateOrInsert(AbstractEntityPersister.java:3311)
    at org.hibernate.persister.entity.AbstractEntityPersister.update(AbstractEntityPersister.java:3723)
    at org.hibernate.action.internal.EntityUpdateAction.execute(EntityUpdateAction.java:201)
    at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:604)
    at org.hibernate.engine.spi.ActionQueue.lambda$executeActions$1(ActionQueue.java:478)
    at java.base/java.util.Li

It is understandable that why this error could be triggered. To identify each page I have used this identifier

def hitcounter = "${controllerName}/${actionName}/${id}"

I simply check if hitcounter with this identifier is present, if no I create one but if it is already present then I increment the hits by 1. In other words, the first time the page is visited the PageHitCounter object is created, after that the object is fetched and the hits variable is incremented by 1. So if many people visit the page simultaneously then we can see while the object is being fetched and updated other updates might occur which can cause the optimistic locking failure.

This is the only solution I could think of for implementing the hit counter functionality. Maybe there is a better thread safe approach to implementing hit counter. How can I get this to work in a high concurrency environment?

Answer 1

Updated answer

Sorry, my mistake, transactions will not shield you against optimistic locking failures.

I think you'll need to lock down row updates in the application layer to prevent this.

package demo

class PageHitCounter {

    String id
    int hits

    static mapping = {
        id generator: 'assigned'
    }
}

package demo

class DemoController {

    private static final Map<String,Object> pageHitCounterLocks = new ConcurrentHashMap<>()

    def verifyRegistration(Long id) {

        ...

        def hits = 0
        def pageIdentifier = "${controllerName}/${actionName}/${id}"

        synchronized(pageHitCounterLocks.computeIfAbsent(pageIdentifier, k -> new Object())) {
            PageHitCounter.withTransaction {
                def pageHitCounter = PageHitCounter.get(pageIdentifier)
                if (!pageHitCounter) {
                    pageHitCounter = new PageHitCounter()
                    pageHitCounter.id = pageIdentifier
                    pageHitCounter.hits = 1
                } else {
                    pageHitCounter.hits = pageHitCounter.hits + 1
                }
                hits = pageHitCounter.hits
                pageHitCounter.save()
            }
        }

        [id: id, hits: hits]
    }
}

Old (wrong answer)

When you're not performing both the read and update operations within the same transaction, there's a risk that another thread could modify the database row between the moment you read the value and when you attempt to update it. That is what happens when you get an HibernateOptimisticLockingFailureException.

You should move reading from the database to occur inside the transaction.

Also, you can find or create the hit counter in one go.

def verifyRegistration(Long id) {

    ...

    def pageIdentifier = "${controllerName}/${actionName}/${id}"
    def hitCounter = null

    PageHitCounter.withTransaction {
        hitCounter = PageHitCounter.findOrCreateByPageIdentifier(pageIdentifier)
        hitCounter.hits = hitCounter.hits + 1
        hitCounter.save()
    }

    [id: id, hits: hitCounter?.hits]
}