search

Ericsson / ered

An Erlang client library for Valkey/Redis Cluster
MIT License
16 stars 8 forks source link

Remove hard-coded check for >= 2 nodes #36

Closed zuiderkwast closed 1 year ago

drmull commented 1 year ago

Have you confirmed that the scenario described in the comment is not possible anymore? If the scenario is still valid then this change could lead to a premature cluster ready signal to the library user. If unsure then I suggest you make the cluster node check configurable and default it to two as to avoid introducing a regression.

zuiderkwast commented 1 year ago

@drmull Nice to see you're still watching this project. :-)

Have you confirmed that the scenario described in the comment is not possible anymore?

Are you referring to this?

%% Need at least two nodes in the cluster. During some startup scenarios it %% is possible to have a intermittent situation with only one node.

There is another check: that all slots are covered. We rely on that for 'cluster_ok'. That's why I didn't add a config for the minimum number of nodes. (There is now a use-case for a single-node cluster.)

It's possible to get the problem you described if you start one node, give it all the slots and then migrate the slots over to other nodes. Then this can happen, but it's not a very wise way of starting up a cluster because slot migration is a bit buggy. (There are some race conditions that can lead to inconsistent slot ownership, etc. e.g. if there is a failover or a CLUSTER MEET at the same time as a slot migration.)

drmull commented 1 year ago

@drmull Nice to see you're still watching this project. :-)

I check my mailbox from time to time :)

I would not be surprised if there was some non-standard procedures involved when this was a problem. I was not involved at that time so I am not sure if it was encountered in a live scenario or some kind of test set-up. Anyway it might be good to double check with Ericsson internal users just to make sure this is not an issue anymore.