infoscout / django-cache-utils

Django cache utilities
Other
26 stars 19 forks source link

Prevent double exectution of cached function when it is not cached yet, and takes some time of excecution #16

Open igor-kremin opened 6 years ago

igor-kremin commented 6 years ago

Hello, I with help of my friend found an issue which is in when getting a value from cache. When two processes goes one by one, and function takes some time to return result, both are checking the value in the cache, for example it is absent and both cache.get returns None, then both run function, which makes duplicated action.

So to avoid this behaviour you should use lock key and use cache.add function which returns True if value pushed in cache succesfully and false if it is already in the cache and 0 if cache backend is not runing. cache.add is atomic function.

Here is my implementation of cached decorator

def cached(timeout, force_key=None):
    def _cached(func):
        func_type = _func_type(func)
        @wraps(func)
        def wrapper(*args, **kwargs):
            if not hasattr(wrapper, '_full_name'):
                name, _args = _func_info(func, args)
                wrapper._full_name = name
            key_hash  = _cache_key(wrapper._full_name, func_type, args, kwargs, force_key)
            ok = cache.add(key_hash, True, timeout)  
            if ok is False:         # not ok is is not what is perfect it can be int 0  when backend not running
                start = time.time()
                while (time.time() - start) <= timeout and cache.get(key_hash):
                    result = cache.get(key_hash)
                    if isinstance(result, list):
                        return result[0]
                    else:
                        time.sleep(0.01)  wait fo result 
            elif ok == 0:
                pass
                # some action to inform user to start cache backend
            result = func(*args, **kwargs)
            cache.set(key_hash, [result], timeout)
            return result
        def invalidate(*args, **kwargs):
            if not hasattr(wrapper, '_full_name'):
                return
            key = _cache_key(wrapper._full_name, 'function', args, kwargs, force_key)
            cache.delete(key)
        wrapper.invalidate = invalidate
        return wrapper
    return _cached
RevolutionTech commented 5 years ago

Hi @igor-kremin, sorry for the late reply on this issue. It's taken a while for me to get the time to respond to this.

I think the issue you've brought up is valid, if I've understood it correctly. I'm not the biggest fan of the while loop however. How do you feel about using cache.get_or_set() instead? I think get_or_set() might handle this race condition.

igor-kremin commented 5 years ago

Hi, get_or_set will not solve double execution, I tried this before and had some problems on it. this while loop waits while a function in another process finished and sets the result in cache. Some times it is better to wait 0.1 -2 sec than execute function again. Especialy when functions takes some time.

RevolutionTech commented 5 years ago

Realistically I don't think we can merge in a change like this, because for some applications waiting for 0.1 seconds would not be acceptable, and in my opinion django-cache-utils should not be designed in a way that excludes these use cases.

Is there a way we can solve your use case while at the same time avoiding the use of time.sleep()? What issue did you run into with get_or_set()?

igor-kremin commented 5 years ago

Hello Lucas, sorry for long delay, When the first run of cache_backend.get(), the function is not yet executed (the function result does not exist yet), and what you put in the cache? So I decided to put a Truevalue to avoid duplicate execution of the function. cache_backend.add() an atomic function, it is important. It's make semaphore True until function ends and on end it replace True with result of the function. Usualy this decorators used on heavy (long) function and the delay 0.1 s is minor,

I did this changes in my project when I wrote the first message here , and everything is going well till now.