chunked: should it flush ?

[ivantodorovich]

The chunked method is invalidating the cache on each iteration, but it's never flushing.

I'm encountering an issue with this in Odoo 16.0, where by using chunked I get into CacheMiss errors due to the cache being lost, including the previous one before it's being called ⚠️ .

By looking at the code I see it's invalidating the cache but it's never flushing. This looks like a red flag to me, although I've never encountered any issues with chunked before.

Perhaps it's due to odoo/odoo#66938

[pedrobaeza]

chunked was programmed a long time ago, and it's not adapted to newer versions.

[ivantodorovich]

Ouch, that explains it then 😢
I'm worried now about all the other openupgradelib's method I used in the past 😓

💡 Perhaps we could have a warning system to manually whitelist methods for new versions

[pedrobaeza]

Well, this is the only method where it mixes ORM. The rest is thought for handling database things, so I don't think the rest are affected.

[StefanRijnhart]

It's a good question, and I don't think anybody explicitly checked this after 16.0 was released, but invalidate_all actually executes a flush by default: https://github.com/odoo/odoo/blob/16.0/odoo/api.py#L727-L736

[ivantodorovich]

It's a good question, and I don't think anybody explicitly checked this after 16.0 was released, but invalidate_all actually executes a flush by default: https://github.com/odoo/odoo/blob/16.0/odoo/api.py#L727-L736

Indeed it does, since: odoo/odoo#66938

The thing is we've stopped calling invalidate_all for version > 10 (we call env.cache.invalidate instead).

The fix could be to simply go back to calling env.invalidate_all for version >= 16.

However I'm wondering if we should explicitly call flush for version >= 12 instead, when flush was introduced. By reading the code, IMO it's a miracle that it works. I can't explain why the data is not lost in versions 12, 13, 14 and 15; there must be something else that's flushing and saving it. The intention of this method is clearly to flush our changes to the db, whilst clearing the cache before the next iteration.

[StefanRijnhart]

@ivantodorovich Thanks for clarifying! I think it's a very good idea of yours to go back to invalidate_all now that it is reintroduced, and to flush explicitly for the earlier versions that support it.

[ivantodorovich]

I've created this PR with a fix proposal:

• [FIX] chunked: explicitly flush on every iteration #364