Polly icon indicating copy to clipboard operation
Polly copied to clipboard

How to Invalidate a Cache item using Polly [Add Remove and RemoveAsync to cache provider interfaces]

Open jaumemilian opened this issue 5 years ago • 14 comments

We are using Cache system with Polly in our NetCore solution and we can't find the way to invalidate specific cache registries.

When using IMemoryCache from NetCore we were able to do it using CancellationTokenSource

This is an example about what we are doing to Cache an HttpClient Request using Polly:

var policyExecutionContext = new Context("myCacheEntryKey");
var response = await _cachePolicy.ExecuteAsync(
(context) => httpClient.GetAsync(uri), policyExecutionContext);

jaumemilian avatar Jun 27 '19 16:06 jaumemilian

Hi @jaumemilian . Polly does not provide a specific API for evicting cache entries. So far, it takes the view that, when you created the cache Policy, you had access to the underlying cache (IMemoryCache or IDistributedCache), and you can use this directly to invalidate specific cache entries.


We could happily add Remove(string) and Remove(string, CancellationToken) overloads to the ISyncCacheProvider and IAsyncCacheProvider interfaces respectively.

Then, CachePolicy (and its variants) could also have methods Remove(string) and RemoveAsync(string, CancellationToken) which could chain on to those.

We would also have to update the two cache providers from the App-vNext team, to match those changes: Polly.Caching.MemoryCache and Polly.Caching.IDistributedCache.

Marking this up-for-grabs, as it's a great first issue for anyone to take on! (I am focusing on pushing out Distributed Circuit Breaker at the moment.)

reisenberger avatar Jun 27 '19 20:06 reisenberger

Hi, I would like to work on this issue. Can you please give me some more insights on how I should proceed with it ?

komalg1 avatar Sep 16 '19 04:09 komalg1

@komalg1 Thanks! I'll aim to add some more notes in the next couple of days/as soon as I can get time.

reisenberger avatar Sep 17 '19 12:09 reisenberger

Thanks again @komalg1 for offering to contribute!

First, see our general notes on Git workflow and configuring line endings.

If this is your first open source contribution, google can return lots of general guidance on beginning contributing to OSS - but do fire away with any specific questions.

I'm not sure how familiar you are with Polly, so if any of the below is too obvious, bear with me.

The below looks to me like a route through what's needed, but if something isn't making sense, feel free to ask more questions; or if it doesn't look like a good first issue, again, feel free to say.

Start

  • Understand the respective roles of CachePolicy and its CacheProviders. CachePolicy contains the cache-aside logic - try to get the result from cache; if result was retrieved from underlying system instead, store it in the cache. CacheProviders are different actual cache implementations (memory cache; distributed cache etc), and not stored in this repo.
  • Find ISyncCacheProvider and IAsyncCacheProvider in the codebase
  • In your branch (see Gitflow above!), add void Remove(string) overloads to the ISyncCacheProvider and ISyncCacheProvider<TResult> interfaces; and Task RemoveAsync(string, CancellationToken) to the similar IAsyncCacheProvider interfaces.

At this point the Polly project (but not Polly.Specs) should compile.

Make cache policy classes have Remove methods chaining on to cache provider Remove methods

  • Create IAsyncCachePolicy and IAsyncCachePolicy<TResult> interfaces (they're missing; follow the pattern for ISyncCachePolicy and ISyncCachePolicy<TResult>), and make AsyncCachePolicy : IAsyncCachePolicy and AsyncCachePolicy<TResult> : IAsyncCachePolicy<TResult>.
  • Add a void Remove(string) overload to ICachePolicy.
  • Add a Task Remove(string, CancellationToken) overload to IAsyncCachePolicy.
  • Fulfil those methods in CachePolicy, CachePolicy<TResult>, AsyncCachePolicy and AsyncCachePolicy<TResult> just by delegating to the relevant overloads on the cache provider which the policies store.

Again, at this point the Polly project (but not Polly.Specs) should compile.

Unit tests

  • Because the only action in CachePolicy implementations will be to delegate any Remove(...) or RemoveAsync(...) calls to the configured cache provider, it would be acceptable, for tests, to use Moq just to verify that the call is delegated in that way.
  • Implementations of the remove methods in StubCacheProvider will be needed for completeness. (This is an intentionally naive non-concurrency-safe in-memory cache provider implementation, which serves as something simple for tests while avoiding getting into the differences between MemoryCache in .Net Framework and .Net Core. For Remove: obvious, simple implementations will do.)
  • Implementations of the remove methods in StubErroringCacheProvider will be needed for completeness. They can just delegate to the wrapped StubCacheProvider. I guess I'm proposing that cache policies won't do any special handling of errors thrown by Remove calls - they'll just let any errors bubble outwards. (The policies do special handling of errors during get or put, because we elected that cache errors should not bring down the whole call through Polly.)

reisenberger avatar Sep 21 '19 19:09 reisenberger

@reisenberger thanks for the notes. Correct me if I am wrong but what I understood is that we want to add a functionality to remove a key from the cache if it is no more valid? I have started with the first part, as mentioned in the notes. I will soon provide more updates.

[edit] I took the code from branch v712-or-v720, I am not able to see 'ISyncCachePolicy' and 'ISyncCachePolicy<TResult>'. Are these interfaces already existing ?

komalg1 avatar Sep 22 '19 19:09 komalg1

Hi,

First of all, thanks for your offer to develop that.

Let me try to clarify what was my first request looking for:

In the IDistributedCache, there is already a mechanism to remove cache entries if you know the key of the entry you want to remove.

_cacheInstance.Remove(key)

What I was looking for in my question was a nice feature that is available for the IMemoryCache, where you can associate several cache entries to one "CancellationTokenSource", and just cancelling that CancellationTokenSource, all the cache entries associated are automatically removed from the cache https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.caching.memory.memorycacheentryextensions.addexpirationtoken?view=aspnetcore-2.2

We can't find that feature in the IDistriburedCache, and I just was wondering if could be Polly was offering on top of IDistributedCache something similar to that

Kind Regards, Jaume

jaumemilian avatar Sep 23 '19 06:09 jaumemilian

@jaumemilian Thanks for the extra detail. No, Polly does not provide that. (I would consider that an interesting separate project, but outside the scope of Polly.)

reisenberger avatar Sep 23 '19 07:09 reisenberger

Hi @komalg1

what I understood is that we want to add a functionality to remove a key from the cache if it is no more valid?

👍

I took the code from branch v712-or-v720,

perfect!

I am not able to see 'ISyncCachePolicy' and 'ISyncCachePolicy'. Are these interfaces already existing ?

Thanks for +1 :eyes: spotting what I missed! Those interfaces are not yet there; they would need adding. Looking at the patterns again, we would need:

ISyncCachePolicy : ICachePolicy
ISyncCachePolicy<TResult> : ISyncCachePolicy
IAsyncCachePolicy : ICachePolicy
IAsyncCachePolicy<TResult> : IAsyncCachePolicy

and then:

CachePolicy : ISyncCachePolicy 
CachePolicy<TResult> : ISyncCachePolicy<TResult>
AsyncCachePolicy : IAsyncCachePolicy
AsyncCachePolicy<TResult> : IAsyncCachePolicy<TResult>

Thus: ICachePolicy is just a marker interface for "this is a cache policy" (it might in future contain something that is common to cache policies whether sync or async). Then, the new Remove(string) overload goes on ISyncCachePolicy (not ICachePolicy as I originally said).

Thanks for spotting this; let me know if anything else doesn't seem right!

reisenberger avatar Sep 23 '19 07:09 reisenberger

The extra interfaces mentioned in the above comment have now been added, for Polly v8.0.0, by #739, in the v8.0.0 branch.

If anyone continues to pick up this issue as an up-for-grabs, build on the v8.0.0 (or later) branch. Thanks!

reisenberger avatar Dec 30 '19 13:12 reisenberger

Hi, I would like to pick this ticket up if no one is currently working on it. Thanks.

hana26 avatar Jan 21 '20 13:01 hana26

Thanks @hana26 ! Unless @komalg1 comes back on this in the next day or two (or has any existing progress to share?), I would say yes, @hana26 , please do go ahead. Thanks!

reisenberger avatar Jan 21 '20 18:01 reisenberger

We now have a PR #745 fulfilling this, so it is no longer up-for-grabs.

reisenberger avatar Jan 24 '20 18:01 reisenberger

When Polly v8.0.0 is released, the following cache providers will need to be updated to the new interfaces in #745 (ie add trivial .Remove() and .RemoveAsync() pass-through implementations):

  • https://github.com/App-vNext/Polly.Caching.IDistributedCache/issues/20
  • https://github.com/App-vNext/Polly.Caching.MemoryCache/issues/40

reisenberger avatar Jan 24 '20 19:01 reisenberger

It would be nice if there were a delegate or event to subscribe to for evicting entries. This would allow you to implement the Observer Design Pattern to send events that the Polly Policy could subscribe to for cache eviction.

Or, even a custom attribute that could be added to a method that would cause cache entries to be evicted upon that method being called.

@dreisenberger Any suggestions for implementing this before my aspirations mentioned above are considered?

udlose avatar Dec 30 '20 23:12 udlose

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

github-actions[bot] avatar Aug 16 '23 01:08 github-actions[bot]

@reisenberger bump

udlose avatar Aug 16 '23 03:08 udlose

This issue, as described, is not going to be part of Polly v8. A future version may invest in a new caching policy that builds on top of the MS cache implementation, which is linked to the issue to consider this use case if and when such work happens.

martincostello avatar Aug 16 '23 05:08 martincostello