Skip to content

Commit

Permalink
Fix MemcachedClient Get() function (#500)
Browse files Browse the repository at this point in the history
* Fix MemcacheClient get function

When there was a cache miss after Get() the item was nil and we were
using its key attribute to wrap an error with more information.

This was wrong and now the error is using the input key string.

Also added a unit test for this scenario.

* fix go imports
  • Loading branch information
jesusvazquez committed Nov 16, 2021
1 parent c28eb2a commit c61b0bf
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/chunk/cache/memcached_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (c *MemcachedClient) Delete(key string) error {
func (c *MemcachedClient) Get(key string) (*memcache.Item, error) {
item, err := c.client.Get(key)
if err != nil {
return nil, c.wrapErrWithServer(item.Key, err)
return nil, c.wrapErrWithServer(key, err)
}
return item, nil
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/chunk/cache/memcached_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ package cache_test

import (
"sync"
"testing"
"time"

"github.com/bradfitz/gomemcache/memcache"
"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"

"github.com/grafana/mimir/pkg/chunk/cache"
)

type mockMemcachedBasicClient struct {
Expand Down Expand Up @@ -42,3 +49,18 @@ func (m *mockMemcachedBasicClient) Set(item *memcache.Item) error {
m.contents[item.Key] = item.Value
return nil
}

func TestMemcachedClient_Get_Error(t *testing.T) {
m := cache.NewMemcachedClient(
cache.MemcachedClientConfig{
UpdateInterval: time.Second,
},
"test",
prometheus.NewRegistry(),
log.NewNopLogger(),
)
t.Cleanup(m.Stop)
item, err := m.Get("foo")
assert.Error(t, err)
assert.Nil(t, item)
}

0 comments on commit c61b0bf

Please sign in to comment.