Add memcached and enable memcached index cache for Thanos Store #238

kakkoyun · 2020-04-06T13:57:47Z

This PR adds Memcached index cache support for Thanos Store.

Depends:

environments/openshift/obs.jsonnet

brancz · 2020-04-06T17:48:30Z

components/observatorium.libsonnet

@@ -92,6 +93,9 @@ local k = import 'ksonnet/ksonnet.beta.4/k.libsonnet';
          },
          replicas: 1,
          ignoreDeletionMarksDelay: '24h',
+          memcached+: {
+            addresses: ['dnssrv+_http._tcp.%s.%s.svc.cluster.local' % [obs.storeCache.service.metadata.name, obs.storeCache.service.metadata.namespace]],


we can't rely on the .cluster.local suffix

@brancz Could you please explain a little bit more? We already rely heavily on .cluster.local across the board, what do I miss?

We do 😱 . The problem is that this suffix is configurable for every kubernetes, cluster so we need to omit it. The local DNS server will still resolve it correctly for the local service, but it may not be correct if we add the suffix.

tl;dr We need to remove it wherever we use it. (I'm fine if that happens in a separate PR)

environments/base/manifests/store-cache-statefulSet.yaml

bwplotka

Nice, looking good generally! Some minor comments only. Does it work? (:

environments/base/manifests/store-cache-statefulSet.yaml

environments/openshift/obs.jsonnet

environments/base/default-config.libsonnet

environments/operator/operator-config.libsonnet

squat

I think there are a couple of things that could be cleaned up and I have some small questions. Great work so far!

environments/openshift/obs.jsonnet

api/v1alpha1/observatorium_types.go

components/memcached.libsonnet

environments/base/default-config.libsonnet

fixed

bwplotka

NIce, some comments (:

bwplotka · 2020-04-17T08:03:49Z

components/memcached.libsonnet

+        { name: 'client', containerPort: mc.service.spec.ports[0].port },
+      ]) +
+      container.withArgs([
+        '-m %(memoryLimitMb)s' % mc.config,


For future, using long flags is preferable for readability.

bwplotka · 2020-04-17T08:05:03Z

components/memcached.libsonnet

+    // Convert number to k8s "quantity" (ie 1.5Gi -> "1536Mi")
+    // as per https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/api/resource/quantity.go
+    // Original from https://github.com/grafana/jsonnet-libs/blob/master/memcached/memcached.libsonnet
+    bytesToK8sQuantity(i)::


Wow

And someone just said DON'T USE GOLANG IT HAS IFs? =D

bwplotka · 2020-04-17T08:07:00Z

environments/dev/manifests/thanos-store-shard0-statefulSet.yaml

+        - --experimental.enable-index-cache-postings-compression
+        - |-
+          --index-cache.config="config":
+            "addresses":


How we ensure how many items we can cache? 🤔

@kakkoyun ?

We limit the max item size and max memory, this is how we control maximum number of items in the cache. For our own deployment, we will have another internal PR I'll address them over there.

Also I have created an issue as a follow up to this #243

kakkoyun requested review from metalmatze, brancz, bwplotka and squat April 6, 2020 13:57

aditya-konarde previously requested changes Apr 6, 2020

View reviewed changes

environments/openshift/obs.jsonnet Outdated Show resolved Hide resolved

kakkoyun force-pushed the store_memcached branch 4 times, most recently from 0eea2db to 5d2829b Compare April 6, 2020 14:21

kakkoyun marked this pull request as ready for review April 6, 2020 14:38

kakkoyun requested a review from aditya-konarde April 6, 2020 14:39

brancz reviewed Apr 6, 2020

View reviewed changes

environments/base/manifests/store-cache-statefulSet.yaml Outdated Show resolved Hide resolved

bwplotka reviewed Apr 6, 2020

View reviewed changes

environments/base/manifests/store-cache-statefulSet.yaml Outdated Show resolved Hide resolved

environments/openshift/obs.jsonnet Outdated Show resolved Hide resolved

environments/openshift/obs.jsonnet Outdated Show resolved Hide resolved

rollandf reviewed Apr 7, 2020

View reviewed changes

environments/base/default-config.libsonnet Show resolved Hide resolved

kakkoyun removed request for metalmatze and squat April 7, 2020 08:42

rollandf reviewed Apr 7, 2020

View reviewed changes

environments/operator/operator-config.libsonnet Outdated Show resolved Hide resolved

kakkoyun force-pushed the store_memcached branch 2 times, most recently from 9e88212 to 79a4c0c Compare April 7, 2020 12:12

rollandf reviewed Apr 7, 2020

View reviewed changes

environments/operator/operator-config.libsonnet Outdated Show resolved Hide resolved

kakkoyun force-pushed the store_memcached branch 2 times, most recently from b9afe0a to 54b7c7a Compare April 7, 2020 13:31

kakkoyun added the do not merge label Apr 7, 2020

squat reviewed Apr 7, 2020

View reviewed changes

This was referenced Apr 7, 2020

Remove .cluster.local #242

Open

Re-use memcached component for Telemeter #243

Open

kakkoyun changed the title ~~Add memcached and enable memcached index cache for Thanos Store~~ WIP: Add memcached and enable memcached index cache for Thanos Store Apr 7, 2020

kakkoyun force-pushed the store_memcached branch from 17ba982 to 6ee411f Compare April 8, 2020 18:34

kakkoyun changed the title ~~WIP: Add memcached and enable memcached index cache for Thanos Store~~ Add memcached and enable memcached index cache for Thanos Store Apr 8, 2020

kakkoyun force-pushed the store_memcached branch from 6ee411f to 77a517a Compare April 9, 2020 08:44

Add memcached and enable memcached index cache for Thanos Store

5c74443

kakkoyun force-pushed the store_memcached branch from 77a517a to 5c74443 Compare April 16, 2020 14:08

brancz approved these changes Apr 17, 2020

View reviewed changes

bwplotka reviewed Apr 17, 2020

View reviewed changes

kakkoyun merged commit 97d80db into observatorium:master Apr 17, 2020

kakkoyun deleted the store_memcached branch April 17, 2020 08:17

kakkoyun removed the do not merge label Sep 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add memcached and enable memcached index cache for Thanos Store #238

Add memcached and enable memcached index cache for Thanos Store #238

kakkoyun commented Apr 6, 2020 •

edited

brancz Apr 6, 2020

kakkoyun Apr 7, 2020

brancz Apr 7, 2020

brancz Apr 7, 2020 •

edited

bwplotka left a comment

squat left a comment

bwplotka left a comment

bwplotka Apr 17, 2020

bwplotka Apr 17, 2020

bwplotka Apr 17, 2020

bwplotka Apr 17, 2020

kakkoyun Apr 17, 2020

Add memcached and enable memcached index cache for Thanos Store #238

Add memcached and enable memcached index cache for Thanos Store #238

Conversation

kakkoyun commented Apr 6, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brancz Apr 7, 2020 • edited

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

squat left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kakkoyun commented Apr 6, 2020 •

edited

brancz Apr 7, 2020 •

edited