RedisCluster all keys lost: Leader&Follower Pods are terminated&updated together when the spec changed

wkd-woo commented 1 month ago

What version of redis operator are you using?

redis-operator version: v0.16.0

Does this issue reproduce with the latest release? I don't check it yet.

What operating system and processor architecture are you using (kubectl version)?

ubuntu22.04.4 LTS / amd64

kubectl version Output

$ kubectl version
Server Version: v1.24.17

What did you do?

I deployed the redis cluster with the redis operator. And we inserted 1000 keys as test data, The upgrade of the redis engine version was performed (v7.0.12 to v7.0.15)

The upgrade was successful, but the pads of the "Leader" statefulSet and "Follower" statefulSet were upgraded to rolling at the same time, and all data was lost.

There are specific changes such as image changes, so it is necessary to preserve data when the rolling update proceeds.

$ kubectl get all

NAME                           READY   STATUS    RESTARTS   AGE
pod/nosql-cluster-follower-0   2/2     Running   0          129m
pod/nosql-cluster-follower-1   2/2     Running   0          129m
pod/nosql-cluster-follower-2   2/2     Running   0          129m
pod/nosql-cluster-leader-0     2/2     Running   0          130m
pod/nosql-cluster-leader-1     2/2     Running   0          130m
pod/nosql-cluster-leader-2     2/2     Running   0          130m

...

NAME                                      READY   AGE
statefulset.apps/nosql-cluster-follower   3/3     129m
statefulset.apps/nosql-cluster-leader     3/3     130m

At first, there are leader-staetfulSet and follower-statefulset.

$ k exec -it pod/nosql-cluster-leader-0 -- redis-cli -c INFO KEYSPACE
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
# Keyspace
db0:keys=339,expires=0,avg_ttl=0

$ k exec -it pod/nosql-cluster-leader-1 -- redis-cli -c INFO KEYSPACE
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
# Keyspace
db0:keys=329,expires=0,avg_ttl=0

$ k exec -it pod/nosql-cluster-leader-2 -- redis-cli -c INFO KEYSPACE
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
# Keyspace
db0:keys=332,expires=0,avg_ttl=0

And they have 3 shards, with 1000 keys

---
redisCluster:
  name: "nosql-cluster"
  clusterSize: 6
  clusterVersion: v7
  persistenceEnabled: false
  image: cr.undercloud.io/redis-on-k8s/redis
  tag: v7.0.12
  imagePullPolicy: IfNotPresent
  imagePullSecrets:

before upgrade, redis v7.0.12

---
redisCluster:
  name: "nosql-cluster"
  clusterSize: 6
  clusterVersion: v7
  persistenceEnabled: false
  image: cr.undercloud.io/redis-on-k8s/redis
  tag: v7.0.15
  imagePullPolicy: IfNotPresent
  imagePullSecrets:

after upgrade, redis v7.0.15

$ helm upgrade nosql-cluster helm/charts/redis-cluster -f redis-cluster.yaml

helm upgrade

$ kubectl get all
NAME                           READY   STATUS        RESTARTS   AGE
pod/nosql-cluster-follower-0   2/2     Running       0          132m
pod/nosql-cluster-follower-1   2/2     Running       0          132m
pod/nosql-cluster-follower-2   2/2     Terminating   0          132m
pod/nosql-cluster-leader-0     2/2     Running       0          134m
pod/nosql-cluster-leader-1     2/2     Running       0          133m
pod/nosql-cluster-leader-2     2/2     Terminating   0          133m

...

NAME                                      READY   AGE
statefulset.apps/nosql-cluster-follower   3/3     132m
statefulset.apps/nosql-cluster-leader     3/3     134m

Leader-2 and Follower-2 are being upgraded (Terminalitig->ContainerCreating) at the same time.

$ kubectl get all
NAME                           READY   STATUS        RESTARTS   AGE
pod/nosql-cluster-follower-0   2/2     Running       0          133m
pod/nosql-cluster-follower-1   2/2     Terminating   0          133m
pod/nosql-cluster-follower-2   2/2     Running       0          21s
pod/nosql-cluster-leader-0     2/2     Running       0          134m
pod/nosql-cluster-leader-1     2/2     Terminating   0          134m
pod/nosql-cluster-leader-2     2/2     Running       0          22s
...

NAME                                      READY   AGE
statefulset.apps/nosql-cluster-follower   3/3     133m
statefulset.apps/nosql-cluster-leader     3/3     134m

Leader-1 and Follower-1 are being upgraded (Terminalitig->ContainerCreating) at the same time.

NAME                           READY   STATUS        RESTARTS   AGE
pod/nosql-cluster-follower-0   2/2     Terminating   0          133m
pod/nosql-cluster-follower-1   2/2     Running       0          18s
pod/nosql-cluster-follower-2   2/2     Running       0          53s
pod/nosql-cluster-leader-0     2/2     Terminating   0          135m
pod/nosql-cluster-leader-1     1/2     Running       0          11s
pod/nosql-cluster-leader-2     2/2     Running       0          54s

...
NAME                                      READY   AGE
statefulset.apps/nosql-cluster-follower   3/3     133m
statefulset.apps/nosql-cluster-leader     2/3     134m

Leader-0 and Follower-0 are being upgraded (Terminalitig->ContainerCreating) at the same time.

$ k exec -it pod/nosql-cluster-leader-0 -- redis-cli -c INFO KEYSPACE
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
# Keyspace

$ k exec -it pod/nosql-cluster-leader-1 -- redis-cli -c INFO KEYSPACE
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
# Keyspace

$ k exec -it pod/nosql-cluster-leader-2 -- redis-cli -c INFO KEYSPACE
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
# Keyspace

All the data has gone.

$ k exec -it pod/nosql-cluster-leader-0 -- redis-cli -c hello
Defaulted container "nosql-cluster-leader" out of: nosql-cluster-leader, redis-exporter
 1) "server"
 2) "redis"
 3) "version"
 4) "7.0.15"
 5) "proto"
 6) (integer) 2
 7) "id"
 8) (integer) 209
 9) "mode"
10) "cluster"
11) "role"
12) "replica"
13) "modules"
14) (empty array)

version upgrade was successful.

What did you expect to see? I hope the Leader, Follower PODs are not removed & updated at the same time. HA must be ensured to ensure data integrity at the shard level.

drivebyer commented 1 month ago

How about enabling PVC to solve this problem?

wkd-woo commented 1 month ago

@drivebyer That's how I intend to respond to the prblem in our actual prduct operating environment. Take persistence with enabling PVC.

However, as you already know, there is a write performance degradation problem, and there is also a potential data integrity problem between master and slave due to asynchronous replication.

(And this is a personal story, it's not easy to manage to the volume level because there lots of Redis sets I manage.)

When the Redis Cluster bootstrapping by the operator, the follower statefulSet build after the leader was builded. Like this, when the update situation, how about making the follower statefulSet upgraded after the leader's upgrade has done?

drivebyer commented 1 month ago

When the Redis Cluster bootstrapping by the operator, the follower statefulSet build after the leader was builded. Like this, when the update situation, how about making the follower statefulSet upgraded after the leader's upgrade has done?

Maybe we could handle it by operator.

wkd-woo commented 3 weeks ago

When the Redis Cluster bootstrapping by the operator, the follower statefulSet build after the leader was builded. Like this, when the update situation, how about making the follower statefulSet upgraded after the leader's upgrade has done?

Maybe we could handle it by operator.

@drivebyer May I ask if you plan to develop a new feature in the operator to solve this problem?

If it seems like it will take time to add features, I think I can contribute to the project to solve this problem.

drivebyer commented 3 weeks ago

@wkd-woo Sure

I believe the main issue lies in this code: https://github.com/OT-CONTAINER-KIT/redis-operator/blob/a207422faccca8773b705e7e69b3db545e457bfe/controllers/rediscluster_controller.go#L118-L136.

After applying the new StatefulSet spec, the code retrieves the StatefulSet immediately, but there may be delays in StatefulSet status changes. Perhaps we could more accurately verify the StatefulSet readiness. For example, by comparing the status.currentRevision and status.updateRevision.

OT-CONTAINER-KIT / redis-operator

RedisCluster all keys lost: Leader&Follower Pods are terminated&updated together when the spec changed #951