-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Different RedisFailover's sentinels join together #550
Comments
Do you mean different the sentinel from a different
|
Yes for example A Redis-failover sentinels are joining B Redis-failover sentinels. In some cases because one Redis has a password and the other does not the actual Redis pods can not join together thus the data will not combine.
|
When this happens the operator doesn't have any way to fix it. To give you more context we have over 90 RedisFailovers in our Production cluster and the cluster's node's CPUs are under heavy pressure. Sentinels from different RedisFailovers joining together is a huge problem for us before our current cluster we fixed any kind of problem by using Network Policies to prevent different Redises and Sentinels to connect to eachother. |
@tparsa is there a deterministic way to reproduce the issue? |
I can't provide any way to reproduce the issue. But here is the sentinels.conf of one of the following sentinels:
Replicacount of this RedisFailover is 3 sentinels and 5 redises but as you can see there are 12 (4 * 3) slaves and 8 other sentinels which indicates that 3 different RedisFailovers joined together. |
@tparsa do all the sentinels point to same master, in their config? |
I'm trying to upgrade to redis operator more than 1.0 so we can upgrade k8s to 1.22 and what happened is something similar. I tried the latest version first and what happened with me is the new redis operator chooses redis 6.2 by default so all redis replicas and sentinels got changed at the same time after they settled down some replicas didn't get synced into any clusters and I've two big clusters with 36 sentinels and 27 replicas. When I downgraded the operator to 1.1.1 The replicas that weren't ready got ready but still I've the same situation two big clusters with 36 sentinels and more replicas. |
I still have the situation going on in a test env and I've no idea how to fix it except with removing redis failovers and applying them agian one by one but I'm not sure if it'll help. Now I'm trying to search for a better way to do the migration if anyone can suggest something I'd be thankful. |
@zekena2 In your test env, could you please confirm if all unruly sentinels point to same master? Also, could you please share operator logs for the same?
|
also, which version of operator are you using currently? |
Currently I'm using v1.2.4. using this command I can confirm that almost all sentinels point to the same master except three of the same failover point and some random ones that points to localhost or literally an ip that doesn't belong to any pod.
This is the logs for one of the failovers. I also get the same logs mentioned in this issue. |
Thanks for the details @zekena2 |
@zekena2 are the failovers running in bootstrap mode? |
Also, can you please confirm if you dont see the issue in v.1.2.3? |
@zekena2 could you please share logs with debug mode enabled? |
We don't use any bootstraping. |
@zekena2 thanks for checking; please try |
What I saw in the operator metrics, indicates that the operator does realize there is a problem with the number of sentinels. But the only fix that the operator does is sending a |
@tparsa please do share debug logs if you have the setup running. |
here's debug logs. |
@raghu-nandan-bs I'm afraid of restarting the operator :)) It might cause more sentinels to join together. It can be catastrophic for us. |
I can confirm that deleting and creating the failovers fix the problem but this isn't really a good solution or an upgrade strategy. |
@zekena2 Also deleting all sentinel pods will fix the problem as well. |
@tparsa I can confirm that it helps, Thanks for the tip. |
we suffer from the exact same issue using K8s 1.23 and on some K8s clusters we have ~10 RedisFailover which are working perfectly fine |
We solved our problem by using network policies denying all connections except the ones we needed. But there is a problem somewhere; this shouldn't be the ultimate solution. |
@tparsa @EladDolev I checked this issue again today, tried forcefully reproducing the issue by setting all redis instances and sentinel instances to be consider a different failover's instance as master. that way, redis operator will be in a state where it wont fix the issue. while we can try updating the operator code to mitigate this, it would be really nice to understand how the redisfailovers ended up in this state in the first place... @EladDolev if you are still having this issue, could you please help by giving some info -
|
I'm still facing this issue whenever we make a node rotation for upgrading, we put an alert to mitigate it manually for now. In my case custom config is only these two:
So I guess deleting multiple different sentinels at the same time causes it? |
1- I've 16 failovers every one of them has 3 sentinels and 3 replicas, total 96 all running in the same namespace. |
Regarding the sentinels and replicas being on the same node, I checked from KSM and it happens to be one of the clusters that joined eachother recently had a sentinel and a replica on the same node before the node rotation but the other cluster didn't. |
sure. we're using some custom configurations - appendonly no
- hz 100
- save ""
- maxmemory 0
- maxmemory-policy noeviction for sentinel - down-after-milliseconds 3000
- failover-timeout 120000 we're running on GKE, and we're currently witnessing this issue happens on a cluster that is running on spots |
Thanks for sharing the details @EladDolev , will take some time to test and get back with code changes, will update here... |
I think the operator should move away from using IP addresses and utilize the feature in redis 7.0 with using hostnames instead that will eliminate a lot of the possible bugs, but that's probably will require a huge rewrite. |
The best thing to do in this case is to use custom ports for individual deployments, and keep them unique per failover deployment. We do this consciously in our redis failover deployments. The other way is to instead of using mymaster for every sentinel may be we use a unique name per deployment. This would require a code change. I see this issue cannot be fixed by any code change on the operator side as the sentinels inherent property is to remember it's redis. Your best shot currently is to use custom, unique, port. But if you are using default port the second option wherein the code change is required to replace the mymaster with a unique name per redisfailover deployment. |
On second thought we should fix the mymaster thing in any case. |
This does not help our case. |
Different ports will fix it I'm not saying it won't but we're not finding the root cause. |
I'm pretty sure I never saw this issue in v1.0 there's definitely a regression happened somewhere. |
One way that fixes the issue but is not a good way to fix it is to delete all sentinel pods and simultaneously set a master in RedisFailover. Then when the sentinels are running, the operator will set the right pod to monitor. |
We just recently had a case where two sentinels "joined together". There was some event on Kubernetes that caused a bunch of Redis and Sentinel pods to reschedule (ie. a node was scaled down). During rescheduling one of the Redis pod IP addresses was reused by a Redis pod in another cluster. In the sentinel logs for cluster A, ther was this:
Here the master There was also a corresponding log entry in the Redis Operator logs, that points this to
(note: should really log the IP there as well). Looking at It seems that setting different auth credentials to different clusters would mitigate this. Possibly having different master ID and not using |
It became eaiser to reproduce thanks to @mpihlak discovery. We will need calico cni in order to reuse the IP addresses consistently using the IPPool feature. Start the cluster with minikube using this command Install the operator with whatever way you want. I simply ran Apply the failovers and the IPPools. apiVersion: databases.spotahome.com/v1
kind: RedisFailover
metadata:
name: foo
spec:
sentinel:
replicas: 3
resources:
requests:
cpu: 100m
limits:
memory: 100Mi
redis:
podAnnotations:
'cni.projectcalico.org/ipv4pools': '["new-pool1"]'
replicas: 3
resources:
requests:
cpu: 100m
memory: 100Mi
limits:
cpu: 400m
memory: 500Mi
---
apiVersion: databases.spotahome.com/v1
kind: RedisFailover
metadata:
name: bar
spec:
sentinel:
replicas: 3
resources:
requests:
cpu: 100m
limits:
memory: 100Mi
redis:
podAnnotations:
'cni.projectcalico.org/ipv4pools': '["new-pool2"]'
replicas: 3
resources:
requests:
cpu: 100m
memory: 100Mi
limits:
cpu: 400m
memory: 500Mi
---
apiVersion: crd.projectcalico.org/v1
kind: IPPool
metadata:
name: new-pool1
spec:
blockSize: 30
cidr: 10.0.0.0/30
ipipMode: Never
natOutgoing: true
---
apiVersion: crd.projectcalico.org/v1
kind: IPPool
metadata:
name: new-pool2
spec:
blockSize: 30
cidr: 10.0.0.4/30
ipipMode: Never
natOutgoing: true Now change the annotations in the failovers to point to the other IPPool apiVersion: databases.spotahome.com/v1
kind: RedisFailover
metadata:
name: foo
spec:
sentinel:
replicas: 3
resources:
requests:
cpu: 100m
limits:
memory: 100Mi
redis:
podAnnotations:
'cni.projectcalico.org/ipv4pools': '["new-pool2"]'
replicas: 3
resources:
requests:
cpu: 100m
memory: 100Mi
limits:
cpu: 400m
memory: 500Mi
---
apiVersion: databases.spotahome.com/v1
kind: RedisFailover
metadata:
name: bar
spec:
sentinel:
replicas: 3
resources:
requests:
cpu: 100m
limits:
memory: 100Mi
redis:
podAnnotations:
'cni.projectcalico.org/ipv4pools': '["new-pool1"]'
replicas: 3
resources:
requests:
cpu: 100m
memory: 100Mi
limits:
cpu: 400m
memory: 500Mi
---
apiVersion: crd.projectcalico.org/v1
kind: IPPool
metadata:
name: new-pool1
spec:
blockSize: 30
cidr: 10.0.0.0/30
ipipMode: Never
natOutgoing: true
---
apiVersion: crd.projectcalico.org/v1
kind: IPPool
metadata:
name: new-pool2
spec:
blockSize: 30
cidr: 10.0.0.4/30
ipipMode: Never
natOutgoing: true The operator will start restarting the redis replicas and the sentinels will start to join eachother as the ip addresses will be switched between the two clusters. Note it will happen most of the time but not everytime(for me it happened 4 out of 5) and that proves more that there's a race condition. |
Hi! As I was thinking about using this operator I came across this issue and I felt immediately remembered about the problems I had getting this "fixed" with Google cloud running Redis Sentinel clusters on VMs. We have around 20 of these running and from time to time you of course want to upgrade or install OS updates. Since we do immutable infrastructure we just don't update VMs (via Ansible, Chef, Puppet, ...) but we completely replace them. All our Redis clusters have three instances. So if we update such a cluster we shutdown one Redis node (which always contains one Redis and Sentinel process). We build our OS image with Hashicorp Packer. So our automation picks up the latest OS image built and then a new VM starts and re-joins the cluster. And then this also happens with the other two remaining nodes. We first replace the two replicas and finally the primary. Before shutting down the primary we issue a primary failover. As long as you do this only with one Redis Sentinel cluster it normally works fine. But since the update process runs in parallel all 20 Redis Sentinel cluster are recreated at the same time. All that works just fine. But at the beginning we also discovered that some nodes suddenly tried to join other clusters during that process. All clusters have a different We tried a lot of workarounds but nothing really worked reliable as Redis/Sentinel always "remembered" the IP addresses if the old (now gone) nodes. And that's actually the problem. During that Redis Sentinel clusters recreation process it was likely that one node got an internal IP address that was previously used by a node that belonged to a different Redis Sentinel cluster. So our "solution" was to give every VM node fixed internal IP addresses. So they don't change IP addresses as the IP address is allocated once then is then always assigned to the VM that used it before. That "fixed" this issue once and forever 😃 But AFAIK you can't do this in Kubernetes. So from what I've read so far in this thread using |
This issue is stale because it has been open for 45 days with no activity. |
This issue was closed because it has been inactive for 14 days since being marked as stale. |
Any updates on this? |
I wanted to check if the new 1.3.0-rc1 might solve it but nothing has changed, clusters still join eachother. @ese Can you please take a look at this bug before the stable release? |
In #4, we added a NetworkPolicy. The intent was to prevent the Redis and/or Sentinel pods from differing RedisFailovers from joining up with one another (See: spotahome#550). These policies have proven to be too coarsely grained. We end up deploying supplemental NetworkPolicies to allow ingress traffic. This change narrows the scope of the NetworkPolicies manged by the operator. One policy allows traffic to the Redis node pods ONLY on the redis port and monitoring port for traffic originating from within the namespace. The other policy allows traffic to the Sentinel pods ONLY on the sentinel port for traffic originating from within the namespace. All other traffic to these pods will be dropped. Connections to the HAProxy pods - IE access to the redis master node - will now be allowed by default. This achieves the goals of #4 and allows us to stop littering additional NetworkPolicies to allow external communication with a Redis instance.
We just had this happen as well. Three sentinel clusters joined together, possibly connected to some flaky nodes. Lucklily these were all staging deployments. Otherwise that would have caused critical errors and data loss. |
For everyone facing this problem I suggest to move from this redis-operator to opstree redis-operator. in opstree they have capability to set which sentinel to specific redis server (redis-replication)
this configuration save us. cheers 😃 |
I think a good solution for this would be to migrate from using IP-addresses to hostnames for both Sentinel's and Redis instances. Ie. use Hostname support is documented at: https://redis.io/docs/latest/operate/oss_and_stack/management/sentinel/#ip-addresses-and-dns-names The issues around environments which swap IP-addresses around is sort of documented at https://redis.io/docs/latest/operate/oss_and_stack/management/sentinel/#sentinel-docker-nat-and-possible-issues. But it focuses on Docker environments rather than Kubernetes. We are currently working on a DIY helm template hostname based approach to replace this operator after having a critical data loss in our production environment due to Sentinels getting mixed up and telling random Redis instances from different clusters to start following each other. |
I can recommend Bitnami Redis Helm chart: https://artifacthub.io/packages/helm/bitnami/redis It also supports Redis Sentinel. It works very well for us with 20+ Redis Sentinel deployments in a Kubernetes cluster. The chart handles various issues you can face with Redis Sentinel very well. They've quite a lot Helm charts for other services too: https://github.com/bitnami/charts/tree/main/bitnami |
@githubixx correct better using Bitnami Redis. I got issue in OT-REDIS-OPERATOR regarding failover. the failover doesn't running smoothly. |
Environment
How are the pieces configured?
The text was updated successfully, but these errors were encountered: