Distributor HA tracker ring should allow inmemory as a KV store

[mehta-ankit]

Describe the bug
V1.14.1 for cortex throws an error when using distributor HA tracker with inmemoery ring and stops them from running.

error validating config: invalid distributor config: invalid HATracker KV store type: inmemory

I believe the error comes from this PR: #4881

To Reproduce
Steps to reproduce the behavior:

1. Start Cortex v1.14.1
2. Have configuration for distributor as follows:

distributor:
  ha_tracker:
    enable_ha_tracker: true
    kvstore:
      store: inmemory

Expected behavior
Since docs mention inmemory as an option for the HAtracker store: https://cortexmetrics.io/docs/configuration/arguments/#ringha-tracker-store
https://cortexmetrics.io/docs/configuration/configuration-file/#distributor_config
It should not be validating against it and this list should accept consul, etcd or inmemory as valid values.
https://github.com/cortexproject/cortex/blob/v1.14.1/pkg/distributor/ha_tracker.go#L92

Environment:

• Infrastructure: Kubernetes v1.23
• Deployment tool: Helm

Additional Context
We were running v1.11.1 with this configuration and it was working fine. I do not see any changelog mentioning breaking change where inmemory would not be supported so this seems like regression and bad validation in my opinion. But please correct me if i am not understanding it correctly.

[alanprot]

hum.. but are you running more than 1 distributor? because if so, they may pick different servers to accept the traffic what will cause duplicated samples on the ingesters.

Can you explain a little more your use case?

[mehta-ankit]

hum.. but are you running more than 1 distributor? because if so, they may pick different servers to accept the traffic what will cause duplicated samples on the ingesters.

Can you explain a little more your use case?

Hi @alanprot
Thank you for the message! I was looking it from the perspective of something was working and and it did not after upgrade. But you make a good point. I looked at the docs in detail and it seems consul and etcd are only ones supported in microservices mode (running more than 1 distributor)

This is what confused me:

{ring,distributor.ha-tracker}.store Backend storage to use for the HA Tracker (consul, etcd, inmemory, multi).

&

maybe we can add the info in docs there to explain it better.

To answer your question, yes we are running more than 1 Distributor (using cortex helm chart to deploy all the components). So inmemory would not work as each distributor will have its on in memory store. Right ?

[friedrichg]

So inmemory would not work as each distributor will have its on in memory store. Right ?

In memory will not work correctly with multiple distributors as each distributor can have a different state. I can forsee, ingestion will get randomly all sorts of out of order errors.

Maybe we can fix it, but it always will be for demo purposes only. As it was in v1.11.1

Probably we should also put a warning to prevent users from having the wrong expectation from inmemory ha-tracker.

[mehta-ankit]

@friedrichg yea a warning or change of docs to highlight this would be helpful. For now I am trying to use consul for my hatracker ring use case.
do you want to use the issue for any sort of change ? If not, feel free to close it as my expectation was wrong here and there is no bug.

[alvinlin123]

We should definitely update the document if it's misleading. We can use this issue to fix the document in my opinion.

[dsabsay]

This is a good first issue. I suggest an update to the docs that ideally removes inmemory and memberlist as options in the config reference. If that is cumbersome based on how this doc is generated, we should at least add a warning about the inmemory store, similar to the warning already present for memberlist.