Cache data using Redis Cache in an ASP.NET Core 3.1 project

This is an unanswered code review request of mine from Code Review Stack Exchange.

Overview

I have developed a small ASP.NET Core 3.1 Web API that provides information that is rarely changed (several times a day) but often read (~ 10 K / day).

For me, it is a good opportunity to toy with Redis Cache and the Stack Exchange client.

I want to achieve the following:

• allow services to transparently get and set data through cache: just provide a key, a function to construct the data if not in cache and an expiration period
• should be thread-safe

The code

RedisCacheService.cs - the generic service that allows other services to transparently use the Redis cache

public class RedisCacheService : IRedisCacheService
{
    private ConnectionMultiplexer Redis = null;
    private IDatabase Database = null;

    private ILoggingService Logger { get; }

    private static readonly object SyncLock = new object();

    public RedisCacheService(ILoggingService loggingService)
    {
        Logger = loggingService;
    }

    /// <summary>
    /// checks that database is initialized. If not it is initialized
    /// </summary>
    private void EnsureConnection()
    {
        if (Database == null)
        {
            lock (SyncLock)
            {
                if (Database == null)
                {
                    //TODO: use appsettings 
                    Redis = ConnectionMultiplexer.Connect("localhost");
                    Database = Redis.GetDatabase();
                }
            }
        }
    }

    [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2007:Consider calling ConfigureAwait on the awaited task", Justification = "<Pending>")]
    [System.Diagnostics.CodeAnalysis.SuppressMessage("Globalization", "CA1303:Do not pass literals as localized parameters", Justification = "<Pending>")]
    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "<Pending>")]
    public async Task<T> GetInfoThroughCache<T>(string key, int expirationSeconds, Func<T> computeData)
    {
        if (computeData == null)
            throw new ArgumentException($"{nameof(computeData)} is null");

        string cachedValue;
        try
        {
            EnsureConnection();

            // checking the cache (no locking yet)
            cachedValue = await Database.StringGetAsync(key);
        }
        catch(Exception exc )
        {
            Logger.LogError(exc, $"Error getting cached value for key {key}");
            return computeData();
        }

        if (string.IsNullOrEmpty(cachedValue) || cachedValue == "nil")
        {
            lock(SyncLock)
            {
                T newData = computeData();
                string serializedData = JsonConvert.SerializeObject(newData);
                TimeSpan expires = new TimeSpan(0, 0, expirationSeconds);

                try
                {
                    bool set = Database.StringSet(key, serializedData, expires);
                }
                catch(Exception exc)
                {
                    Logger.LogError(exc, $"Error setting cached value for key {key}");
                    return computeData();
                }

                return newData;
            }
        }

        // information is cached
        try
        {
            T cacheData = JsonConvert.DeserializeObject<T>(cachedValue);
            return cacheData;
        }
        catch(Exception exc)
        {
            Logger.LogError(exc, $"Error deserializing  cached value for key {key}");
            throw;
        }
    }
}

Usage example

var questionInfo = await RedisCacheService.GetInfoThroughCache($"question_{questionId}", QuestionFullInfoCacheExpiration, () =>
    {
        var questionInfo = GetQuestionHeaderInfo(questionId);
        if (questionInfo == null)
            throw new ArgumentException($"No question info for id = {questionId}");

        GetQuestionFullInfo_QuestionComments(questionInfo);
        GetQuestionFullInfo_Answers(questionInfo);

        return questionInfo;
    });

This seems to work fine (caching spares significant database processing time), although I have not performed any parallel testing (multiple threads accessing this functionality).

Next step would be to try a faster serializer (Protobuf?).

Any thoughts? I am interested mostly in code design rather than performance aspects.

posted about 4 years ago

CC BY-NC-SA 4.0

Alexei‭

5003 reputation 112 101 690 486