Allow using multiple memcached clients at the same time. (#2648)

* Allow using multiple memcached clients at the same time. Signed-off-by: Peter Štibraný <[email protected]> * Added unit test to verify that we can have multiple clients. Signed-off-by: Peter Štibraný <[email protected]> * Added CHANGELOG.md entry. Signed-off-by: Peter Štibraný <[email protected]> * Remove unused argument. Signed-off-by: Peter Štibraný <[email protected]>
thanos-io · May 24, 2020 · 079ad42 · 079ad42
1 parent 16f5315
commit 079ad42
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 ### Fixed
 
 * [#2637](https://github.com/thanos-io/thanos/pull/2637) Compact: detect retryable errors that are inside of a wrapped `tsdb.MultiError`
+* [#2648](https://github.com/thanos-io/thanos/pull/2648) Store: allow index cache and caching bucket to be configured at the same time.
 
 ## [v0.13.0](https://github.com/thanos-io/thanos/releases) - IN PROGRESS
 

diff --git a/pkg/cacheutil/memcached_client.go b/pkg/cacheutil/memcached_client.go
@@ -182,12 +182,14 @@ func NewMemcachedClientWithConfig(logger log.Logger, name string, config Memcach
 	client.Timeout = config.Timeout
 	client.MaxIdleConns = config.MaxIdleConnections
 
-	return newMemcachedClient(logger, name, client, selector, config, reg)
+	if reg != nil {
+		reg = prometheus.WrapRegistererWith(prometheus.Labels{"name": name}, reg)
+	}
+	return newMemcachedClient(logger, client, selector, config, reg)
 }
 
 func newMemcachedClient(
 	logger log.Logger,
-	name string,
 	client memcachedClientBackend,
 	selector *MemcachedJumpHashSelector,
 	config MemcachedClientConfig,
@@ -196,7 +198,7 @@ func newMemcachedClient(
 	dnsProvider := dns.NewProvider(
 		logger,
 		extprom.WrapRegistererWithPrefix("thanos_memcached_", reg),
-		dns.ResolverType(dns.GolangResolverType),
+		dns.GolangResolverType,
 	)
 
 	c := &memcachedClient{
@@ -214,34 +216,30 @@ func newMemcachedClient(
 	}
 
 	c.operations = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
-		Name:        "thanos_memcached_operations_total",
-		Help:        "Total number of operations against memcached.",
-		ConstLabels: prometheus.Labels{"name": name},
+		Name: "thanos_memcached_operations_total",
+		Help: "Total number of operations against memcached.",
 	}, []string{"operation"})
 	c.operations.WithLabelValues(opGetMulti)
 	c.operations.WithLabelValues(opSet)
 
 	c.failures = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
-		Name:        "thanos_memcached_operation_failures_total",
-		Help:        "Total number of operations against memcached that failed.",
-		ConstLabels: prometheus.Labels{"name": name},
+		Name: "thanos_memcached_operation_failures_total",
+		Help: "Total number of operations against memcached that failed.",
 	}, []string{"operation"})
 	c.failures.WithLabelValues(opGetMulti)
 	c.failures.WithLabelValues(opSet)
 
 	c.skipped = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
-		Name:        "thanos_memcached_operation_skipped_total",
-		Help:        "Total number of operations against memcached that have been skipped.",
-		ConstLabels: prometheus.Labels{"name": name},
+		Name: "thanos_memcached_operation_skipped_total",
+		Help: "Total number of operations against memcached that have been skipped.",
 	}, []string{"operation", "reason"})
 	c.skipped.WithLabelValues(opGetMulti, reasonMaxItemSize)
 	c.skipped.WithLabelValues(opSet, reasonMaxItemSize)
 
 	c.duration = promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{
-		Name:        "thanos_memcached_operation_duration_seconds",
-		Help:        "Duration of operations against memcached.",
-		ConstLabels: prometheus.Labels{"name": name},
-		Buckets:     []float64{0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.2, 0.5, 1, 3, 6, 10},
+		Name:    "thanos_memcached_operation_duration_seconds",
+		Help:    "Duration of operations against memcached.",
+		Buckets: []float64{0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.2, 0.5, 1, 3, 6, 10},
 	}, []string{"operation"})
 	c.duration.WithLabelValues(opGetMulti)
 	c.duration.WithLabelValues(opSet)

diff --git a/pkg/cacheutil/memcached_client_test.go b/pkg/cacheutil/memcached_client_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/fortytw2/leaktest"
 	"github.com/go-kit/kit/log"
 	"github.com/pkg/errors"
+	"github.com/prometheus/client_golang/prometheus"
 	prom_testutil "github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/thanos-io/thanos/pkg/model"
 	"github.com/thanos-io/thanos/pkg/testutil"
@@ -378,7 +379,7 @@ func TestMemcachedClient_GetMulti(t *testing.T) {
 func prepare(config MemcachedClientConfig, backendMock *memcachedClientBackendMock) (*memcachedClient, error) {
 	logger := log.NewNopLogger()
 	selector := &MemcachedJumpHashSelector{}
-	client, err := newMemcachedClient(logger, "test", backendMock, selector, config, nil)
+	client, err := newMemcachedClient(logger, backendMock, selector, config, nil)
 
 	return client, err
 }
@@ -439,3 +440,18 @@ func (c *memcachedClientBackendMock) waitItems(expected int) error {
 
 	return errors.New("timeout expired while waiting for items in the memcached mock")
 }
+
+func TestMultipleClientsCanUseSameRegistry(t *testing.T) {
+	reg := prometheus.NewRegistry()
+
+	config := defaultMemcachedClientConfig
+	config.Addresses = []string{"127.0.0.1:11211"}
+
+	client1, err := NewMemcachedClientWithConfig(log.NewNopLogger(), "a", config, reg)
+	testutil.Ok(t, err)
+	defer client1.Stop()
+
+	client2, err := NewMemcachedClientWithConfig(log.NewNopLogger(), "b", config, reg)
+	testutil.Ok(t, err)
+	defer client2.Stop()
+}