From 8ef322d76d1c5f01fbc8dcb40feb4ae716e3dcb9 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Thu, 9 Jan 2020 21:39:23 +0100 Subject: [PATCH] Fixed crash in the memcached servers selector when there's only 1 server (#1975) * Fixed crash in the memcached servers selector when there's only 1 server Signed-off-by: Marco Pracucci * Updated changelog Signed-off-by: Marco Pracucci * Assert expected error in tests Signed-off-by: Marco Pracucci --- CHANGELOG.md | 3 +- pkg/cacheutil/memcached_server_selector.go | 5 +- .../memcached_server_selector_test.go | 48 +++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8dc8e8dd..422cefdb47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan We use *breaking* word for marking changes that are not backward compatible (relates only to v0.y.z releases.) -## [v0.10.0-rc.0](https://github.com/thanos-io/thanos/releases/tag/v0.10.0-rc.0) - 2020.01.08 +## [v0.10.0-rc.1](https://github.com/thanos-io/thanos/releases/tag/v0.10.0-rc.1) - 2020.01.10 ### Fixed @@ -29,6 +29,7 @@ Compactor now properly handles partial block uploads for all operation like rete - [#1872](https://github.com/thanos-io/thanos/pull/1872) Ruler: `/api/v1/rules` now shows a properly formatted value - [#1945](https://github.com/thanos-io/thanos/pull/1945) `master` container images are now built with Go 1.13 - [#1956](https://github.com/thanos-io/thanos/pull/1956) Ruler: now properly ignores duplicated query addresses +- [#1975](https://github.com/thanos-io/thanos/pull/1975) Store Gateway: fixed panic caused by memcached servers selector when there's 1 memcached node ### Added diff --git a/pkg/cacheutil/memcached_server_selector.go b/pkg/cacheutil/memcached_server_selector.go index 1318574045..37b693367c 100644 --- a/pkg/cacheutil/memcached_server_selector.go +++ b/pkg/cacheutil/memcached_server_selector.go @@ -74,9 +74,12 @@ func (s *MemcachedJumpHashSelector) PickServer(key string) (net.Addr, error) { return nil, memcache.ErrNoServers } if len(addrs) == 1 { + picked := addrs[0] + addrs = (addrs)[:0] addrsPool.Put(&addrs) - return (addrs)[0], nil + + return picked, nil } // Pick a server using the jump hash. diff --git a/pkg/cacheutil/memcached_server_selector_test.go b/pkg/cacheutil/memcached_server_selector_test.go index a215604ece..466db1c401 100644 --- a/pkg/cacheutil/memcached_server_selector_test.go +++ b/pkg/cacheutil/memcached_server_selector_test.go @@ -37,6 +37,54 @@ func TestNatSort(t *testing.T) { testutil.Equals(t, expected, input) } +func TestMemcachedJumpHashSelector_PickServer(t *testing.T) { + defer leaktest.CheckTimeout(t, 10*time.Second)() + + tests := []struct { + addrs []string + key string + expectedAddr string + expectedErr error + }{ + { + addrs: []string{}, + key: "test-1", + expectedErr: memcache.ErrNoServers, + }, + { + addrs: []string{"127.0.0.1:11211"}, + key: "test-1", + expectedAddr: "127.0.0.1:11211", + }, + { + addrs: []string{"127.0.0.1:11211", "127.0.0.2:11211"}, + key: "test-1", + expectedAddr: "127.0.0.1:11211", + }, + { + addrs: []string{"127.0.0.1:11211", "127.0.0.2:11211"}, + key: "test-2", + expectedAddr: "127.0.0.2:11211", + }, + } + + s := MemcachedJumpHashSelector{} + + for _, test := range tests { + testutil.Ok(t, s.SetServers(test.addrs...)) + + actualAddr, err := s.PickServer(test.key) + + if test.expectedErr != nil { + testutil.Equals(t, test.expectedErr, err) + testutil.Equals(t, nil, actualAddr) + } else { + testutil.Ok(t, err) + testutil.Equals(t, test.expectedAddr, actualAddr.String()) + } + } +} + func TestMemcachedJumpHashSelector_Each_ShouldRespectServersOrdering(t *testing.T) { defer leaktest.CheckTimeout(t, 10*time.Second)()