From c9cd14b50b582b909b3135758091967dd61b6008 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 20 Dec 2019 17:04:19 +0530 Subject: [PATCH] MM-21331: Add aggregate cache metrics for channelByNameCache (#13440) The `channelByNameCache` in the `GetByNames` query checks each teamId+channelName combination in the cache and if any of the keys is not present, it queries the DB. In this case, this is effectively a cache miss from the point of view of user. But since the cache is at a teamId+channelName level, there may be several hits and some misses. This is misleading as it does not generate proper metrics and might lead to a false assumption of how effective the cache is. We create separate metrics which account for all or none cache hits/misses. This removes the old metrics which are used by other queries. So they are now separated, and it will improve both the old and new metric. Because the old metric is not contaminated, and the new metric is also separate. --- store/sqlstore/channel_store.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 4839fa0ca5..7d694eed29 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1107,14 +1107,8 @@ func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCach } visited[name] = struct{}{} if cacheItem, ok := channelByNameCache.Get(teamId + name); ok { - if s.metrics != nil { - s.metrics.IncrementMemCacheHitCounter("Channel By Name") - } channels = append(channels, cacheItem.(*model.Channel)) } else { - if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Channel By Name") - } misses = append(misses, name) } } @@ -1146,6 +1140,15 @@ func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCach channelByNameCache.AddWithExpiresInSecs(teamId+channel.Name, channel, CHANNEL_CACHE_SEC) channels = append(channels, channel) } + // Not all channels are in cache. Increment aggregate miss counter. + if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter("Channel By Name - Aggregate") + } + } else { + // All of the channel names are in cache. Increment aggregate hit counter. + if s.metrics != nil { + s.metrics.IncrementMemCacheHitCounter("Channel By Name - Aggregate") + } } return channels, nil