From e97bbcddd18ff4146742d9f249245b9c2c5f0a2d Mon Sep 17 00:00:00 2001 From: kizuna-lek Date: Mon, 23 Dec 2024 15:39:22 +0800 Subject: [PATCH 1/3] fix: redis get role overtime --- pkg/lorry/engines/redis/get_replica_role.go | 16 ++++++++++++---- pkg/lorry/engines/redis/manager.go | 8 +++----- pkg/lorry/engines/redis/redis.go | 8 ++++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pkg/lorry/engines/redis/get_replica_role.go b/pkg/lorry/engines/redis/get_replica_role.go index 2f734a55ef9..a0359c97729 100644 --- a/pkg/lorry/engines/redis/get_replica_role.go +++ b/pkg/lorry/engines/redis/get_replica_role.go @@ -34,10 +34,8 @@ func (mgr *Manager) GetReplicaRole(ctx context.Context, _ *dcs.Cluster) (string, return mgr.role, nil } - // We use the role obtained from Sentinel as the sole source of truth. - masterAddr, err := mgr.sentinelClient.GetMasterAddrByName(ctx, mgr.ClusterCompName).Result() - if err != nil { - // when we can't get role from sentinel, we query redis instead + // when we can't get role from sentinel, we query redis instead + getRoleFromRedisClient := func() (string, error) { var role string result, err := mgr.client.Info(ctx, "Replication").Result() if err != nil { @@ -61,6 +59,16 @@ func (mgr *Manager) GetReplicaRole(ctx context.Context, _ *dcs.Cluster) (string, } } + if mgr.sentinelClient == nil { + return getRoleFromRedisClient() + } + + // We use the role obtained from Sentinel as the sole source of truth. + masterAddr, err := mgr.sentinelClient.GetMasterAddrByName(ctx, mgr.ClusterCompName).Result() + if err != nil { + return getRoleFromRedisClient() + } + masterName := strings.Split(masterAddr[0], ".")[0] // if current member is not master from sentinel, just return secondary to avoid double master if masterName != mgr.CurrentMemberName { diff --git a/pkg/lorry/engines/redis/manager.go b/pkg/lorry/engines/redis/manager.go index e37bded9ac3..d2c8e70737a 100644 --- a/pkg/lorry/engines/redis/manager.go +++ b/pkg/lorry/engines/redis/manager.go @@ -44,8 +44,6 @@ type Manager struct { clientSettings *Settings sentinelClient *redis.SentinelClient - ctx context.Context - cancel context.CancelFunc startAt time.Time role string roleSubscribeUpdateTime int64 @@ -90,10 +88,10 @@ func NewManager(properties engines.Properties) (engines.DBManager, error) { } mgr.sentinelClient = newSentinelClient(mgr.clientSettings, mgr.ClusterCompName) + if mgr.sentinelClient != nil { + go mgr.SubscribeRoleChange(context.Background()) + } - mgr.ctx, mgr.cancel = context.WithCancel(context.Background()) - - go mgr.SubscribeRoleChange(mgr.ctx) return mgr, nil } diff --git a/pkg/lorry/engines/redis/redis.go b/pkg/lorry/engines/redis/redis.go index bf8a5ff76ec..c106d87aefb 100644 --- a/pkg/lorry/engines/redis/redis.go +++ b/pkg/lorry/engines/redis/redis.go @@ -145,7 +145,15 @@ func newClient(s *Settings) redis.UniversalClient { } func newSentinelClient(s *Settings, clusterCompName string) *redis.SentinelClient { + if !viper.IsSet("SENTINEL_COMPONENT_NAME") { + // cluster has no sentinel + return nil + } + sentinelHost := fmt.Sprintf("%s-sentinel-headless", clusterCompName) + if viper.IsSet("SENTINEL_HEADLESS_SERVICE_NAME") { + sentinelHost = viper.GetString("SENTINEL_HEADLESS_SERVICE_NAME") + } sentinelPort := "26379" if viper.IsSet("REDIS_SENTINEL_HOST_NETWORK_PORT") { sentinelPort = viper.GetString("REDIS_SENTINEL_HOST_NETWORK_PORT") From 2855b0ad80c6d93401bd86b107862d31050047dd Mon Sep 17 00:00:00 2001 From: kizuna-lek Date: Mon, 23 Dec 2024 15:45:20 +0800 Subject: [PATCH 2/3] add test log --- pkg/lorry/engines/redis/get_replica_role.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/lorry/engines/redis/get_replica_role.go b/pkg/lorry/engines/redis/get_replica_role.go index a0359c97729..93ed315a321 100644 --- a/pkg/lorry/engines/redis/get_replica_role.go +++ b/pkg/lorry/engines/redis/get_replica_role.go @@ -40,6 +40,8 @@ func (mgr *Manager) GetReplicaRole(ctx context.Context, _ *dcs.Cluster) (string, result, err := mgr.client.Info(ctx, "Replication").Result() if err != nil { mgr.Logger.Info("Role query failed", "error", err.Error()) + // fixme: test log + mgr.Logger.Info("ctx info", "ctx error", ctx.Err()) return role, err } else { // split the result into lines From 3e8d2ac032520d382421a28bde4206d73df32511 Mon Sep 17 00:00:00 2001 From: kizuna-lek Date: Tue, 24 Dec 2024 16:30:54 +0800 Subject: [PATCH 3/3] delete test log --- pkg/lorry/engines/redis/get_replica_role.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/lorry/engines/redis/get_replica_role.go b/pkg/lorry/engines/redis/get_replica_role.go index 93ed315a321..a0359c97729 100644 --- a/pkg/lorry/engines/redis/get_replica_role.go +++ b/pkg/lorry/engines/redis/get_replica_role.go @@ -40,8 +40,6 @@ func (mgr *Manager) GetReplicaRole(ctx context.Context, _ *dcs.Cluster) (string, result, err := mgr.client.Info(ctx, "Replication").Result() if err != nil { mgr.Logger.Info("Role query failed", "error", err.Error()) - // fixme: test log - mgr.Logger.Info("ctx info", "ctx error", ctx.Err()) return role, err } else { // split the result into lines