fastapi-cache icon indicating copy to clipboard operation
fastapi-cache copied to clipboard

Redis backend clear namespace bug

Open matanper opened this issue 11 months ago • 1 comments

Hey, I went over the code of RedisBackend and I've seen the usage of lua script to delete dynamic keys:

if namespace:
    lua = f"for i, name in ipairs(redis.call('KEYS', '{namespace}:*')) do redis.call('DEL', name); end"
    return await self.redis.eval(lua, numkeys=0)

According to redis docs this is discouraged:

Important: to ensure the correct execution of scripts, both in standalone and clustered deployments, all names of keys that a script accesses must be explicitly provided as input key arguments. The script should only access keys whose names are given as input arguments. Scripts should never access keys with programmatically-generated names or based on the contents of data structures stored in the database.

Even if we ignore this warning, for clustered deployments the keys might be partitioned in different nodes, which will raise error, so at least we should change the default key builder to wrap the prefix inside curly brackets:

Hash tags are documented in the Redis Cluster specification, but the gist is that if there is a substring between {} brackets in a key, only what is inside the string is hashed

For this reason I suggest to move the default_key_builder to be part of the backend instead of directly on FastAPICache

As for the lua script it can be replaced with scanning:

async def clear(self, namespace: Optional[str] = None, key: Optional[str] = None) -> int:
    if namespace:
        cursor = 0
        removed = 0
        while True:
            cursor, keys = await self.redis.scan(cursor, match=f'{namespace}:*', count=500)
            removed += await self.redis.delete(*keys)
            if cursor == 0:
                return removed
    elif key:
        return await self.redis.delete(key)
    return 0

matanper avatar Jul 17 '23 07:07 matanper