cortex icon indicating copy to clipboard operation
cortex copied to clipboard

make memberlist kvstore to default

Open songjiayang opened this issue 3 years ago • 11 comments

Is your feature request related to a problem? Please describe.

currently the ring kvstore default value is consul, how about changing it to memberlist.

songjiayang avatar Nov 16 '22 07:11 songjiayang

I think this idea is worth the discussing. What pros and cons do you think changing the default would have?

For me the cons is it would be a breaking change? Pros is better out-of-box experience ( no need to deploy extra service out-of-box).

alvinlin123 avatar Nov 16 '22 17:11 alvinlin123

Frankly speaking, this is indeed a break change. but when we want to use consul, we must configure consul_config, eg the address, so in fact, only a few people did not set store to consule.

if memberlist is default, we can use common memberlist_config, configuration will be more simpler.

memberlist isn't default:

memberlist:
  join_members: [cortex-1, cortex-2, cortex-3]

ingester:
  lifecycler:
    ring:
      kvstore:
        store: memberlist

store_gateway:
  sharding_enabled: true
  sharding_ring:
    kvstore:
      store: memberlist

ruler:
  enable_api: true
  enable_sharding: true
  poll_interval: 2s
  ring:
    kvstore:
      store: memberlist

memberlist is default:

memberlist:
  join_members: [cortex-1, cortex-2, cortex-3]

store_gateway:
  sharding_enabled: true

ruler:
  enable_api: true
  enable_sharding: true
  poll_interval: 2s

songjiayang avatar Nov 17 '22 02:11 songjiayang

+1 to this change. I love the out of box experience and memberlist should be good enough for most of the users.

yeya24 avatar Nov 19 '22 19:11 yeya24

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 02 '23 12:04 stale[bot]

Will this continue to move forward?

wgliang avatar Aug 09 '24 02:08 wgliang

I think make sense to change it to default!

Im afraid about the breaking change indeed.. we may break customer on next release if they did not explicitly configure this? Even though i think its unlikely that we configure the other consul config but not the type.

alanprot avatar Aug 09 '24 02:08 alanprot

I think we can start by supporting optional use of memberlist, and as for changing the default, we can transition to a later version.

wgliang avatar Aug 09 '24 02:08 wgliang

To some extent, customers do not want to have external dependencies (Etcd...risk) in production environments. We are way behind the other one project in some aspects, you know what I mean.

wgliang avatar Aug 09 '24 02:08 wgliang

If we want to change it anyway then there is breaking change for sure so I am not concerned about breaking customers. I think I am ok to change memberlist as the default.

That being said, there is nothing blocking this change. It is always welcome to create a PR to Cortex if this is the feature you are looking for.

yeya24 avatar Aug 09 '24 02:08 yeya24

I think we can start by supporting optional use of memberlist, and as for changing the default, we can transition to a later version.

Cortex already support memberlist.

alanprot avatar Aug 09 '24 02:08 alanprot

I think we can start by supporting optional use of memberlist, and as for changing the default, we can transition to a later version.

Cortex already support memberlist.

Yeah, I missed it. But tracker kv store only supports consul and etcd.

wgliang avatar Aug 09 '24 06:08 wgliang