Fixed crash in the memcached servers selector when there's only 1 ser…

…ver (thanos-io#1975) * Fixed crash in the memcached servers selector when there's only 1 server Signed-off-by: Marco Pracucci <[email protected]> * Updated changelog Signed-off-by: Marco Pracucci <[email protected]> * Assert expected error in tests Signed-off-by: Marco Pracucci <[email protected]>
GiedriusS · Jan 10, 2020 · 8ef322d · 8ef322d
1 parent bfd65e2
commit 8ef322d
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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)()