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

Possible bug: Setting isolation level disables query cache #2887

Merged

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Nov 9, 2022

We discovered an issue, that the query cache does not work, when isolation level is set.

use case
We show a counter in the UI how many unread messages a user has. It is important, that this method is fast and does not block so we set READ_UNCOMMITED. We also want to use the query-cache in this case.

As we only want to show an indicator of unread messages, it is not important to get the exact correct number. (it is better to get a possible wrong number than blocking the UI request when some tables are locked)

What happens
TransactionFactory.setIsolationLevel calls connection and JdbcTransaction.connection sets queryOnly = false: https:/ebean-orm/ebean/blob/master/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java#L768

Question
Is this really a bug or intended, that the query cache will be disabled when isolation level is changed?

I know, that the result of READ_UNCOMMITTED may differ from the cache. But it seems it does not matter to which value you set the isolation level.

@rbygrave
Copy link
Member

rbygrave commented Nov 9, 2022

Is this really a bug

Yes this is a bug.

There is public Transaction.connection() and internal SpiTransaction.getInternalConnection() and it should be using getInternalConnection() here.

The intention wrt connection() setting queryOnly=false is that when application code [not ebean code] in particular accesses the Connection directly using Transaction.connection() then we can no longer assume that it's read only as ebean has no idea what the application code is going to do with that connection.

The bug here is that in TransactionFactory.setIsolationLevel() we really do know that the only thing happening here is that the isolation level is being set SO ... it should use getInternalConnection() here.

On a similar note, we can see one other case in CQueryBuilder.createNativeSqlTree(), that should also change to use getInternalConnection().

rbygrave added a commit that referenced this pull request Nov 9, 2022
Use getInternalConnection() in order to set the queryOnly = false flag
which is done via connection()
@rbygrave rbygrave merged commit 0fd2f34 into ebean-orm:master Nov 9, 2022
@rbygrave rbygrave added the bug label Nov 9, 2022
@rbygrave rbygrave added this to the 13.10.2 milestone Nov 9, 2022
@rPraml rPraml deleted the bug/transactional-affects-query-cache branch January 23, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants