Cacheops don't invalidate on old state when force_update=True is used about django-cacheops

[bourivouh]

I have the same bug. Django-rest-framework 2.2.7 set force_update to True when models update (https://github.com/tomchristie/django-rest-framework/blob/2.2.7/rest_framework/mixins.py#L124).
Do you have any ideas about this?

[Suor]

The simplest alternative is to fetch old object on pre_save signal. That will add one more db call for any .save(). Other variant is to catch object attribute writes, preserve old state somehow and then use it in invalidation. This one adds overhead on all attribute writes on instances of cached models.

I'll prefer second one, but it's harder to implement.

[bourivouh]

Thanks!
I solved this problem in my concrete case by adding .exists() before save (only in force_update case). It's not universal, but enough for now.
I'm interested do you plan to fix this bug?

[Suor]

Some day

[tumb1er]

You can move "hack" from QuerySetMixin.exists() method to _update(values) method which is called just after (force_update or <check exists()>) condition:
https://github.com/django/django/blob/stable/1.4.x/django/db/models/base.py line 626

It will work in 1.3, 1.4 and 1.5 versions, but in master branch code is different :(

[tumb1er]

In django master there is a method _do_update, which also calls _update, so fix may work with future versions also.

[Suor]

I'm working on better solution, which will use
http://github.com/Suor/django-dirty

[Suor]

After some research, looks like there is no easy and cheap way to track changes in model instance. Still overriding _update doesn't make sense, it will lead to extra request to db instead of just using the one done anyway in exists call. And if you are ok making extra request, you should just do it on pre_save.

[tumb1er]

Кажется возможно решить проблему хотя бы статистически. По степени критичности проявления ошибки можно выделить:

1. Однопоточное исполнение, выбираем объект модели по pk, меняем свойства с force_update и оп! бага с инвалидацией старых значений. При этом объект модели уже находится в кэше на момент выполнения _update. Стандартная ситуация при PUT в djangorestframework 2-че-то-там.
2. Однопоточное исполнение, выбираем objects.filter(field_name=field_value)[0], объект есть в кэше, но закеширован не по первичному ключу. При сохранении с force_update вызов _update плодит запрос к БД, но сама ситуация это изменение списка объектов в цикле с использованием force_update. Может и существует в природе, но имхо не часто встречается.
3. Многопоточное исполнение, выбираем объект модели по pk, ждем 5 минут, сохраняем с force_update и вот она бага, да еще и с гонками. Статистически редка, и тоже фиксится модификацией _update.
4. Многопоточное исполнение, queryset[0], ситуация гонки с модификацией через другой queryset. Мозг вытек обдумывать способы решения данной проблемы, но кажется тут только запрос к БД решит.

Убедил?

It seems that it's possible to fix this bug at least statistically. There are several ways to reproduce this bug with different level of importance.

1. Single-threaded: getting object with PK, saving with force_update and here is the bug. At the time of _update call object is already in cache. This usecase is typical for PUT in djangorestframework 2.x
2. Single-threaded when changine objects.filter(field_name=field_value)[0]. Object is in cache but not cached with PK value. With _update fix it will query database, but IMHO the usecase is very rare.
3. Multi-threaded execution, getting object with PK, waiting for 5 min, saving with force_update and here is the bug with race-conditions. Very rare usecase and could be fixed with _update modification.
4. Multi-threaded excution, selecting queryset[0], changing anotherset[3]. It blowed my mind, and I think only DB query could solve it.

Am I right?

[Suor]

I don't understand what point are you trying to make here?

If you are saying that extra request isn't that bad since it will be usually cached anyway then I agree.
If you are trying to convince me to override _update() then it still doesn't make any sense since attaching handler to pre_save event would be cleaner while being same in efficiency.

[tumb1er]

I'm saying that fixing pre_save in next release is better than do nothing while trying to propose better decision and it will not harm performance seriously.

[Suor]

Django 1.6+ default behavior is to use pre_save signal now. Which will bypass this problem. cacheops will still use old algorithm in case select_on_save = True is set on models Meta.

It's possible I'll get rid of old faster, but buggy method in future cacheops.