Skip to content

Commit

Permalink
[FIX] registry: avoid re-signaling invalidated cache
Browse files Browse the repository at this point in the history
In a multi-threaded environment, when the cache is being cleared through
a call to `registry.clear_caches`, it's possible any number of other
threads signal the database that the cache has been invalidated,
causing extra, unnecessary invalidations, possibly in a looping
fashion.

A scenario where this can cause issues:

1) There are 2 threads busy handling a request each, [A] & [B].
2) The database has its ´base_cache_signaling´ sequence incremented.
3) [A] calls `registry.check_signaling`, sees the cache should be
   invalidated and calls `registry.clear_caches`, setting
   `registry.cache_invalidated` to True for each model it clears the
   cache for.
4) [B] finished serving its request, calls `check_signaling`, sees
   `registry.clear_caches` is True and signals the database by
   increasing the `base_cache_signaling` sequence.
5) [A] finishes looping through the models being invalidated and
   attempts to clear the `registry.cache_invalidated`, but it's too
   late because [B] has already read it.

Should 2 or more multi-threaded servers run in parallel on the same
database (both handling many concurrent requests), it's possible this
triggers alternatively on each server and loops indefinitely.

To fix this problem, we avoid setting `registry.cache_invalidated` to
True when clearing the cache.

It's interesting to note that the cache invalidation is not thread-safe,
and can lead to inconsistent cache states during its invalidation &
clearing. Fixing this fully requires a bigger design change that may come
in the future, but most likely not as a fix in a stable version.

X-original-commit: 8a7fe5f
  • Loading branch information
Icallhimtest authored and odony committed Dec 6, 2019
1 parent 9294060 commit 89e8169
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions odoo/modules/registry.py
Expand Up @@ -394,8 +394,9 @@ def check_signaling(self):
# Check if the model caches must be invalidated.
elif self.cache_sequence != c:
_logger.info("Invalidating all model caches after database signaling.")
self.clear_caches()
self.cache_invalidated = False
# Bypass self.clear_caches() to avoid invalidation loops in multi-threaded
# configs due to the `cache_invalidated` flag being set, causing more signaling.
self.cache.clear()
self.registry_sequence = r
self.cache_sequence = c

Expand Down

0 comments on commit 89e8169

Please sign in to comment.