fix(sentinel): do not close sentinel when replica list is empty (#3795)

* fix: do not close sentinel when replicas list is empty

When a sentinel-monitored Redis master has no replicas (master-only
setup), replicaAddrss() was calling closeSentinel() on receiving an
empty replica list without error. This destroyed the sentinel
connection (and its pub/sub failover subscription), forcing a full
sentinel rediscovery on every subsequent operation — creating a
continuous rediscovery loop that flooded logs and added latency.

The empty-replica case is a valid, stable state for master-only
deployments. The sentinel connection must be preserved for ongoing
master discovery and failover monitoring via pub/sub.

The fix returns early with an empty slice instead of tearing down
the sentinel. A test is added to verify the sentinel survives
repeated ReplicaAddrs calls when no replicas exist.

Fixes: sentinel rediscovery loop on master-only clusters

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: preserve UseDisconnectedReplicas fallback path

Address review feedback: the early return for zero replicas must only
apply when useDisconnected=false (the normal path). When
useDisconnected=true, fall through to the discovery loop which
correctly passes useDisconnected to parseReplicaAddrs — since
getReplicaAddrs hardcodes false, the cached sentinel path can never
find disconnected replicas.

Also remove unused SetSentinelForTest helper.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: use SENTINEL MONITOR to create a true zero-replica master

Address review feedback: the previous test used sentinelName which
has two replicas in the test environment, so getReplicaAddrs returned
a non-empty list and the zero-replica code path was never exercised.

Now the test registers a temporary master-only name via SENTINEL
MONITOR (pointing to the existing master port but with a new name
and no replicas), verifies that ReplicaAddrs returns empty without
closing the sentinel, and cleans up via SENTINEL REMOVE.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: skip zero-replica sentinel test when infrastructure unavailable

Add a connectivity check at the start of the test so it is skipped
(not failed) in environments where sentinel processes are not running.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: use standalone Redis (no replicas) for zero-replica sentinel test

Point SENTINEL MONITOR at the standalone Redis instance (redisPort)
instead of sentinelMasterPort, which already has two replicas. This
prevents Sentinel from discovering replicas via INFO REPLICATION and
ensures the test reliably exercises the zero-replica code path.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: re-trigger checks

* ci: re-trigger checks (flaky PubSub ClientSetName and conn_state tests)

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Nedyalko Dyakov <[email protected]>

export_test.go

@@ -106,3 +106,27 @@ func (c *ModuleLoadexConfig) ToArgs() []interface{} {
 func ShouldRetry(err error, retryTimeout bool) bool {
 	return shouldRetry(err, retryTimeout)
 }
+
+// SentinelFailoverHasSentinel returns true if the sentinelFailover
+// still holds a live sentinel connection. Used in tests to verify
+// that replicaAddrs does not tear down the sentinel for master-only
+// (zero-replica) setups.
+func (c *sentinelFailover) HasSentinel() bool {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+	return c.sentinel != nil
+}
+
+// NewTestSentinelFailover creates a sentinelFailover with minimal
+// options for unit testing. The caller must set c.sentinel manually.
+func NewTestSentinelFailover(opt *FailoverOptions, sentinelAddrs []string) *sentinelFailover {
+	return &sentinelFailover{
+		opt:           opt,
+		sentinelAddrs: sentinelAddrs,
+	}
+}
+
+// ReplicaAddrs is exported for testing.
+func (c *sentinelFailover) ReplicaAddrs(ctx context.Context) ([]string, error) {
+	return c.replicaAddrs(ctx, false)
+}

sentinel.go

@@ -1016,8 +1016,16 @@ func (c *sentinelFailover) replicaAddrs(ctx context.Context, useDisconnected boo
 				c.opt.MasterName, err)
 		} else if len(addrs) > 0 {
 			return addrs, nil
+		} else if !useDisconnected {
+			// No error and no replicas — valid steady state for master-only setups.
+			// Preserve the sentinel connection for master discovery and failover
+			// pub/sub monitoring. Only return early when useDisconnected is false;
+			// when true, fall through to the discovery loop which passes
+			// useDisconnected to parseReplicaAddrs (getReplicaAddrs hardcodes false).
+			return []string{}, nil
 		} else {
-			// No error and no replicas.
+			// useDisconnected=true: close sentinel so the discovery loop can call
+			// setSentinel if it finds disconnected replicas.
 			_ = c.closeSentinel()
 		}
 	}

sentinel_test.go

@@ -855,3 +855,97 @@ var _ = Describe("Sentinel Failover with Conns", Serial, Ordered, func() {
 		fmt.Println("Sentinel cluster is now stable")
 	})
 })
+
+// TestSentinelFailover_ReplicaAddrs_NoReplicas verifies that when a
+// sentinel-monitored master has zero replicas, the sentinel connection
+// is preserved and not torn down. Before the fix, closeSentinel() was
+// called when getReplicaAddrs returned an empty list without error,
+// causing a continuous rediscovery loop on every subsequent operation.
+func TestSentinelFailover_ReplicaAddrs_NoReplicas(t *testing.T) {
+	ctx := context.Background()
+
+	// Skip if sentinel infrastructure is not available.
+	sentinel0 := redis.NewSentinelClient(&redis.Options{Addr: sentinelAddrs[0], DialTimeout: 2 * time.Second})
+	if err := sentinel0.Ping(ctx).Err(); err != nil {
+		sentinel0.Close()
+		t.Skipf("sentinel not available at %s: %v", sentinelAddrs[0], err)
+	}
+	sentinel0.Close()
+
+	// Register a temporary master-only name pointing at the standalone Redis
+	// instance (redisPort). Unlike sentinelMasterPort which already has two
+	// replicas configured, the standalone instance has none, so Sentinel will
+	// never discover replicas via INFO REPLICATION.
+	masterOnlyName := "master-only-test"
+	masterIP := "127.0.0.1"
+	masterPort := redisPort
+	quorum := "2"
+
+	// Monitor the master-only name on all sentinels.
+	for _, addr := range sentinelAddrs {
+		sentinel := redis.NewSentinelClient(&redis.Options{Addr: addr})
+		err := sentinel.Monitor(ctx, masterOnlyName, masterIP, masterPort, quorum).Err()
+		if err != nil && !strings.Contains(err.Error(), "Duplicated") {
+			t.Fatalf("SENTINEL MONITOR on %s failed: %v", addr, err)
+		}
+		sentinel.Close()
+	}
+
+	// Clean up: remove the monitored master after the test.
+	defer func() {
+		for _, addr := range sentinelAddrs {
+			sentinel := redis.NewSentinelClient(&redis.Options{Addr: addr})
+			_ = sentinel.Remove(ctx, masterOnlyName).Err()
+			sentinel.Close()
+		}
+	}()
+
+	failover := redis.NewTestSentinelFailover(&redis.FailoverOptions{
+		MasterName:    masterOnlyName,
+		SentinelAddrs: sentinelAddrs,
+		DialTimeout:   5 * time.Second,
+		ReadTimeout:   5 * time.Second,
+	}, sentinelAddrs)
+	defer failover.Close()
+
+	// Bootstrap: perform initial master discovery so that a sentinel
+	// client is established (setSentinel is called internally).
+	addr, err := failover.MasterAddr(ctx)
+	if err != nil {
+		t.Fatalf("MasterAddr failed: %v", err)
+	}
+	if addr == "" {
+		t.Fatal("MasterAddr returned empty address")
+	}
+
+	if !failover.HasSentinel() {
+		t.Fatal("expected sentinel to be set after MasterAddr, got nil")
+	}
+
+	// Call ReplicaAddrs. With zero replicas, getReplicaAddrs returns an
+	// empty list without error. Before the fix, this called closeSentinel()
+	// which destroyed the sentinel connection and caused a rediscovery loop.
+	replicas, err := failover.ReplicaAddrs(ctx)
+	if err != nil {
+		t.Fatalf("ReplicaAddrs failed: %v", err)
+	}
+	if len(replicas) != 0 {
+		t.Fatalf("expected zero replicas for master-only setup, got %v", replicas)
+	}
+
+	if !failover.HasSentinel() {
+		t.Fatal("sentinel was closed after ReplicaAddrs returned zero replicas; " +
+			"this causes a continuous rediscovery loop on master-only setups")
+	}
+
+	// Call ReplicaAddrs a second time to confirm the sentinel stays
+	// alive across repeated calls (no degradation over time).
+	_, err = failover.ReplicaAddrs(ctx)
+	if err != nil {
+		t.Fatalf("second ReplicaAddrs call failed: %v", err)
+	}
+
+	if !failover.HasSentinel() {
+		t.Fatal("sentinel was closed after second ReplicaAddrs call")
+	}
+}