App-vNext / Polly

Polly is a .NET resilience and transient-fault-handling library that allows developers to express policies such as Retry, Circuit Breaker, Timeout, Bulkhead Isolation, and Fallback in a fluent and thread-safe manner. From version 6.0.1, Polly targets .NET Standard 1.1 and 2.0+.
https://www.thepollyproject.org
BSD 3-Clause "New" or "Revised" License
13.44k stars 1.23k forks source link

Wrap cache policy in retry for transient error handling of errors thrown by cache #760

Closed asasine closed 4 years ago

asasine commented 4 years ago

Summary: What are you wanting to achieve? I'd like to wrap my calls to a cache policy in a retry policy such that any failed cache gets/puts will be retried per the policy. When using a distributed cache such as Redis, transient errors may occur such as timeouts or connection exceptions. These exceptions do not always indicate that the cache is unavailable and retries may often succeed. Wrapping the cache policy in a retry could help improve cache hits.

How can I achieve this using Polly's cache, retry, and wrap policies?



What code or approach do you have so far?
I've tried to wrap a cache policy with a retry policy that handles transient errors however this does not work. After reading through the documentation, it appears this may be by-design:

No exceptions due to caching operations are thrown. If the underlying cacheProvider throws an exception during a cache operation... the execution continues.

From https://github.com/App-vNext/Polly/wiki/Cache#throws

Using Microsoft.Extensions.DependencyInjection and other related packages, I'm registring policies into the policy registry:

services.AddSingleton<IReadOnlyPolicyRegistry<string>, PolicyRegistry>(serviceProvider =>
{
    var registry = new PolicyRegistry();

    static string typedCacheKeyStrategy<T>(Context context)
        => $"{typeof(T).FullName}-{context.OperationKey}";

    var cacheProvider = serviceProvider.GetRequiredService<MemoryCacheProvider>().AsyncFor<MyResult>();
    var memoryCachePolicy = Policy.CacheAsync(
        cacheProvider: cacheProvider,
        ttl: TimeSpan.FromMinutes(5),
        cacheKeyStrategy: typedCacheKeyStrategy<MyResult>);

    var delay = Backoff.DecorrelatedJitterBackoffV2(medianFirstRetryDelay: TimeSpan.FromSeconds(1), retryCount: 3);
    var distributedCacheTransientErrorRetryPolicy = Policy
        .Handle<RedisConnectionException>()
        .Or<RedisTimeoutException>()
        .WaitAndRetryAsync(
            delay,
            onRetry: (exception, delay, context) =>
            {
                var logger = context["logger"] as ILogger;
                logger?.LogWarning("Retrying {key} with delay {delay}", context.OperationKey, delay);
            })
        .AsAsyncPolicy<MyResult>();

    var serializerSettings = new JsonSerializerSettings();
    var serializer = new JsonSerializer<MyResult>(serializerSettings);
    var distributedCache = serviceProvider
        .GetRequiredService<IDistributedCache>()
        .AsAsyncCacheProvider<string>()
        .WithSerializer<MyResult, string>(serializer);

    // calls to distributed cache should handle and retry transient errors
    var distributedCachePolicy = Policy.WrapAsync(
        distributedCacheTransientErrorRetryPolicy,
        Policy.CacheAsync(
            cacheProvider: distributedCache,
            ttl: TimeSpan.FromDays(7),
            cacheKeyStrategy: typedCacheKeyStrategy<MyResult>,
            onCacheGet: (context, key) => { },
            onCacheMiss: (context, key) => { },
            onCachePut: (context, key) => { },
            onCacheGetError: (context, key, exception) =>
            {
                var logger = context["logger"] as ILogger;
                logger?.LogWarning("Failed to get key {key}: {ex}", key, exception);
            },
            onCachePutError: (context, key, exception) =>
            {
                var logger = context["logger"] as ILogger;
                logger?.LogWarning("Failed to put key {key}: {ex}", key, exception);
            }));

        // check memory cache, then check distributed cache
        var fullCachePolicy = Policy.WrapAsync(
            memoryCachePolicy,
            distributedCachePolicy);

    registry.Add("myPolicy", fullCachePolicy);
    return registry;
});

Then I'm getting this policy and using it as such:

public class MyClass
{
    private readonly IAsyncPolicy<MyResult> myResultPolicy;
    private readonly ILogger logger;

    public MyClass(IReadOnlyPolicyRegistry<string> registry, ILogger<MyClass> logger)
    {
        this.myResultPolicy = registry.Get<IAsyncPolicy<MyResult>>("myPolicy");
        this.logger = logger;
    }

    public async Task<MyResult> GetMyResult(string id)
    {
        var context = new Context(id)
        {
            ["logger"] = this.logger,
        };

        return await this.myResultPolicy.ExecuteAsync(func, context);

        async Task<RatingsResult> getRatingsAsync(Context context)
        {
            var id = context.OperationKey;
            this.logger.LogDebug("Getting my results for {id} from underlying service", id);
            var result = await ... ; // do http stuff
            return result;
        }
    }
}

To test this, I stopped the local redis process that the distributed cache was connecting to then executed the "myPolicy" policy. This forces every connection attempt to throw a RedisConnectionException which is logged in the onCacheGetError and onCachePutError delegates. I never see the onRetry delegate log anything though.

reisenberger commented 4 years ago

As you say, the cache engine intentionally catches caching exceptions and passes them to the onCacheGetError (etc) delegates for custom handling. You can do what you like in those handler delegates, though. To achieve your goal, you could simply code the onCacheGetError and onCachePutError delegates to throw exceptions which the wrapping retry policy handles. The delegates could just rethrow the exception passed as an input parameter, if desired.

onCacheGetError: (context, key, exception) =>
        {
            var logger = context["logger"] as ILogger;
            logger?.LogWarning("Failed to get key {key}: {ex}", key, exception);
            throw exception; // or using ExceptionDispatchInfo, if you prefer
        }
reisenberger commented 4 years ago

Note also an alternative approach to this problem is to place the retry-for-cache-errors policy within a custom cache provider implementation, as discussed and exemplified here.

asasine commented 4 years ago

I think the PolicyGuardedAsyncCacheProvider<TResult> implementation you suggest in your link is smart and provides a clean solution, one that is much more intentful than rethrowing exceptions in onCacheGetError. I'll try that implementation for my use case, thanks!