grails / grails-data-mapping

GORM - Groovy Object Mapping
http://gorm.grails.org/
217 stars 198 forks source link

Multi-tenant entities not eligible for hibernate 2nd level caching #1088

Open longwa opened 6 years ago

longwa commented 6 years ago

In testing multi-tenancy for our application I realized that any entity which implements MultiTenant is not eligible for 2nd level caching at all in Hibernate.

This is currently by design in AbstractHibernateGormStaticApi.groovy in the get(...) method:

        if(persistentEntity.isMultiTenant()) {
            // for multi-tenant entities we process get(..) via a query
            (D)hibernateTemplate.execute(  { Session session ->
                def criteria = session.createCriteria(persistentEntity.javaClass)
                criteria.add Restrictions.idEq(id)
                firePreQueryEvent(session,criteria)
                def result = (D) criteria.uniqueResult()
                firePostQueryEvent(session, criteria, result)
                return proxyHandler.unwrap( result )
            } )
        }
        else {
            // for non multi-tenant entities we process get(..) via the second level cache
            return (D)proxyHandler.unwrap(
                hibernateTemplate.get(persistentEntity.javaClass, id)
            )
        }

Hibernate 5.1 and later support cache aware multi-tenancy since the QueryKey class used for querying the second level cache is tenant aware (it includes the tenantIdentifier in the key).

However, it seems like Grails isn't really configuring the hibernate multi-tenancy (or not configuring it fully) since the sessionOptions on the SessionFactory configured by Grails indicates tenancy of 'NONE' even when multi-tenancy is enable for Grails. Perhaps this is to abstract multi-tenancy to support more datastore types, I'm not sure.

This is a huge performance impact for our application as most of our domain objects needs to exist in both tenants and therefore don't make good use of the 2nd level cache, causing thousands of extract queries for some things.

Steps to Reproduce

  1. Create a domain object that implements MultiTenant and make it cached (nonstrict-read-write) in our case.

  2. Fetch that object multiple times in different sessions and observe queries each time

Expected Behaviour

Ideally, Grails should configure and defer the tenancy to Hibernate and use the configured hibernateTemplate.get(...) for everything. As long as hibernate is "tenant-aware", it looks like it should respect the tenant when pulling from the cache.

graemerocher commented 6 years ago

We investigated integrating native multi tenancy but it requires integration at the connection/session level and had various problems at the time.

2nd level cache support is maybe desirable, however caching in general is actually better applied at the service level ie( @Cacheable ) where it can be tuned and controlled better IMO

longwa commented 6 years ago

We do use @Cacheable a good amount but that's at a higher level than this. Our application has a lot of domain classes with to-one reference data is that very efficiently loaded from the 2nd level cache. For our multi-tenant needs, all of our domains exist in both databases (it's basically a live vs. reporting (frozen) view of the data).

Do you have any notes that you could pass along on some of the problems you encountered with the native implementation?

I might be able to spend some time looking into a version that worked natively.

graemerocher commented 6 years ago

the main issue is around switching between two tenants. If you call 2 methods against 2 different tenants the session management becomes problematic since the tenant id is specified when opening a session and there is no apparent way to switch tenants for an active session:

Session session = sessionFactory.withOptions()
        .tenantIdentifier( yourTenantIdentifier )
        ...
        .openSession();
longwa commented 6 years ago

I wouldn't expect switching tenants for a single active session would ever be supported. I always assumed that if you are working with two tenants you would have two distinct sessions.

I guess the challenge just becomes binding those sessions in the right context and making sure they are used correctly. That would probably mean the bindNewSession code and things along those lines need to be tenant-aware as well.

jchharris commented 4 years ago

Any plans to revisit using the hibernate native multi-tenancy?

oblakhh commented 4 years ago

Prove me wrong, but I discovered today that the very same code as mentioned above even leads to circumvention of the 1st level cache in all cases.

Today I started trying to port an existing application to multiTenancy.mode: DATABASE and I thought I was done when I realized very poor performance of some particular requests during testing. Turns out, that Spring Security is calling User.get(id) over and over again, when calling springSecurityService.getCurrentUser(). Usually the hibernate session cache would deal with that, so no subsequent queries. Now, with multi tenancy in place, the database gets hammered again again with the same query select [...] from user where id = 123.

So, for DATABASE mode it would be a true relief if you could make a more elaborate distinction here, even if it is the less common mode.

leehaotan commented 3 years ago

@oblakhh I am also facing this issue while upgrading from Grails 2 to 4. A simple domain get() method will trigger repeated queries instead of getting from the first level cache in the case when it's called in a loop.

This wasn't an issue back in Grails 2, but it is slower in Grails 4 since it triggers an AutoFlushEvent.