-
- Downloads
[FIX] core: handle recursion error when resolving stored fields
Issue discovered in the uninstall (and reinstall) of sale_project: a dump has ~100 tasks, when reinstalling `sale_line_id` has to be initialised, this is done by marking `sale_line_id` on all extant tasks as to-recompute, which triggers their computation on the next `flush`. Because it's a recursive field, `Field.recompute` ensures only one record at a time gets recomputed (as there could be cross-dependencies in the recorset which protection would prevent from resolving). As the field computation runs, it accesses itself, which triggers a cache miss, which triggers a `_fetch_field` (to get the currently stored value), this calls `_read`, which flushes the field we're trying to read. The problem here is that for efficiency the cache miss will look for all records in the cache without a value for the field (`_in_cache_without`) and try to `fetch` on them as well. This means rather than not doing anything in flush, we're going to `Field.recompute` on all records except the one selected the first time around, which repeats the cycle until there is no more additional record found in `_in_cache_without`, which could trigger the next round of `recompute`, and the entire thing unwinds, and we probably perform a ton of unnecessary additional `compute_value`. Except that doesn't even happen, because the process from one compute to the next takes 12~13 stack frames, which given the default recursion limit of 1000 gives a hard limit of 76 fields before hitting a RecursionError. As this is less than 100, a recursion error [is what we get](https://runbot.odoo.com/runbot/build/31726625). In 15.2, this was fixed by only expanding the fetch on non-recursive fields, pessimizing recursive fields (5c2511115b14299516fce4aa3737a62faaf5b653). Test-wise this only impacted mail performances and in a relatively minor manner. In 16.0, the mail tests actually match already (so that part was skipped by the cherrypicking) however this impacts the knowledge perf tests much more significantly e.g. `test_article_creation_multi_roots` gets +9 queries when creating 10 top-level articles, which is a bit much. So use an alternative which is ugly as hell but which I didn't consider for 15.2 (may want to backport it one day if the current fix is an issue): catch the recursion error and use the existing fallback (of fetching just the requested record's field without expanding the recordset). This likely makes for a pretty inefficient situation in the original case as we're certainly going to hit the recursion limit repeatedly, but that still fixes the issue, and it avoids deoptimising cases which fall short of the recursion limit (resolving under 60 records or so). Plus despite creating giant stacks we might actually get good efficiency as we're going to hit recursion limits repeatedly but that's pure python, once we fall below the limit we can resolve everything at once with a single SQL query (or something along those lines). Part-of: odoo/odoo#119808
Loading