CODEWITHSUNDEEP
phplaravelcachingredis
Stas Coder
Stas Coder

Reputation: 347

Laravel cache returns corrupt data (redis driver)

I have an API written in Laravel. There is the following code in it:

public function getData($cacheKey)
{
    if(Cache::has($cacheKey)) {
        return Cache::get($cacheKey);
    }

    // if cache is empty for the key, get data from external service
    $dataFromService = $this->makeRequest($cacheKey);
    $dataMapped = array_map([$this->transformer, 'transformData'], $dataFromService);

    Cache::put($cacheKey, $dataMapped);

    return $dataMapped;
}

In getData() if cache contains necessary key, data returned from cache. If cache does not have necessary key, data is fetched from external API, processed and placed to cache and after that returned.

The problem is: when there are many concurrent requests to the method, data is corrupted. I guess, data is written to cache incorrectly because of race conditions.

Upvotes: 3

Views: 736

Answers (2)

Stas Coder
Stas Coder

Reputation: 347

In the end I came to the following solution: I use retry() function from Laravel 5.5 helpers to get cache value until it is written there normally with interval of 1 second.

Upvotes: 0

apokryfos
apokryfos

Reputation: 40690

You seem to be experiencing some sort of critical section problem. But here's the thing. Redis operations are atomic however Laravel does its own checks before calling Redis.

The major issue here is that all concurrent requests will all cause a request to be made and then all of them will write the results to the cache (which is definitely not good). I would suggest implementing a simple mutual exclusion lock on your code.

Replace your current method body with the following:

public function getData($cacheKey)
{
    $mutexKey = "getDataMutex";
    if (!Redis::setnx($mutexKey,true)) {
       //Already running, you can either do a busy wait until the cache key is ready or fail this request and assume that another one will succeed 
       //Definately don't trust what the cache says at this point
    }

    $value = Cache::rememberForever($cacheKey, function () { //This part is just the convinience method, it doesn't change anything
        $dataFromService = $this->makeRequest($cacheKey); 
        $dataMapped = array_map([$this->transformer, 'transformData'], $dataFromService);

        return $dataMapped;
    });
    Redis::del($mutexKey);
    return $value;
}

setnx is a native redis command that sets a value if it doesn't exist already. This is done atomically so it can be used to implement a simple locking mechanism, but (as mentioned in the manual) will not work if you're using a redis cluster. In that case the redis manual describes a method to implement distributed locks

Upvotes: 1

Related Questions