Skip to content

Improve reusability of the cache invalidation counter (#66310)#71802

Merged
ywangd merged 1 commit intoelastic:7.xfrom
ywangd:cache-clearing-refactor-privilege-store-7.x
Apr 19, 2021
Merged

Improve reusability of the cache invalidation counter (#66310)#71802
ywangd merged 1 commit intoelastic:7.xfrom
ywangd:cache-clearing-refactor-privilege-store-7.x

Conversation

@ywangd
Copy link
Copy Markdown
Member

@ywangd ywangd commented Apr 19, 2021

This PR is a follow-up refactoring for #59376. It replaces
InvalidationCountingCacheWrapper with a simpler and more resuable
LockingAtomicCounter.

The change includes extracting the logic of "minimizing chance of caching stale
results" into a new class, which is now generic and not tied to any cache
related operations. This in turn allows it to be reused by different caches,
which often have subtle, important and anonying differences that make it
impossible (or at least cubersome and painful) to have a generic solution. So
this change of reducing the size of reusable code is the right move.

As an example of applying this new pattern, the caches for NativePrivilegeStore
are now migrated and managed by the centrialied CacheInvalidatorRegistry. The
clear privilege cache rest and transport actions are also updated to use the
new implementation. The Request/Response classes are intentionally untouched
and plan to have a separate PR to address them because they would involve user
visible deprecation changes.

Co-authored-by: Albert Zaharovits albert.zaharovits@elastic.co

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS and architecture that we support?
  • If you are submitting this code for a class then read our policy for that.

This PR is a follow-up refactoring for elastic#59376. It replaces
InvalidationCountingCacheWrapper with a simpler and more resuable
LockingAtomicCounter.

The change includes extracting the logic of "minimizing chance of caching stale
results" into a new class, which is now generic and not tied to any cache
related operations. This in turn allows it to be reused by different caches,
which often have subtle, important and anonying differences that make it
impossible (or at least cubersome and painful) to have a generic solution. So
this change of reducing the size of reusable code is the right move.

As an example of applying this new pattern, the caches for NativePrivilegeStore
are now migrated and managed by the centrialied CacheInvalidatorRegistry. The
clear privilege cache rest and transport actions are also updated to use the
new implementation. The Request/Response classes are intentionally untouched
and plan to have a separate PR to address them because they would involve user
visible deprecation changes.

Co-authored-by: Albert Zaharovits <albert.zaharovits@elastic.co>
@ywangd ywangd merged commit 8076001 into elastic:7.x Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant