Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate read-only status as FlushMode.MANUAL to Query instances [SPR-15759] #20314

Closed
spring-projects-issues opened this issue Jul 11, 2017 · 5 comments

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 11, 2017

Eduardo Simioni opened SPR-15759 and commented

Our setup uses

org.springframework.orm.hibernate5.HibernateTransactionManager and
org.springframework.orm.hibernate5.LocalSessionFactoryBean
Hibernate 5.2.10.Final

We also use compile time aspects for the transactions, although I don't think this would be a determinant factor for this bug.

The problem

I've put the following code inside a @Transactional(readOnly = true) method (the only transactional method in the chain):

LOG.info("Hibernate flush: " + sessionFactory.getCurrentSession().getHibernateFlushMode());
LOG.info("JPA flush: " + sessionFactory.getCurrentSession().getFlushMode());

This yielded the following result:

Hibernate flush: MANUAL
JPA flush: COMMIT

Plus the following Hibernate log line:

[org.hibernate.jpa.internal.util.FlushModeTypeHelper] Interpreting Hibernate FlushMode#MANUAL to JPA FlushModeType#COMMIT; may cause problems if relying on FlushMode#MANUAL-specific behavior

I've noticed this by chance. After setting one of our main queries (single method request) to FlushMode.MANUAL, the total request time dropped from 550ms to 110ms on average.
Most of the time was being spent on Hibernate checking dirty entities, which led me to conclude that Spring was not setting the FlushMode correctly.

I'm not sure if this bug is present on 5.0 branch, but since I didn't find any similar issue, I'd guess it is.


Affects: 4.3.9, 5.0 RC2

Issue Links:

  • #21494 Propagate read-only status to Hibernate Session through setDefaultReadOnly
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 11, 2017

Juergen Hoeller commented

Hibernate 5.2 has refactored its entire JPA handling into the Session contract which is why there are two flush modes now... However, the primary flush mode is still Hibernate's, and on commit, FlushMode.MANUAL still suppresses the flush. The JPA flush mode enum cannot express that state so it is indicating COMMIT there; but that's just for that particular getFlushMode() getter, not reflecting the actual internal state of the Session.

So you're saying that setting the flush mode on the Query itself suppresses the flush - but the general session-wide flush mode doesn't? Could you try to debug a bit further to find out why SessionImpl.flushBeforeTransactionCompletion doesn't respect a session-wide flush mode MANUAL as it is expected to?

As a side note, a downgrade to Hibernate 5.1.8 might help since that generation still comes with the original Hibernate EntityManager separation: that is, no JPA flush mode at the Session level to begin with, so I assume no interference with MANUAL whatsoever.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 14, 2017

Eduardo Simioni commented

I've had assumed too much. After debugging SessionImpl.flushBeforeTransactionCompletion I can confirm it is working as expected.
The problem seems to be in Hibernate itself, I thought that setting a query to FlushMode.MANUAL on a MANUAL Session should have no effect, but it does.

I've had also assumed that the extra time to execute the method was due to Hibernate dirty checking mechanism, but it was due to how Hibernate is populating the results of the query whether it is set to FlushMode.MANUAL or not, regardless of the Session FlushMode.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 14, 2017

Juergen Hoeller commented

Well, let's turn this into an improvement request then :-)

We can try to propagate the read-only status as FlushMode.MANUAL to Query instances as well. We could create a Session proxy for read-only transactions that transparently applies FlushMode.MANUAL to each created Query. I wonder why Hibernate isn't doing that itself though; is there a specific disadvantage to such flush-restricted queries?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 17, 2017

Eduardo Simioni commented

The results may vary, but in our experience changing all queries to FlushMode.MANUAL only had positive impacts.
It didn't affect the functional behavior of our application, even for writable transactions.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 3, 2018

Juergen Hoeller commented

Closing this in favor of #21494 since Session.setDefaultReadOnly seems to have the same effect and is easier to apply. If there is any extra benefit in Query-level settings - whether setReadOnly or setFlushMode -, we may reopen this issue... or ideally raise the remaining differences with the Hibernate team first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.