Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jaumemilian opened this issue Jun 27, 2019 · 17 comments

Comments

@jaumemilian
Copy link

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);
@reisenberger
Copy link
Member

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 reisenberger changed the title How to Invalidate a Cache using Polly How to Invalidate a Cache using Polly [UP-FOR-GRABS: Add Remove and RemoveAsync to cache provider interfaces] Jun 27, 2019
@komalg1
Copy link

komalg1 commented Sep 16, 2019

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

@reisenberger
Copy link
Member

reisenberger commented Sep 17, 2019

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

@reisenberger
Copy link
Member

reisenberger commented Sep 21, 2019

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.)

@komalg1
Copy link

komalg1 commented Sep 22, 2019

@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'. Are these interfaces already existing ?

@jaumemilian
Copy link
Author

jaumemilian commented Sep 23, 2019

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

@reisenberger
Copy link
Member

@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
Copy link
Member

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 👀 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
Copy link
Member

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!

@hana26
Copy link

hana26 commented Jan 21, 2020

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

@reisenberger
Copy link
Member

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 reisenberger changed the title How to Invalidate a Cache using Polly [UP-FOR-GRABS: Add Remove and RemoveAsync to cache provider interfaces] How to Invalidate a Cache item using Polly [Add Remove and RemoveAsync to cache provider interfaces] Jan 24, 2020
@reisenberger
Copy link
Member

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

@reisenberger reisenberger added this to the v8.0.0 milestone Jan 24, 2020
@reisenberger
Copy link
Member

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):

@udlose
Copy link

udlose commented Dec 30, 2020

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?

@martincostello martincostello removed this from the v8.0.0 milestone Jun 16, 2023
@github-actions
Copy link
Contributor

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 github-actions bot added the stale Stale issues or pull requests label Aug 16, 2023
@udlose
Copy link

udlose commented Aug 16, 2023

@reisenberger bump

@martincostello
Copy link
Member

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 martincostello closed this as not planned Won't fix, can't repro, duplicate, stale Aug 16, 2023
@martincostello martincostello removed the stale Stale issues or pull requests label Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants