V13: Clear Member Username Cache in Load Balanced Environments#19191
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the issue of stale member username cache entries in load balanced environments by adding an extra cache clearance in the MemberCacheRefresher.
- Introduces a new static field "UserNameCachePrefix" to support clearing a specific cache key.
- Changes the cache-clearing condition to skip cache clearing when memberCache.Success is false.
- Adds detailed inline comments explaining the rationale for the hacky cache key handling.
Comments suppressed due to low confidence (1)
src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs:15
- Consider declaring the UserNameCachePrefix field as 'readonly' since its value is constant throughout runtime to prevent unintended modifications.
private static string UserNameCachePrefix = "uRepo_userNameKey+";
src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs
Outdated
Show resolved
Hide resolved
…er.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
AndyButland
left a comment
There was a problem hiding this comment.
Looks fine to me as a fix. Maybe you could consider moving the UserNameCachePrefix constants somewhere it can be used in MemberRepository.cs too? But you might also be happy just to do that when the clean up you reference is tackled.
I'll leave it to you and decide to merge with or without an update there.
|
Excellent point, thanks, I've moved it to the |
* Clear usernamekey * Odd explaining comment * Update src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Make UserNameCachePrefix readonly for better immutabilityly * Move prefix to CacheKeys constants --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> # Conflicts: # src/Umbraco.Core/Cache/CacheKeys.cs
* Clear usernamekey * Odd explaining comment * Update src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Make UserNameCachePrefix readonly for better immutabilityly * Move prefix to CacheKeys constants --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> # Conflicts: # src/Umbraco.Core/Cache/CacheKeys.cs
Fixes #18781
The issue was that after #17350 the member username key wasn't evicted from the cache.
To fix this, I've for now added an extra line clearing this in the
MemberCacheRefresherI think a separate task is warranted to align how we generate these cache keys. Currently, each repository generates this cache key internally, which is why this "extension" of the member cache is added as
MemberRepositoryUsernameCachePolicy; however, at the same time, we haveRepositoryCacheKeys, which is used to generate the cache keys when clearing the cache in cache refreshers.RepositoryCacheKeysalso has an optimization to avoid allocation of strings, however, we're missing out on this when populating the caches. I think we should use something likeRepositoryCacheKeysboth places, however, make this extendable on an pr. entity basis, remove the need for theMemberRepositoryUsernameCachePolicybecause it's kind of hacky 😄 I'll get a task made on the board 🫡Testing
See original issue, however I've already tested this on a load balanced setup, and the logic is quite straightforward.
This should be cherry picked up to 15+