-
Notifications
You must be signed in to change notification settings - Fork 28
feat: Rework tenant cache #1601
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
base: main
Are you sure you want to change the base?
Conversation
In order to have more resilience in the face of outages, implement an out-of-process cache using memcached. In order to support the existing mode of operation, add an in-process-cache using Otter. Rework the tenant cache to use this new system. Signed-off-by: Marcelo E. Magallon <marcelo.magallon@grafana.com>
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.
This is the bit that needs the most attention.
| // | ||
| // This interface allows the agent to use different cache backends interchangeably, | ||
| // including a no-op implementation that eliminates the need for nil checks in client code. | ||
| type Cache interface { |
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.
There are three implementations of this interface.
A memcached one. This is the one I would like to use eventually.
A local one, based on Otter.
A noop one, which is a fallback in case the other two are not available.
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.
I did consider a multi-tiered implementation, too, using L1 with Otter and L2 with memcached. I would prefer to get some numbers before going that way.
| type Cache interface { | ||
| // Set stores a value in the cache with the specified expiration time. | ||
| // Returns an error if the operation fails. | ||
| Set(ctx context.Context, key string, value any, expiration time.Duration) error |
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.
This is the bit where I struggled the most.
Otter uses generics to have a more strongly-typed cache. Since the goal is to use memcached, which does not have a strongly-typed API, I thought about multiple clients, one per type, but it was weird.
|
Apologies about the large PR. I did consider breaking it up into multiple steps, but it's hard to see where this is going without the changes in the tenant manager code, and it's hard to make the tenant manager changes without the other code. |
In order to have more resilience in the face of outages, implement an out-of-process cache using memcached. In order to support the existing mode of operation, add an in-process-cache using Otter. Rework the tenant cache to use this new system.