Propagate read-only status to Hibernate Session through setDefaultReadOnly [SPR-16956]

[spring-projects-issues]

MIhalcea Vlad opened SPR-16956 and commented

By default,  @Transactional(readOnly = true) sets the FlushMode to MANUAL. However, Hibernate can save up lots of memory by discarding the associated hydrated state if we also set the session.setDefaultReadOnly(true).

Not only that we save memory, but we also save CPU cycles because, if the user tries to do a manual flush, we won't propagate it to any entity since they are virtually read-only.

---

Affects: 5.0.7

Issue Links:

• Support for Hibernate ORM 5.3 [SPR-16303] #20850 Support for Hibernate ORM 5.3
• Support Hibernate 5.3's ManagedBeanRegistry for dependency injection [SPR-16305] #20852 Support Hibernate 5.3's ManagedBeanRegistry for dependency injection
• Propagate read-only status as FlushMode.MANUAL to Query instances [SPR-15759] #20314 Propagate read-only status as FlushMode.MANUAL to Query instances
• HibernateTransactionManager (unintentionally) bound to Hibernate 5.2 SharedSessionContractImplementor [SPR-17557] #22089 HibernateTransactionManager (unintentionally) bound to Hibernate 5.2 SharedSessionContractImplementor
• LocalSessionFactoryBean and HibernateTransactionManager for JPA EntityManagerFactory setup [SPR-17002] #21540 LocalSessionFactoryBean and HibernateTransactionManager for JPA EntityManagerFactory setup
• HibernateTransactionManager should lazily acquire JDBC Connection (like HibernateJpaDialect) [SPR-17216] #21749 HibernateTransactionManager should lazily acquire JDBC Connection (like HibernateJpaDialect)
• HibernateJpaDialect cannot translate JDBCException to custom DataAccessException [SPR-17015] #21553 HibernateJpaDialect cannot translate JDBCException to custom DataAccessException

Referenced from: pull request #1861, and commits 010ba33, d22d408

0 votes, 5 watchers

[spring-projects-issues]

MIhalcea Vlad commented

Pull Request: #1861

[spring-projects-issues]

Juergen Hoeller commented

MIhalcea Vlad, does it make sense to also set each Query to FlushMode.MANUAL in such a scenario, as suggested by #20314? Or is this effectively covered by Session.setDefaultReadOnly(true) already?

[spring-projects-issues]

Juergen Hoeller commented

I've rolled this into HibernateTransactionManager in a slightly revision fashion, with no need to store the previous read-only setting at Session level since we're only applying it to a new Session which we're immediately closing afterwards anyway. This should also cover the intent of #20314, avoiding any dirty checking overhead at query level.

For the time being, I'd prefer leaving this as a Hibernate-native feature. After all, as of Hibernate 5.2, LocalSessionFactoryBean can even be used for a JPA setup since the SessionFactory exposes itself as an EntityManagerFactory directly. This allows us to put some features only there, with HibernateJpaDialect providing less impactful default adaptations with a focus on matching EclipseLinkJpaDialect (more so than on exposing Hibernate specifics). Our flush mode handling is an easier pill to swallow there since it is an extension of a JPA concept and just suppresses default flushes but doesn't impact the ability for custom flush attempts.

[spring-projects-issues]

MIhalcea Vlad commented

Hi, Jurgen.

I've just seen your question. I would not set the flush mode to MANUAL as that could prevent the Persistence Context to flush prior to executing the query and break read-your-own-write consistency issues.

Related to your changes, does Spring Boot use the HibernateTransactionManager or it always picks the JpaTRanscationMnagaer no matter the underlying JPA DIalect?

If Spring Boot always picks the JpaTranscationManager, then this fix will not have a significant impact.

[spring-projects-issues]

Juergen Hoeller commented

I've added corresponding behavior to HibernateJpaDialect as well, based on a specific marker on the passed-in TransactionDefinition which indicates a transactional-local resource (analogous to the check in HibernateTransactionManager) and therefore allows for side-effect free hard read-only behavior... and also avoiding the need for post-transaction cleanup. This brings the same benefit to JpaTransactionManager now.

As for the flushing question, we were specifically wondering whether an explicit query-level FlushMode.MANUAL setting in addition to session-level FlushMode.MANUAL makes any difference... in particular next to Session.setDefaultReadOnly(true). As far as I can see in current Hibernate code, a query-level FlushMode simply temporarily overrides the session-level FlushMode, so there doesn't seem to be any need for repeating MANUAL there.

[spring-projects-issues]

MIhalcea Vlad commented

That's great to hear. I'll test it once 5.1 RC1 is released.

As for the query-level flush setting, I don't see how it could make any difference to the Session-level one. If the Session is set to FlushMode.MANUAL, no Query is allowed to flush anyway.

Thanks for integrating it. 

[spring-projects-issues]

Neale Upstone commented

Juergen Hoeller Would you consider this for backport to 4.3.x?

It appears a safe (and sounding significant) performance improvement, which multiplied across many 4.3.x applications would save quite some CPU cycles and energy consumption.  Naturally I'm thinking anything that'll help save the snow in Austrian ski resorts.

[spring-projects-issues]

Juergen Hoeller commented

I'd expect all the Austrian-affecting Spring/Hibernate applications to upgrade to 5.1 immediately in any case ;-)

On a serious note, it's actually mostly memory saved here since in a regular Spring read-only transaction no flush is ever being triggered... and even there, it's mostly memory being made available to the GC earlier than before. It might not have much effect in a typical short-lived transaction at all if the GC only kicks in after the transaction in any case.

CPU cycles would only really be saved in case of a user-level flush attempt. Ironically this is exactly the problem with backwards-compatibility: If a user tried such a manual flush() call in a Spring read-only setup, it would currently override the read-only setting and actually flush all loaded entities. As of 5.1, any such manual flush will be in vain, and entities loaded within a read-only transaction are strictly read-only. While this is arguably ok for our intentions, I'm sure that some existing code out there - maybe totally unaware of the read-only flag set somewhere around it - would break, and it'd be totally non-obvious to the innocent patch release upgrader. This in particular applies to JPA access code (since JPA has no read-only or manual flush concept) but also to native Hibernate access code.

All in all, I'm willing to enforce and defend such stricter semantics in a new feature release but I'm not sure that backporting to 4.3.x would be wise here. The JPA part is rather involved to begin with since it needs an extension in the dialect contract and spans a wider range of Hibernate versions in 4.3.x. Even for the native Hibernate part, a backport might cause more hard-to-track harm than noticeable benefit.

[spring-projects-issues]

Neale Upstone commented

Thanks Juergen Hoeller.  I'll just have to push the upgrade forwards in our case. I suspected that might be the answer, but it was worth checking.