fix(HMS-1800): refactor and add more caching #534
Conversation
| AzureInfo *clients.AzureSourceDetail `json:"azure" nullable:"true"` | ||
| Provider string `json:"provider"` | ||
| AwsInfo *clients.AccountDetailsAWS `json:"aws" nullable:"true"` | ||
| AzureInfo *clients.AccountDetailsAzure `json:"azure" nullable:"true"` | ||
| } |
There was a problem hiding this comment.
Because the new cache key is based on generics and type infers the prefix, string build-in type cannot be used. So I introduced a special type here.
|
|
||
| type Cacheable interface { | ||
| models.Account | clients.AccountDetailsAWS | clients.AzureTenantId | ||
| } |
There was a problem hiding this comment.
To add a new generic type to cache, change here.
internal/cache/redis.go
Outdated
| default: | ||
| return "", ErrUnknownType | ||
| } | ||
| } |
There was a problem hiding this comment.
To add a new generic type to cache, change here.
| // nolint: wrapcheck | ||
| func Set[T Cacheable](ctx context.Context, key string, value *T) error { | ||
| return SetExpires(ctx, key, value, config.Application.Cache.Expiration) | ||
| } |
There was a problem hiding this comment.
The interface is simple:
- Find
- Set
- SetForever (with a very long expiration)
- SetExpires (with your own expiration duration)
I see I forgot to document these functions, will do during rebase.
internal/cache/redis.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func Prefix[T Cacheable](value *T) (string, error) { |
There was a problem hiding this comment.
Looks weird to use generics and then still switch by type.
Why is it better then having SetXXX functions? 🤔
EDIT: or interface Cacheable with CacheKey method?
There was a problem hiding this comment.
Yeah I rebased the code to use interface normally instead of constraint. This is indeed more natural and better, good point.
lzap
force-pushed
the
cache-hsacc
branch
9 times, most recently
from
d7d03ba
to
28e10db
Compare
|
Linter error solution: #537 |
Signed-off-by: Lukáš Zapletal <lzap+git@redhat.com>
This is complete refactoring of the caching package. Initially, there was a memory implementation, then Redis was added and also noop for tests - many interfaces and it was a bit over-engineered to my taste. Everything is now simplified: cache can be enabled (Redis) or disabled (no caching, the default). The API is vastly more simple with only few functions available and it makes use of Go generics as well as Gob binary encoding for seamless experience. It is now a type-safe cache that is easy to work with and adding a new model/type is just three lines change.
It started as adding support for upload info caching but the more I digged into the old design the more I wanted to refactor this from scratch. As part of one commit, this also adds caching for:
Finally, metrics were improved - alongside with hits and misses also errors are reported. I am going to create a new graph on the dashboard so we can take a look on cache success rate or error ratio. Maybe as part of this PR if I have chance.
Added also integration tests for Redis. No upgrade is needed, everytime a cache encounters a deserialization error, it is reported into logs and prometheus, but ErrNotFound is returned so the caller will effectively overwrite the old entry with new data.