Deprecate -distributor.extend-writes and set it always to false (#1856)

Signed-off-by: Marco Pracucci <marco@pracucci.com>
grafana · May 17, 2022 · 4eb86ed · 4eb86ed
1 parent 5075013
commit 4eb86ed
Show file tree

Hide file tree

Showing 25 changed files with 13 additions and 70 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@
 * [CHANGE] Distributor: removed the following metrics tracking the number of requests from a distributor to ingesters: #1799
   * `cortex_distributor_ingester_appends_total`
   * `cortex_distributor_ingester_append_failures_total`
+* [CHANGE] Distributor / Ruler: deprecated `-distributor.extend-writes`. Now Mimir always behaves as if this setting was set to `false`, which we expect to be safe for every Mimir cluster setup. #1856
 * [FEATURE] Querier: Added support for [streaming remote read](https://prometheus.io/blog/2019/10/10/remote-read-meets-streaming/). Should be noted that benefits of chunking the response are partial here, since in a typical `query-frontend` setup responses will be buffered until they've been completed. #1735
 * [FEATURE] Ruler: Allow setting `evaluation_delay` for each rule group via rules group configuration file. #1474
 * [FEATURE] Ruler: Added support for expression remote evaluation. #1536 #1818

diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json
@@ -910,17 +910,6 @@
           "fieldType": "duration",
           "fieldCategory": "advanced"
         },
-        {
-          "kind": "field",
-          "name": "extend_writes",
-          "required": false,
-          "desc": "Try writing to an additional ingester in the presence of an ingester not in the ACTIVE state. It is useful to disable this along with -ingester.ring.unregister-on-shutdown=false in order to not spread samples to extra ingesters during rolling restarts with consistent naming.",
-          "fieldValue": null,
-          "fieldDefaultValue": true,
-          "fieldFlag": "distributor.extend-writes",
-          "fieldType": "boolean",
-          "fieldCategory": "advanced"
-        },
         {
           "kind": "block",
           "name": "ring",
@@ -2182,7 +2171,7 @@
               "kind": "field",
               "name": "unregister_on_shutdown",
               "required": false,
-              "desc": "Unregister from the ring upon clean shutdown. It can be useful to disable for rolling restarts with consistent naming in conjunction with -distributor.extend-writes=false.",
+              "desc": "Unregister from the ring upon clean shutdown. It can be useful to disable for rolling restarts with consistent naming.",
               "fieldValue": null,
               "fieldDefaultValue": true,
               "fieldFlag": "ingester.ring.unregister-on-shutdown",

diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl
@@ -637,8 +637,8 @@ Usage of ./cmd/mimir/mimir:
     	How frequently to clean up clients for ingesters that have gone away. (default 15s)
   -distributor.drop-label value
     	This flag can be used to specify label names that to drop during sample ingestion within the distributor and can be repeated in order to drop multiple labels.
-  -distributor.extend-writes
-    	Try writing to an additional ingester in the presence of an ingester not in the ACTIVE state. It is useful to disable this along with -ingester.ring.unregister-on-shutdown=false in order to not spread samples to extra ingesters during rolling restarts with consistent naming. (default true)
+  -distributor.extend-writes value
+    	Deprecated: this setting was used to try writing to an additional ingester in the presence of an ingester not in the ACTIVE state. Mimir now behaves as this setting is always disabled.
   -distributor.forwarding.enabled
     	[experimental] Enables the feature to forward certain metrics in remote_write requests, depending on defined rules.
   -distributor.forwarding.request-timeout duration
@@ -934,7 +934,7 @@ Usage of ./cmd/mimir/mimir:
   -ingester.ring.tokens-file-path string
     	File path where tokens are stored. If empty, tokens are not stored at shutdown and restored at startup.
   -ingester.ring.unregister-on-shutdown
-    	Unregister from the ring upon clean shutdown. It can be useful to disable for rolling restarts with consistent naming in conjunction with -distributor.extend-writes=false. (default true)
+    	Unregister from the ring upon clean shutdown. It can be useful to disable for rolling restarts with consistent naming. (default true)
   -ingester.ring.zone-awareness-enabled
     	True to enable the zone-awareness and replicate ingested samples across different availability zones. This option needs be set on ingesters, distributors, queriers and rulers when running in microservices mode.
   -ingester.stream-chunks-when-using-blocks

diff --git a/cmd/mimir/help.txt.tmpl b/cmd/mimir/help.txt.tmpl
@@ -225,6 +225,8 @@ Usage of ./cmd/mimir/mimir:
     	Expands ${var} or $var in config according to the values of the environment variables.
   -config.file value
     	Configuration file to load.
+  -distributor.extend-writes value
+    	Deprecated: this setting was used to try writing to an additional ingester in the presence of an ingester not in the ACTIVE state. Mimir now behaves as this setting is always disabled.
   -distributor.ha-tracker.cluster string
     	Prometheus label to look for in samples to identify a Prometheus HA cluster. (default "cluster")
   -distributor.ha-tracker.consul.hostname string

diff --git a/...sources/operators-guide/configuring/reference-configuration-parameters/index.md b/...sources/operators-guide/configuring/reference-configuration-parameters/index.md
@@ -475,13 +475,6 @@ ha_tracker:
 # CLI flag: -distributor.remote-timeout
 [remote_timeout: <duration> | default = 20s]
 
-# (advanced) Try writing to an additional ingester in the presence of an
-# ingester not in the ACTIVE state. It is useful to disable this along with
-# -ingester.ring.unregister-on-shutdown=false in order to not spread samples to
-# extra ingesters during rolling restarts with consistent naming.
-# CLI flag: -distributor.extend-writes
-[extend_writes: <boolean> | default = true]
-
 ring:
   kvstore:
     # Backend storage to use for the ring. Supported values are: consul, etcd,
@@ -677,8 +670,7 @@ ring:
   [instance_availability_zone: <string> | default = ""]
 
   # (advanced) Unregister from the ring upon clean shutdown. It can be useful to
-  # disable for rolling restarts with consistent naming in conjunction with
-  # -distributor.extend-writes=false.
+  # disable for rolling restarts with consistent naming.
   # CLI flag: -ingester.ring.unregister-on-shutdown
   [unregister_on_shutdown: <boolean> | default = true]
 

diff --git a/docs/sources/operators-guide/tools/mimirtool.md b/docs/sources/operators-guide/tools/mimirtool.md
@@ -812,7 +812,6 @@ The script outputs results that are similar to the following:
 
 ```console
 -consul.hostname=consul.cortex-to-mimir.svc.cluster.local:8500
--distributor.extend-writes=true
 -distributor.ha-tracker.enable=false
 -distributor.ha-tracker.enable-for-all-users=true
 -distributor.ha-tracker.etcd.endpoints=etcd-client.cortex-to-mimir.svc.cluster.local.:2379

diff --git a/operations/mimir-tests/test-defaults-generated.yaml b/operations/mimir-tests/test-defaults-generated.yaml
@@ -661,7 +661,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379

diff --git a/operations/mimir-tests/test-disable-chunk-streaming-generated.yaml b/operations/mimir-tests/test-disable-chunk-streaming-generated.yaml
@@ -750,7 +750,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1070,7 +1069,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-gossip-generated.yaml b/operations/mimir-tests/test-gossip-generated.yaml
@@ -777,7 +777,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1105,7 +1104,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.heartbeat-timeout=10m
         - -ingester.ring.prefix=

diff --git a/operations/mimir-tests/test-gossip-multi-zone-generated.yaml b/operations/mimir-tests/test-gossip-multi-zone-generated.yaml
@@ -939,7 +939,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1315,7 +1314,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.heartbeat-timeout=10m
         - -ingester.ring.prefix=

diff --git a/operations/mimir-tests/test-gossip-multikv-generated.yaml b/operations/mimir-tests/test-gossip-multikv-generated.yaml
@@ -780,7 +780,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1120,7 +1119,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-gossip-multikv-switch-primary-secondary-generated.yaml b/operations/mimir-tests/test-gossip-multikv-switch-primary-secondary-generated.yaml
@@ -780,7 +780,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1120,7 +1119,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-gossip-multikv-teardown-generated.yaml b/operations/mimir-tests/test-gossip-multikv-teardown-generated.yaml
@@ -780,7 +780,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1108,7 +1107,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.heartbeat-timeout=10m
         - -ingester.ring.prefix=

diff --git a/operations/mimir-tests/test-gossip-ruler-disabled-generated.yaml b/operations/mimir-tests/test-gossip-ruler-disabled-generated.yaml
@@ -759,7 +759,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379

diff --git a/operations/mimir-tests/test-multi-zone-generated.yaml b/operations/mimir-tests/test-multi-zone-generated.yaml
@@ -899,7 +899,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1267,7 +1266,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-multi-zone-with-ongoing-migration-generated.yaml b/operations/mimir-tests/test-multi-zone-with-ongoing-migration-generated.yaml
@@ -961,7 +961,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1329,7 +1328,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-query-sharding-generated.yaml b/operations/mimir-tests/test-query-sharding-generated.yaml
@@ -749,7 +749,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1073,7 +1072,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-shuffle-sharding-generated.yaml b/operations/mimir-tests/test-shuffle-sharding-generated.yaml
@@ -749,7 +749,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1075,7 +1074,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -distributor.ingestion-tenant-shard-size=3
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500

diff --git a/operations/mimir-tests/test-storage-azure-generated.yaml b/operations/mimir-tests/test-storage-azure-generated.yaml
@@ -749,7 +749,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1073,7 +1072,6 @@ spec:
         - -blocks-storage.bucket-store.metadata-cache.memcached.max-item-size=1048576
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-storage-gcs-generated.yaml b/operations/mimir-tests/test-storage-gcs-generated.yaml
@@ -749,7 +749,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1069,7 +1068,6 @@ spec:
         - -blocks-storage.bucket-store.sync-dir=/data/tsdb
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.gcs.bucket-name=blocks-bucket
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir-tests/test-storage-s3-generated.yaml b/operations/mimir-tests/test-storage-s3-generated.yaml
@@ -749,7 +749,6 @@ spec:
             topologyKey: kubernetes.io/hostname
       containers:
       - args:
-        - -distributor.extend-writes=true
         - -distributor.ha-tracker.enable=true
         - -distributor.ha-tracker.enable-for-all-users=true
         - -distributor.ha-tracker.etcd.endpoints=etcd-client.default.svc.cluster.local.:2379
@@ -1071,7 +1070,6 @@ spec:
         - -blocks-storage.bucket-store.sync-interval=15m
         - -blocks-storage.s3.bucket-name=blocks-bucket
         - -blocks-storage.s3.endpoint=s3.dualstack.us-east-1.amazonaws.com
-        - -distributor.extend-writes=true
         - -distributor.health-check-ingesters=true
         - -ingester.ring.consul.hostname=consul.default.svc.cluster.local:8500
         - -ingester.ring.heartbeat-timeout=10m

diff --git a/operations/mimir/distributor.libsonnet b/operations/mimir/distributor.libsonnet
@@ -31,10 +31,6 @@
       'distributor.ring.store': 'consul',
       'distributor.ring.consul.hostname': 'consul.%s.svc.cluster.local:8500' % $._config.namespace,
       'distributor.ring.prefix': '',
-
-      // Do not extend the replication set on unhealthy (or LEAVING) ingester when "unregister on shutdown"
-      // is set to false.
-      'distributor.extend-writes': $._config.unregister_ingesters_on_shutdown,
     },
 
   distributor_ports:: $.util.defaultPorts,

diff --git a/operations/mimir/ruler.libsonnet b/operations/mimir/ruler.libsonnet
@@ -31,10 +31,6 @@
       'server.grpc-max-recv-msg-size-bytes': 10 * 1024 * 1024,
 
       'server.http-listen-port': $._config.server_http_port,
-
-      // Do not extend the replication set on unhealthy (or LEAVING) ingester when "unregister on shutdown"
-      // is set to false.
-      'distributor.extend-writes': $._config.unregister_ingesters_on_shutdown,
     },
 
   ruler_container::