Add metrics for morph caches #371

New issue

Closed

opened 2023-05-18 10:34:53 +00:00 by fyrchik · 2 comments

fyrchik commented

2023-05-18 10:34:53 +00:00

Owner

We cache many things that are taken from the blockchain.

Hit/miss metric.
Execution time on misses.

We cache many things that are taken from the blockchain. 1. Hit/miss metric. 2. Execution time on misses.

fyrchik added this to the v0.38.0 milestone 2023-05-18 10:34:53 +00:00

fyrchik added the

frostfs-node

good first issue

observability

labels 2023-05-18 10:34:53 +00:00

aarifullin was assigned by fyrchik

2023-05-24 08:46:12 +00:00

aarifullin referenced this issue

2023-05-26 09:17:06 +00:00

[#371] ir: Add morph cache metrics #400

aarifullin commented

2023-05-31 12:59:14 +00:00

Member

@fyrchik suggested to add metrics for these caches trying to somehow generelize cache metric interface.

I suppose this would look like that:

type CacheMetrics interface {
 AddGetCacheValue(success bool, d time.Duration)
 AddRemoveCacheKey(success bool, d time.Duration)
 AddSetCacheKeyValue(success bool, d time.Duration)
}

and each cache struct must have embedded CacheMetrics implementation, like that:

type ttlNetCache[K comparable, V any] struct {
	/*...*/
 cache *lru.Cache[K, *valueWithTime[V]]
	/*...*/
 metrics *ttlNetCacheMetrics
}

type lruNetmapSource struct {
 netState netmap.State

 cache *lruNetCache
    
 metrics *lruNetmapSourceMetrics
}

This also can be composited within WriteCacheMetrics

But this barely can be used for MorphClientCache, because its cache cannot explicitly implement CacheMetrics - cache is composed from three different caches and without refactoring the implementation will be like this anyway.

So, I suggest to introduce generalized CacheMetrics interface in the next issue and use it for ttlNetCache, lruNetmapSource and writeCacheMetrics and to keep metrics implementation for morphClientCache like in the PR

@fyrchik [suggested](https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/400#issuecomment-11324) to add metrics for [these](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/cmd/frostfs-node/cache.go) caches trying to somehow generelize cache metric interface. I suppose this would look like that: ``` type CacheMetrics interface { AddGetCacheValue(success bool, d time.Duration) AddRemoveCacheKey(success bool, d time.Duration) AddSetCacheKeyValue(success bool, d time.Duration) } ``` and each cache struct must have embedded `CacheMetrics` implementation, like that: ``` type ttlNetCache[K comparable, V any] struct { /*...*/ cache *lru.Cache[K, *valueWithTime[V]] /*...*/ metrics *ttlNetCacheMetrics } type lruNetmapSource struct { netState netmap.State cache *lruNetCache metrics *lruNetmapSourceMetrics } ``` This also can be composited within [WriteCacheMetrics](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/metrics/writecache.go#L24) But this barely can be used for [MorphClientCache](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/morph/client/client.go#L88), because its `cache` cannot explicitly implement `CacheMetrics` - `cache` is composed from three different caches and without refactoring the implementation will be like [this](https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/400/files) anyway. So, I suggest to introduce generalized `CacheMetrics` interface in the next issue and use it for `ttlNetCache`, `lruNetmapSource` and `writeCacheMetrics` and to keep metrics implementation for `morphClientCache` like in the PR

aarifullin referenced this issue

2023-05-31 13:00:30 +00:00

[#371] ir: Add morph cache metrics #400

fyrchik referenced this issue from a commit

2023-06-13 10:10:17 +00:00

[#371] ir: Add morph cache metrics

aarifullin closed this issue

2023-06-19 07:54:46 +00:00

aarifullin reopened this issue

2023-06-19 07:54:56 +00:00

aarifullin referenced this issue

2023-06-19 07:59:48 +00:00