-
Notifications
You must be signed in to change notification settings - Fork 2.8k
V16: Cache Version Mechanism #19747
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
V16: Cache Version Mechanism #19747
Conversation
This is why we use DI folks. 🤦
@nikolajlauridsen if I read it correctly it will check on each request if cache is up to date? I think it would be better to have running background task to do it, as it will not add latency to requests or at least making it switchable from on request to background refresh? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great so far. From visual inspection have made a few comments (and nit-picks) that you'll see below.
From testing locally I can see the migration works as expected and the cache table is getting populated as I make changes to the Umbraco entities. So all looks to be working as expected.
Reviewing got me thinking about the packages story. We'd need to have a way for say Forms or Commerce to be able to opt in to the same approach for repository cache refreshing (Forms uses Umbraco repository base classes, but that's not required - Commerce has it's own data layer). So we should look to prepare documentation for this.
@bielu - I'll let @nikolajlauridsen answer you properly when he's back from holiday, but my understanding is that yes we do need to do this on each request, such that if not the cache can be fast-forwarded such that it is up to date. This is part of an effort to support load balancing the backoffice, and hence it's necessary that caches are up to date before they are relied on.
src/Umbraco.Infrastructure/Cache/DefaultRepositoryCachePolicy.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Cache/MemberRepositoryUsernameCachePolicy.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepositoryBase.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs
Show resolved
Hide resolved
Co-authored-by: Andy Butland <[email protected]>
Thanks for the review Andy, it's appreciated. In regard to the packages thing, that's a good point, if you're using our repositories it'll just happen "automagically". But for packages with their own data layer we need documentation, additionally we might want to extract |
@bielu, thanks for your concern. This is actually how this currently works using cache refreshers, but if load balancing the backoffice this causes issues because it's too slow, some servers might return out-of-date items. Take, for instance,e this scenario:
As you mentioned, this will cause some extra latency; however, the goal is to make this only affect the backoffice, and not your website perfomance. I'm aware that we'll need to handle templates, public access, and members slightly differently to keep this off the frontend. This will also be switchable using the |
it will add significant latency, as it will happen on each operation of getting something from cache if Understand correctly pr? |
It is worth noting that this is not the published cache, this is the repositories caches, so access to the published caches is unaffected. |
b52461d
into
v16/feature/load-balancing-isolated-caches
This pull request introduces a new cache versioning mechanism for repositories, helping to ensure cache consistency with the database in multi-server scenarios. This is the first part of an effort to load balance the backoffice, and by extension, the isolated caches. All this PR does is create a mechanism for us to know if the cache is out of sync; the next part is to be able to synchronize the cache.
Since this can be reviewed as a discrete unit, I've opted to make it its own PR and target a long-lived feature branch.
Testing
Navigate around the backoffice and see that the new cache version table gets created and updated, also try and manually change the version guide to see that the cache is registered as invalid