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 critical bug: cache invalidation in QueryCache #3515

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nPraml
Copy link
Contributor

@nPraml nPraml commented Oct 31, 2024

Hello @rbygrave,

In our application we found an interesting situation when the caches are not working correctly.
We have two threads, T2 is a worker thread that periodically checks a table to see whether there are new records.
T1 creates such data sets.

Use case: T1 inserts a record into the database and after that notifies T2 that the data now exists and he should start working now.
T2 has a query with .setUseQueryCache(true). Normally T2 should now find the inserted record from T1 (that's how the where conditions are defined).
If the timing happens that T1 inserts the record while T2 happens to be executing a DB.find.setUseQueryCache(true), incorrect data is stored in the cache. From then on, the data record will not be found, even though it is in the DB. Not even if you wait a few seconds.

We wrote a test case that reproduces the behavior and tries to explain what happens in the table below:

    Thread 1        |  Thread 2
---------------------------------------------------------
                    |  T2 before FIND1
                    |    find (with .setUseQueryCache(true))
T1 BEFORE Insert    |
  insert into       |  
  clear CACHE       |
T1 AFTER Insert     |
                    |    put CACHE <-- The error is here
                    |  T2 FIND1: 0 <-- ok, not yet synchronized
                synchronize
                    |  T2 before FIND2
                    |    find (with .setUseQueryCache(true))
                    |    hit CACHE
                    |  T2 FIND2: 0 <-- wrong

in my opinion put cache overwrites the current state of what T1 has written. A possible fix could be that we remember the cache.clear count before the find and take this into account when putting cache.
Or implement another type of locking.

Cheers
Noemi

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

Successfully merging this pull request may close these issues.

1 participant