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

[I+D] tools/cache.py: Clean cache for just one method instead of a full clean cache #35012

Closed
moylop260 opened this issue Jul 19, 2019 · 5 comments

Comments

@moylop260
Copy link
Contributor

Steps to reproduce:

  1. Add orm.cache decorator for a method:
  • tools.ormcache('self._uid', 'model_name', 'mode',
    'tuple(self._context.get(k) for k in self._compute_domain_keys())'),
    )
    def _compute_domain(self, model_name, mode="read"):
  1. Now, you need clearing cache:
  • @api.multi
    def unlink(self):
    res = super(IrRule, self).unlink()
    self.clear_caches()
    return res
    @api.model_create_multi
    def create(self, vals_list):
    res = super(IrRule, self).create(vals_list)
    self.clear_caches()
    return res
    @api.multi
    def write(self, vals):
    res = super(IrRule, self).write(vals)
    self.clear_caches()
    return res

Current behavior:

Clearing cache send signal to clean:

  • all caches
    • all modules
      • all cached methods
        • all workers

It is so expensive even if you want to clean just one particular method.

Expected behavior:

For python3 we have a built-in method similar: lru-cache

It adds the option of clean cache just for that method.

I mean,

    @lru_cache()
    def my_method_cached(self):
         return {}

    def write(self):
         self.my_method_cached. cache_clear()  # Notice that we are cleaning cache of that method.
         return super().write()

Then we can use a similar way to clean cache for just one method or even better we can use the built-in method.

@odony
Copy link
Contributor

odony commented Jul 19, 2019

tl;dr: don't waste time with this, you can only make the cache management more expensive on average, and more difficult to synchronize across workers - and you will not be able to get meaningful speedups with this.


Our current cache implementation is based on simple requirements:

  • We must be able to synchronize it easily in multi-worker mode
  • The cache invalidation must be very fast
    These requirements are much more important for performance than having an optimal granularity in cache invalidation.

The current system is excellent to meet these requirements: in multi-worker we only need a "cache version" DB sequence, which is the simplest and cheapest IPC we could find. And the invalidation itself is super-trivial: empty the single cache LRU.

It may seem extremely brutal as an invalidation method, but in practice it is very efficient. If you use the debugging signals to watch worker cache on a heavily used production instance, you will notice several things:

  • Cache invalidations happen often but not too often, so few requests are served with a cold cache
  • Requests served with a cold cache are not particularly slow

Example: on odoo.com today we had 172 invalidation since last daily log rotation, for about 3 million requests served, so 1 invalidation / 17k requests. For us this corresponds to 1 invalidation every 2-3 minutes.

These surprising results are explained by a simple reason: the cache is mostly useful within a single transactional request, when the same function is called many many times by the same transaction, because it's working on batches of the same records (e.g ACLs, record rules, group membership are always the same). The same cache entries can be marginally helpful for subsequent requests, but they will make much less difference, it's a small bonus, because the cached methods are not that slow. If they were, it would be critical to reduce the number of cache invalidation we trigger, and recycling an HTTP worker would be a bug perf hit because the new one starts with an empty cache. But it's not a problem in practice.

If you think about it, we are using a single LRU cache of 8k entries for all the orm_cached entries of the database. Doesn't that seem a little bit too small? Aren't we evicting cache entries all the time and e.g. wasting precious compiled QWeb templates and replacing them with dumb translations?
Well, it turns out that we aren't. Once again, the lifecycle of the cache is so short that we never manage to fill the 8k entries before they get discarded - and that's really fine, because the cache mostly matters inside a transaction.

I encourage you to verify what I'm saying on a really active production instance by sending SIGUSR1 signal to the HTTP workers, the results might surprise you 😉

@moylop260
Copy link
Contributor Author

moylop260 commented Jul 19, 2019

Thanks for quickly answer and very good explanation (as ever)
You are right!

Maybe our case of use has another solution.

We are using ir.rule based on dynamic values changed by user.
I mean, something like:

  • [('warehouse_id', 'in', 'user.partner_id.sale_team_ids.mapped('warehouse_ids').ids')]

Then, we will need a clear cache for each change of:

  • user.partner_id
  • partner_id.sale_team_ids
  • sale_team_ids.warehouse_ids

Then the calls for clear all caches will increase considerably and it is counterproductive.

I will looking for a workaround to minimize the problem.

@nhomar
Copy link
Collaborator

nhomar commented Jul 19, 2019

@moylop260

idea

What about a server action that autocreate the ir.rule with the wired ids?

@JonathanStein
Copy link

What about a server action that autocreate the ir.rule with the wired ids?

Can you be more specific on this idea?

We have a lot of such "expensive" rules, but as far as I can see, we would end up with one rule per user - and all rules would have to be evaluated on each check.

@veryberry
Copy link
Contributor

@odony imagine we have odoo instance running in 10+ workers, user is browsing website pages, it's pretty different from the /web backend (where you only need to load once all cache, and then send small ajax requests while browsing UI menus), on the website every page is like opening /web every time. If system cleans cache between all workers every 2-3 min, and user may not go to then same process (worker) every time opening page, so every next N(num of workers) page clicks he may trigger cache computing and storing , and he will feel very well the slowness of the website. the number of workers multiplies the problem in this case.
I've run an experiment running odoo with 1 worker and with many workers and after clearing all cache (e.g. after some import or editing system params) I've observed logs, response time, number of DB queries when browsing website pages and in the case of many workers the slowness problem is multiplied.
Our customers feel this problem very well when using portal, so I need to think up about how to organise odoo processes and routing and minify cache clearing.
So considering portal and website pages and not /web UI, it's not so happy to live with cache cleaning as you described.

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

5 participants