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

[FW][FIX] registry: avoid re-signaling invalidated cache #41502

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Dec 6, 2019

In a multithreaded 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.

One of multiple scenarios where this can cause issues:

  1. There are 2 threads busy threating a request each, [A] & [B].
  2. The database has it's ´base_cache_signaling´ sequence increased.
  3. [A] calls the registry's check_signaling, sees it should be
    invalidated and calls registry.clear_caches, setting
    registry.cache_invalidated to True while it clears the cache for
    each model.
  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.

Should 2 multi-threaded servers run in parallel on the same database
(both handling many requests), it's possible this triggers alternatively
on each worker 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 be in inconsistent 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.

Forward-Port-Of: #40983

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
@fw-bot
Copy link
Contributor Author

fw-bot commented Dec 6, 2019

This PR targets 13.0 and is part of the forward-port chain. Further PRs will be created up to master.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added forwardport This PR was created by @fw-bot CI 🤖 Robodoo has seen passing statuses labels Dec 6, 2019
@C3POdoo C3POdoo added the RD research & development, internal work label Dec 6, 2019
@robodoo robodoo added the r+ 👌 label Dec 9, 2019
@robodoo robodoo closed this in 5b248e4 Dec 9, 2019
@robodoo robodoo temporarily deployed to merge December 9, 2019 10:38 Inactive
@fw-bot fw-bot deleted the 13.0-11.0-multithreaded-cache-signaling-dve-oX_l-fw branch December 23, 2019 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses forwardport This PR was created by @fw-bot RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants