diff --git a/pkg/cacheutil/memcached_client.go b/pkg/cacheutil/memcached_client.go index 3002b847278..3e7795924c1 100644 --- a/pkg/cacheutil/memcached_client.go +++ b/pkg/cacheutil/memcached_client.go @@ -3,6 +3,7 @@ package cacheutil import ( "context" "errors" + "fmt" "sync" "time" @@ -219,11 +220,15 @@ func (c *memcachedClient) Stop() { func (c *memcachedClient) SetAsync(key string, value []byte, ttl time.Duration) error { return c.enqueueAsync(func() { - c.client.Set(&memcache.Item{ + err := c.client.Set(&memcache.Item{ Key: key, Value: value, Expiration: int32(time.Now().Add(ttl).Unix()), }) + + if err != nil { + level.Warn(c.logger).Log("msg", fmt.Sprintf("failed to store item with key %s to memcached", key), "err", err) + } }) } diff --git a/pkg/cacheutil/memcached_client_test.go b/pkg/cacheutil/memcached_client_test.go index 03b9ab3e2c4..7aa9b7ad257 100644 --- a/pkg/cacheutil/memcached_client_test.go +++ b/pkg/cacheutil/memcached_client_test.go @@ -59,8 +59,11 @@ func TestMemcachedClient_SetAsync(t *testing.T) { testutil.Ok(t, err) defer client.Stop() - client.SetAsync("key-1", []byte("value-1"), time.Second) - client.SetAsync("key-2", []byte("value-2"), time.Second) + err = client.SetAsync("key-1", []byte("value-1"), time.Second) + testutil.Ok(t, err) + + err = client.SetAsync("key-2", []byte("value-2"), time.Second) + testutil.Ok(t, err) err = backendMock.waitItems(2) testutil.Ok(t, err) @@ -200,7 +203,8 @@ func TestMemcachedClient_GetMulti(t *testing.T) { // Populate memcached with the initial items. for _, item := range testData.initialItems { - client.SetAsync(item.Key, item.Value, time.Second) + err := client.SetAsync(item.Key, item.Value, time.Second) + testutil.Ok(t, err) } // Wait until initial items have been added. @@ -212,7 +216,7 @@ func TestMemcachedClient_GetMulti(t *testing.T) { testutil.Equals(t, testData.expectedHits, hits) testutil.Equals(t, testData.expectedErr, err) - // Ensure the client has interected with the backend as expected. + // Ensure the client has interacted with the backend as expected. backendMock.lock.Lock() defer backendMock.lock.Unlock() testutil.Equals(t, testData.expectedGetMultiCount, backendMock.getMultiCount) @@ -251,7 +255,7 @@ func (c *memcachedClientBackendMock) GetMulti(keys []string) (map[string]*memcac return nil, errors.New("mocked GetMulti error") } - items := make(map[string]*memcache.Item, 0) + items := make(map[string]*memcache.Item) for _, key := range keys { if item, ok := c.items[key]; ok { items[key] = item diff --git a/pkg/cacheutil/memcached_server_selector.go b/pkg/cacheutil/memcached_server_selector.go index c2f1b49d021..385a936e43b 100644 --- a/pkg/cacheutil/memcached_server_selector.go +++ b/pkg/cacheutil/memcached_server_selector.go @@ -12,7 +12,8 @@ import ( var ( addrsPool = sync.Pool{ New: func() interface{} { - return make([]net.Addr, 0, 64) + addrs := make([]net.Addr, 0, 64) + return &addrs }, } ) @@ -57,27 +58,34 @@ func (s *MemcachedJumpHashSelector) SetServers(servers ...string) error { func (s *MemcachedJumpHashSelector) PickServer(key string) (net.Addr, error) { // Unfortunately we can't read the list of server addresses from // the original implementation, so we use Each() to fetch all of them. - addrs := addrsPool.Get().([]net.Addr) - s.servers.Each(func(addr net.Addr) error { + addrs := *(addrsPool.Get().(*[]net.Addr)) + err := s.servers.Each(func(addr net.Addr) error { addrs = append(addrs, addr) return nil }) + if err != nil { + return nil, err + } + // No need of a jump hash in case of 0 or 1 servers. if len(addrs) == 0 { - addrsPool.Put(addrs[:0]) + addrs = (addrs)[:0] + addrsPool.Put(&addrs) return nil, memcache.ErrNoServers } else if len(addrs) == 1 { - addrsPool.Put(addrs[:0]) - return addrs[0], nil + addrs = (addrs)[:0] + addrsPool.Put(&addrs) + return (addrs)[0], nil } // Pick a server using the jump hash. cs := xxhash.Sum64String(key) idx := JumpHash(cs, len(addrs)) - picked := addrs[idx] + picked := (addrs)[idx] - addrsPool.Put(addrs[:0]) + addrs = (addrs)[:0] + addrsPool.Put(&addrs) return picked, nil } diff --git a/pkg/cacheutil/memcached_server_selector_test.go b/pkg/cacheutil/memcached_server_selector_test.go index aef903f286d..55ef57fac11 100644 --- a/pkg/cacheutil/memcached_server_selector_test.go +++ b/pkg/cacheutil/memcached_server_selector_test.go @@ -60,11 +60,12 @@ func TestMemcachedJumpHashSelector_Each_ShouldRespectServersOrdering(t *testing. testutil.Ok(t, err) actual := make([]string, 0, 3) - s.Each(func(addr net.Addr) error { + err = s.Each(func(addr net.Addr) error { actual = append(actual, addr.String()) return nil }) + testutil.Ok(t, err) testutil.Equals(t, test.expected, actual) } } @@ -170,11 +171,17 @@ func BenchmarkMemcachedJumpHashSelector_PickServer(b *testing.B) { } selector := MemcachedJumpHashSelector{} - selector.SetServers(servers...) + err := selector.SetServers(servers...) + if err != nil { + b.Error(err) + } b.ResetTimer() for i := 0; i < b.N; i++ { - selector.PickServer(string(i)) + _, err := selector.PickServer(string(i)) + if err != nil { + b.Error(err) + } } }