Open richardvella opened 6 years ago
Hi @richardvella, The Put implementation was intended to run in fire and forget, and yes, errors could be silently discarded. But that is totally ok for many scenarios.
The advantage is higher throughput in writing Data to Redis compared to Add
or Update
If you want to be sure the operation completed (or throws), use Update
to change the value of an existing item, or Add
to cache a new item.
I agree that this is not very obvious and probably not the best design ever. A configuration point could be an option, to disable fire and forget mode.
Hi @MichaCo,
Thanks for the quick reply and for building such a great library :)
My only concern on this is that if for example you run you app and do a Put in a loop inserting a key on each iteration and then execute script flush on redis (which happens on a server restart) no keys will ever get added to redis. I see this as a very big issue. Ideally there should be some setting that changes the Put operation to run in sync mode as suggested.
I was doing some debugging and noticed that the redis put command uses a lua script which runs in fire and forget mode:
https://github.com/MichaCo/CacheManager/blob/dev/src/CacheManager.StackExchange.Redis/RedisCacheHandle.cs#L637
Since lua scripts are preloaded into redis, if a restart occurs or a call to script flush is issued these scripts essentially get wiped out from the redis cache. In sync mode this is not an issue as an exception is thrown and a NOSCRIPT message is sent from redis. This error is correctly caught and handled:
https://github.com/MichaCo/CacheManager/blob/dev/src/CacheManager.StackExchange.Redis/RedisCacheHandle.cs#L1007
However when the script runs using CommandFlags.FireAndForget that exception is never thrown and all Put commands essentially fail silently until we request data from the remote cache using a get.
Should PutInternal be converted to run in sync mode?