Address feedback

Signed-off-by: Goutham Veeramachaneni <[email protected]>

Author: gouthamve

CHANGELOG.md

@@ -2,23 +2,23 @@
 
 ## master / unreleased
 
-* [CHANGE] Renamed the `memcache.write-back-goroutines` and `memcache.write-back-buffer` flags to `cache.write-back-concurrency` and `cache.write-back-buffer`. This affects the following flags:
-  - `-frontend.memcache.write-back-buffer` --> `-frontend.cache.write-back-buffer` 
-  - `-frontend.memcache.write-back-goroutines` --> `-frontend.cache.write-back-concurrency`
-  - `-store.index-cache-read.memcache.write-back-buffer` --> `-store.index-cache-read.cache.write-back-buffer`
-  - `-store.index-cache-read.memcache.write-back-goroutines` --> `-store.index-cache-read.cache.write-back-concurrency`
-  - `-store.index-cache-write.memcache.write-back-buffer` --> `-store.index-cache-write.cache.write-back-buffer`
-  - `-store.index-cache-write.memcache.write-back-goroutines` --> `-store.index-cache-write.cache.write-back-concurrency`
-  - `-memcache.write-back-buffer` --> `-store.chunks-cache.cache.write-back-buffer`. Note the next change log for the difference.
-  - `-memcache.write-back-goroutines` --> `-store.chunks-cache.cache.write-back-concurrency`. Note the next change log for the difference.
+* [CHANGE] Renamed the `memcache.write-back-goroutines` and `memcache.write-back-buffer` flags to `background.write-back-concurrency` and `background.write-back-buffer`. This affects the following flags:
+  - `-frontend.memcache.write-back-buffer` --> `-frontend.background.write-back-buffer` 
+  - `-frontend.memcache.write-back-goroutines` --> `-frontend.background.write-back-concurrency`
+  - `-store.index-cache-read.memcache.write-back-buffer` --> `-store.index-cache-read.background.write-back-buffer`
+  - `-store.index-cache-read.memcache.write-back-goroutines` --> `-store.index-cache-read.background.write-back-concurrency`
+  - `-store.index-cache-write.memcache.write-back-buffer` --> `-store.index-cache-write.background.write-back-buffer`
+  - `-store.index-cache-write.memcache.write-back-goroutines` --> `-store.index-cache-write.background.write-back-concurrency`
+  - `-memcache.write-back-buffer` --> `-store.chunks-cache.background.write-back-buffer`. Note the next change log for the difference.
+  - `-memcache.write-back-goroutines` --> `-store.chunks-cache.background.write-back-concurrency`. Note the next change log for the difference.
 
 * [CHANGE] Renamed the chunk cache flags to have `store.chunks-cache.` as prefix. This means the following flags have been changed:
   - `-cache.enable-fifocache` --> `-store.chunks-cache.cache.enable-fifocache` 
   - `-default-validity` --> `-store.chunks-cache.default-validity` 
   - `-fifocache.duration` --> `-store.chunks-cache.fifocache.duration` 
   - `-fifocache.size` --> `-store.chunks-cache.fifocache.size` 
-  - `-memcache.write-back-buffer` --> `-store.chunks-cache.cache.write-back-buffer`. Note the previous change log for the difference. 
-  - `-memcache.write-back-goroutines` --> `-store.chunks-cache.cache.write-back-concurrency`. Note the previous change log for the difference. 
+  - `-memcache.write-back-buffer` --> `-store.chunks-cache.background.write-back-buffer`. Note the previous change log for the difference. 
+  - `-memcache.write-back-goroutines` --> `-store.chunks-cache.background.write-back-concurrency`. Note the previous change log for the difference. 
   - `-memcached.batchsize` --> `-store.chunks-cache.memcached.batchsize` 
   - `-memcached.consistent-hash` --> `-store.chunks-cache.memcached.consistent-hash` 
   - `-memcached.expiration` --> `-store.chunks-cache.memcached.expiration` 

docs/configuration/arguments.md

@@ -108,11 +108,11 @@ The ingester query API was improved over time, but defaults to the old behaviour
 
    When caching query results, it is desirable to prevent the caching of very recent results that might still be in flux.  Use this parameter to configure the age of results that should be excluded.
 
-- `-memcached.{hostname, service, timeout}`
+- `-frontend.memcached.{hostname, service, timeout}`
 
    Use these flags to specify the location and timeout of the memcached cluster used to cache query results.
 
-- `-redis.{endpoint, timeout}`
+- `-frontend.redis.{endpoint, timeout}`
 
    Use these flags to specify the location and timeout of the Redis service used to cache query results.
 
@@ -288,7 +288,7 @@ It also talks to a KVStore and has it's own copies of the same flags used by the
 
 - `-ingester.spread-flushes`
 
-  Makes the ingester flush each timeseries at a specific point in the `max-chunk-age` cycle. This means multiple replicas of a chunk are very likely to contain the same contents which cuts chunk storage space by up to 66%. Set `-ingester.chunk-age-jitter` to `0` when using this option. If a chunk cache is configured (via `-memcached.hostname`) then duplicate chunk writes are skipped which cuts write IOPs.
+  Makes the ingester flush each timeseries at a specific point in the `max-chunk-age` cycle. This means multiple replicas of a chunk are very likely to contain the same contents which cuts chunk storage space by up to 66%. Set `-ingester.chunk-age-jitter` to `0` when using this option. If a chunk cache is configured (via `-store.chunks-cache.memcached.hostname`) then duplicate chunk writes are skipped which cuts write IOPs.
 
 - `-ingester.join-after`
 

docs/configuration/config-file-reference.md

@@ -645,11 +645,11 @@ results_cache:
 
     background:
       # At what concurrency to write back to cache.
-      # CLI flag: -frontend.cache.write-back-concurrency
+      # CLI flag: -frontend.background.write-back-concurrency
       [writeback_goroutines: <int> | default = 10]
 
       # How many key batches to buffer for background write-back.
-      # CLI flag: -frontend.cache.write-back-buffer
+      # CLI flag: -frontend.background.write-back-buffer
       [writeback_buffer: <int> | default = 10000]
 
     # The memcached_config block configures how data is stored in Memcached (ie.
@@ -1517,12 +1517,12 @@ index_queries_cache_config:
   background:
     # Cache config for index entry reading. At what concurrency to write back to
     # cache.
-    # CLI flag: -store.index-cache-read.cache.write-back-concurrency
+    # CLI flag: -store.index-cache-read.background.write-back-concurrency
     [writeback_goroutines: <int> | default = 10]
 
     # Cache config for index entry reading. How many key batches to buffer for
     # background write-back.
-    # CLI flag: -store.index-cache-read.cache.write-back-buffer
+    # CLI flag: -store.index-cache-read.background.write-back-buffer
     [writeback_buffer: <int> | default = 10000]
 
   # The memcached_config block configures how data is stored in Memcached (ie.
@@ -1570,12 +1570,12 @@ chunk_cache_config:
 
   background:
     # Cache config for chunks. At what concurrency to write back to cache.
-    # CLI flag: -store.chunks-cache.cache.write-back-concurrency
+    # CLI flag: -store.chunks-cache.background.write-back-concurrency
     [writeback_goroutines: <int> | default = 10]
 
     # Cache config for chunks. How many key batches to buffer for background
     # write-back.
-    # CLI flag: -store.chunks-cache.cache.write-back-buffer
+    # CLI flag: -store.chunks-cache.background.write-back-buffer
     [writeback_buffer: <int> | default = 10000]
 
   # The memcached_config block configures how data is stored in Memcached (ie.
@@ -1609,12 +1609,12 @@ write_dedupe_cache_config:
   background:
     # Cache config for index entry writing. At what concurrency to write back to
     # cache.
-    # CLI flag: -store.index-cache-write.cache.write-back-concurrency
+    # CLI flag: -store.index-cache-write.background.write-back-concurrency
     [writeback_goroutines: <int> | default = 10]
 
     # Cache config for index entry writing. How many key batches to buffer for
     # background write-back.
-    # CLI flag: -store.index-cache-write.cache.write-back-buffer
+    # CLI flag: -store.index-cache-write.background.write-back-buffer
     [writeback_buffer: <int> | default = 10000]
 
   # The memcached_config block configures how data is stored in Memcached (ie.
@@ -2093,7 +2093,7 @@ The `memcached_client_config` configures the client used to connect to Memcached
 &nbsp;
 
 ```yaml
-# Hostname for memcached service to use when caching chunks. If empty, no
+# Hostname for memcached service to use. If empty and if addresses is unset, no
 # memcached will be used.
 # CLI flag: -<prefix>.memcached.hostname
 [host: <string> | default = ""]

docs/guides/caching.md

@@ -121,7 +121,7 @@ These are typically a few KB in size, and depend mostly on the duration and enco
 The chunk cache is a write-through cache - chunks are written to the cache as they are flushed to the chunk store.  This ensures the cache always contains the most recent chunks.
 Items stay in the cache indefinitely.
 
-The chunk cache should be configured on the **ingester**, **querier** and **ruler** using the flags with a prefix: `-store.chunks-cache`.
+The chunk cache should be configured on the **ingester**, **querier** and **ruler** using the flags with the prefix `-store.chunks-cache`.
 
 It is best practice to ensure the chunk cache is big enough to accommodate at least 24 hours of chunk data.
 You can use the following query (from the [cortex-mixin](https://github.com/grafana/cortex-jsonnet)) to estimate the required number of memcached replicas:

k8s/memcached-dep.yaml

@@ -17,7 +17,7 @@ spec:
     spec:
       containers:
       - name: memcached
-        image: memcached:1.6.0
+        image: memcached:1.4.25
         imagePullPolicy: IfNotPresent
         args:
         - -m 64 # Maximum memory to use, in megabytes. 64MB is default.

pkg/chunk/cache/background.go

@@ -32,8 +32,8 @@ type BackgroundConfig struct {
 
 // RegisterFlagsWithPrefix adds the flags required to config this to the given FlagSet
 func (cfg *BackgroundConfig) RegisterFlagsWithPrefix(prefix string, description string, f *flag.FlagSet) {
-	f.IntVar(&cfg.WriteBackGoroutines, prefix+"cache.write-back-concurrency", 10, description+"At what concurrency to write back to cache.")
-	f.IntVar(&cfg.WriteBackBuffer, prefix+"cache.write-back-buffer", 10000, description+"How many key batches to buffer for background write-back.")
+	f.IntVar(&cfg.WriteBackGoroutines, prefix+"background.write-back-concurrency", 10, description+"At what concurrency to write back to cache.")
+	f.IntVar(&cfg.WriteBackBuffer, prefix+"background.write-back-buffer", 10000, description+"How many key batches to buffer for background write-back.")
 }
 
 type backgroundCache struct {

pkg/chunk/cache/memcached_client.go

@@ -49,7 +49,7 @@ type MemcachedClientConfig struct {
 
 // RegisterFlagsWithPrefix adds the flags required to config this to the given FlagSet
 func (cfg *MemcachedClientConfig) RegisterFlagsWithPrefix(prefix, description string, f *flag.FlagSet) {
-	f.StringVar(&cfg.Host, prefix+"memcached.hostname", "", description+"Hostname for memcached service to use when caching chunks. If empty, no memcached will be used.")
+	f.StringVar(&cfg.Host, prefix+"memcached.hostname", "", description+"Hostname for memcached service to use. If empty and if addresses is unset, no memcached will be used.")
 	f.StringVar(&cfg.Service, prefix+"memcached.service", "memcached", description+"SRV service used to discover memcache servers.")
 	f.IntVar(&cfg.MaxIdleConns, prefix+"memcached.max-idle-conns", 16, description+"Maximum number of idle connections in pool.")
 	f.DurationVar(&cfg.Timeout, prefix+"memcached.timeout", 100*time.Millisecond, description+"Maximum time to wait before giving up on memcached requests.")

pkg/chunk/chunk_store.go

@@ -55,7 +55,10 @@ type StoreConfig struct {
 	// Limits query start time to be greater than now() - MaxLookBackPeriod, if set.
 	MaxLookBackPeriod time.Duration `yaml:"max_look_back_period"`
 
-	// Not visible in yaml because the setting shouldn't be common between ingesters and queriers
+	// Not visible in yaml because the setting shouldn't be common between ingesters and queriers.
+	// This exists in case we don't want to cache all the chunks but still want to take advantage of
+	// ingester chunk write deduplication. But for the queriers we need the full value. So when this option
+	// is set, use different caches for ingesters and queriers.
 	chunkCacheStubs bool // don't write the full chunk to cache, just a stub entry
 }