Changed the default value for -distributor.ha-tracker.prefix in order to not clash with other keys

CHANGELOG.md

@@ -13,6 +13,7 @@
 * [CHANGE] Deprecated `-distributor.limiter-reload-period` flag. #1766
 * [CHANGE] Ingesters now write only normalised tokens to the ring, although they can still read denormalised tokens used by other ingesters. `-ingester.normalise-tokens` is now deprecated, and ignored. If you want to switch back to using denormalised tokens, you need to downgrade to Cortex 0.4.0. Previous versions don't handle claiming tokens from normalised ingesters correctly. #1809
 * [CHANGE] Overrides mechanism has been renamed to "runtime config", and is now separate from limits. Runtime config is simply a file that is reloaded by Cortex every couple of seconds. Limits and now also multi KV use this mechanism.<br />New arguments were introduced: `-runtime-config.file` (defaults to empty) and `-runtime-config.reload-period` (defaults to 10 seconds), which replace previously used `-limits.per-user-override-config` and `-limits.per-user-override-period` options. Old options are still used if `-runtime-config.file` is not specified. This change is also reflected in YAML configuration, where old `limits.per_tenant_override_config` and `limits.per_tenant_override_period` fields are replaced with `runtime_config.file` and `runtime_config.period` respectively. #1749
+* [CHANGE] Changed the default value for `-distributor.ha-tracker.prefix` from `collectors/` to `ha-tracker/` in order to not clash with other keys (ie. ring) stored in the same key-value store. #1940
 * [FEATURE] The distributor can now drop labels from samples (similar to the removal of the replica label for HA ingestion) per user via the `distributor.drop-label` flag. #1726
 * [FEATURE] Added `global` ingestion rate limiter strategy. Deprecated `-distributor.limiter-reload-period` flag. #1766
 * [FEATURE] Added support for Microsoft Azure blob storage to be used for storing chunk data. #1913

pkg/distributor/distributor_ring.go

@@ -40,7 +40,7 @@ func (cfg *RingConfig) RegisterFlags(f *flag.FlagSet) {
 	}
 
 	// Ring flags
-	cfg.KVStore.RegisterFlagsWithPrefix("distributor.ring.", f)
+	cfg.KVStore.RegisterFlagsWithPrefix("distributor.ring.", "collectors/", f)
 	f.DurationVar(&cfg.HeartbeatPeriod, "distributor.ring.heartbeat-period", 5*time.Second, "Period at which to heartbeat to the ring.")
 	f.DurationVar(&cfg.HeartbeatTimeout, "distributor.ring.heartbeat-timeout", time.Minute, "The heartbeat timeout after which distributors are considered unhealthy within the ring.")
 

pkg/distributor/ha_tracker.go

@@ -115,8 +115,12 @@ func (cfg *HATrackerConfig) RegisterFlags(f *flag.FlagSet) {
 		"distributor.ha-tracker.failover-timeout",
 		30*time.Second,
 		"If we don't receive any samples from the accepted replica for a cluster in this amount of time we will failover to the next replica we receive a sample from. This value must be greater than the update timeout")
-	// We want the ability to use different Consul instances for the ring and for HA cluster tracking.
-	cfg.KVStore.RegisterFlagsWithPrefix("distributor.ha-tracker.", f)
+
+	// We want the ability to use different Consul instances for the ring and
+	// for HA cluster tracking. We also customize the default keys prefix, in
+	// order to not clash with the ring key if they both share the same KVStore
+	// backend (ie. run on the same consul cluster).
+	cfg.KVStore.RegisterFlagsWithPrefix("distributor.ha-tracker.", "ha-tracker/", f)
 }
 
 // Validate config and returns error on failure

pkg/distributor/ha_tracker_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/weaveworks/common/user"
 
 	"github.com/cortexproject/cortex/pkg/ingester/client"
+	"github.com/cortexproject/cortex/pkg/ring"
 	"github.com/cortexproject/cortex/pkg/ring/kv"
 	"github.com/cortexproject/cortex/pkg/ring/kv/codec"
 	"github.com/cortexproject/cortex/pkg/ring/kv/consul"
@@ -492,3 +493,13 @@ func TestFindHALabels(t *testing.T) {
 		assert.Equal(t, c.expected.replica, replica)
 	}
 }
+
+func TestHATrackerConfig_ShouldCustomizePrefixDefaultValue(t *testing.T) {
+	haConfig := HATrackerConfig{}
+	ringConfig := ring.Config{}
+	flagext.DefaultValues(&haConfig)
+	flagext.DefaultValues(&ringConfig)
+
+	assert.Equal(t, "ha-tracker/", haConfig.KVStore.Prefix)
+	assert.NotEqual(t, haConfig.KVStore.Prefix, ringConfig.KVStore.Prefix)
+}

pkg/ring/kv/client.go

@@ -45,21 +45,21 @@ type Config struct {
 // store flag with the prefix ring, so ring.store. For everything else we pass the prefix
 // to the Consul flags.
 // If prefix is not an empty string it should end with a period.
-func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
+func (cfg *Config) RegisterFlagsWithPrefix(flagsPrefix, defaultPrefix string, f *flag.FlagSet) {
 	// We need Consul flags to not have the ring prefix to maintain compatibility.
 	// This needs to be fixed in the future (1.0 release maybe?) when we normalize flags.
 	// At the moment we have consul.<flag-name>, and ring.store, going forward it would
 	// be easier to have everything under ring, so ring.consul.<flag-name>
-	cfg.Consul.RegisterFlags(f, prefix)
-	cfg.Etcd.RegisterFlagsWithPrefix(f, prefix)
-	cfg.Multi.RegisterFlagsWithPrefix(f, prefix)
-	cfg.Memberlist.RegisterFlags(f, prefix)
+	cfg.Consul.RegisterFlags(f, flagsPrefix)
+	cfg.Etcd.RegisterFlagsWithPrefix(f, flagsPrefix)
+	cfg.Multi.RegisterFlagsWithPrefix(f, flagsPrefix)
+	cfg.Memberlist.RegisterFlags(f, flagsPrefix)
 
-	if prefix == "" {
-		prefix = "ring."
+	if flagsPrefix == "" {
+		flagsPrefix = "ring."
 	}
-	f.StringVar(&cfg.Prefix, prefix+"prefix", "collectors/", "The prefix for the keys in the store. Should end with a /.")
-	f.StringVar(&cfg.Store, prefix+"store", "consul", "Backend storage to use for the ring (consul, etcd, inmemory, multi, memberlist [experimental]).")
+	f.StringVar(&cfg.Prefix, flagsPrefix+"prefix", defaultPrefix, "The prefix for the keys in the store. Should end with a /.")
+	f.StringVar(&cfg.Store, flagsPrefix+"store", "consul", "Backend storage to use for the ring (consul, etcd, inmemory, multi, memberlist [experimental]).")
 }
 
 // Client is a high-level client for key-value stores (such as Etcd and

pkg/ring/ring.go

@@ -79,7 +79,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
 
 // RegisterFlagsWithPrefix adds the flags required to config this to the given FlagSet with a specified prefix
 func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
-	cfg.KVStore.RegisterFlagsWithPrefix(prefix, f)
+	cfg.KVStore.RegisterFlagsWithPrefix(prefix, "collectors/", f)
 
 	f.DurationVar(&cfg.HeartbeatTimeout, prefix+"ring.heartbeat-timeout", time.Minute, "The heartbeat timeout after which ingesters are skipped for reads/writes.")
 	f.IntVar(&cfg.ReplicationFactor, prefix+"distributor.replication-factor", 3, "The number of ingesters to write to and read from.")