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

Cache is still used when setting useCache as false #3309

Closed
iscy opened this issue Dec 3, 2024 · 8 comments
Closed

Cache is still used when setting useCache as false #3309

iscy opened this issue Dec 3, 2024 · 8 comments

Comments

@iscy
Copy link

iscy commented Dec 3, 2024

MyBatis version

3.5.17

Database vendor and version

PostgreSQL 16.4

Steps to reproduce

  1. Enable the statement logs in Postgresql
  2. Set @Options(useCache = false) on a read query
  3. Create a single SqlSession and invoke that query multiple times

Expected result

You should be able to see the query in the statement logs of Postgresql

Actual result

The result of the query will be coming from the cache even though it's supposed to be disabled

Explanation

We have a use case where we simply want to ensure that our query gets to the database every time. We don't want to flush the cache. We simply want to control whether the query hits the database or the "cache or database". While investigating, we noticed where the issue is:

CachingExecutor.java:99 has the right logic
BaseExecutor.java:154 is where the problem is

Caching is only allowed when useCache is true and no resultHandler have been specified. CachingExecutor properly checks for both conditions. However, BaseExecutor doesn't currently check useCache. Adding this simple check fixes the problem.

There are also other "reports" of this issue on stackoverflow:
https://stackoverflow.com/a/26487839

The current workaround is to flush the cache, which is not optimal for many use cases.

PR will be coming shortly.

@harawata
Copy link
Member

harawata commented Dec 3, 2024

Hello @iscy ,

MyBatis uses two types of caches i.e. local cache (a.k.a. 1st level cache) and 2nd level cache.

The cache used in the repro steps is the local cache, whereas useCache is a setting for the 2nd level cache.
The proper solution to your problem probably is to set localCacheScope=STATEMENT in the config.

@iscy
Copy link
Author

iscy commented Dec 3, 2024

Hi @harawata,

Unfortunately, the local cache scope setting isn't per query, but a much wider setting. I'm aware of both caches (local & 2nd level), but fail to see why only "half the caches" should respect the useCache setting.

In any case, if we want to retain the current caching behavior and allow a single query to ignore all caches without flushing the cache, then I feel I would need an extra parameter. Would you guys be open to add a new parameter to control this behavior? I don't mind constructing the PR for such a change.

Thank you,

@harawata
Copy link
Member

harawata commented Dec 4, 2024

Well, it has been requested several times already (and there even are some PRs, I believe), but it was rejected by my predecessors repeatedly (because there are some tricky scenarios, I think), so I am not comfortable with accepting it.
If you cannot set localCacheScope=STATEMENT, then your only option at the moment may be 1) to call SqlSession#clearCache() or 2) to set flushCache="true" as mentioned in the SO answers.

@iscy
Copy link
Author

iscy commented Dec 4, 2024

Understood. Since we really don't want to actually touch the caches, the best workaround we currently have is to use a ResultHandler. It's not very self documenting from our code though since all we return from this query is an integer, but it works. A very simple IntResultHandler triggers the right code path and ensure that the rest of the queries are still able to use the caching mechanism.

For anyone reading this in the future, here are the two tricks:
@Options(flushCache = Options.FlushCachePolicy.TRUE) if you don't care about the cache
@ResultType(Integer.class) + IntResultHandler to skip the cache on a single statement

Thank you,

@iscy iscy closed this as completed Dec 4, 2024
@harawata
Copy link
Member

harawata commented Dec 4, 2024

@iscy ,

Just curious, what is the reason you cannot set localCacheScope=STATEMENT?
Do you actually have another place in the same app that requires localCacheScope=SESSION?
Or, you just want to avoid possible breakage caused by the global localCacheScope change?

@iscy
Copy link
Author

iscy commented Dec 4, 2024

@harawata, the SqlSession is used for different queries on different mappers. The current code in question is executed within a longer task and once in a while, a simple query is executed in order to make sure that it is still relevant. This is why we need to bypass it here.

As for the local cache scope set as SESSION, it is helping the separation of concerns within the code base. We don't rely on it for behavior, but to minimize the hits on the databases. It allows us to not have to worry about maintaining our own cache at the application layer.

As a side note, we also use the annotation @Options(flushCache = Options.FlushCachePolicy.TRUE) on some statements as well when we actually want to make sure that the cache gets flushed. This is usually done when queries feature a 'SELECT [...] FOR UPDATE' clause.

@harawata
Copy link
Member

harawata commented Dec 4, 2024

Thank you for the explanation, @iscy !

I asked because it seemed highly unlikely that two statements (or sets of statements) in the same project requires different local cache scope.
In reality, localCacheScope=STATEMENT has negative impact on performance only in rare situations involving nested selects, I think.
Also, localCacheScope=SESSION actually has negative aspect as it requires more memory in general.

Anyway, I appreciate your input.
It is one of the many things we should consider in a future major (or minor) version update.

@iscy
Copy link
Author

iscy commented Dec 4, 2024

Yep, totally agree with you on this! The entire platform has been developed with localCacheScope=SESSION in mind, but then we have a few outliers where we want to skip that cache, which is why we started to look into it.

I can also see why you would want to force the localCache for nested queries. However, in the case that the statement is very simple and should ignore (but not flush) the caches, it would be really nice to have a way to control this behavior. It's basically the same as 'ResultHandler', as when it's specified, it becomes very clear that the final query out of the nest won't use the usual facilities.

The possible solution I'm thinking about, since we started the chat, is to check the 'queryStack' counter to see if we can apply the 'useCache' directive. Example:
list = (ms.isUseCache() || queryStack > 1) && resultHandler == null ? [...]

The code always make sure that we are on the last query before executing the 'flush' directive (line 148 and 163). Such a change wouldn't cause breakage in query stacks and would give opportunity to the client app to control the behavior. Basically the same as the flush policy -- it would work on both the local and the 2nd level caches.

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

No branches or pull requests

2 participants