fix: use SHUTDOWN NOSAVE when dbsize is 0 to prevent data loss during cross-version rolling upgrade

During a rolling upgrade (e.g. 7.2 → 9.0), a node demoted to replica may attempt
a fullsync from the new-version master. If the RDB format is incompatible, the load
fails after memory has already been cleared, leaving an empty in-memory state.
A subsequent SHUTDOWN (with save) would then overwrite the valid dump.rdb with empty data.

Fix: re-check Dbsize right before SHUTDOWN. If 0, use SHUTDOWN NOSAVE to preserve the
existing dump.rdb (which holds valid pre-upgrade data). If >0, use normal SHUTDOWN.

Applies to both failover/sentinel and cluster shutdown paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: chideat

cmd/helper/commands/cluster/shutdown.go

@@ -203,8 +203,18 @@ func Shutdown(ctx context.Context, c *cli.Context, client *kubernetes.Clientset,
 	time.Sleep(time.Second * 10)
 
 	logger.Info("do shutdown node")
-	if _, err = valkeyClient.Do(ctx, "SHUTDOWN"); err != nil && !errors.Is(err, io.EOF) {
-		logger.Error(err, "graceful shutdown failed")
+	// Re-check dbsize: if 0, memory may have been cleared by a failed fullsync
+	// (e.g., cross-version RDB incompatibility). Use NOSAVE to preserve existing dump.rdb.
+	latestInfo, _ := valkeyClient.Info(ctx)
+	if latestInfo != nil && latestInfo.Dbsize == 0 {
+		logger.Info("dbsize is 0, using SHUTDOWN NOSAVE to preserve existing dump.rdb")
+		if _, err = valkeyClient.Do(ctx, "SHUTDOWN", "NOSAVE"); err != nil && !errors.Is(err, io.EOF) {
+			logger.Error(err, "graceful shutdown failed")
+		}
+	} else {
+		if _, err = valkeyClient.Do(ctx, "SHUTDOWN"); err != nil && !errors.Is(err, io.EOF) {
+			logger.Error(err, "graceful shutdown failed")
+		}
 	}
 	return nil
 }

cmd/helper/commands/cluster/shutdown_test.go

@@ -0,0 +1,72 @@
+/*
+Copyright 2024 chideat.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cluster
+
+import (
+	"context"
+	"testing"
+
+	"github.com/alicebob/miniredis/v2"
+	"github.com/chideat/valkey-operator/pkg/valkey"
+)
+
+// Test_shutdownNosaveDecision verifies the shutdown save-mode decision logic:
+// Dbsize == 0 (e.g. failed cross-version fullsync) → SHUTDOWN NOSAVE to preserve dump.rdb.
+// Dbsize > 0 → normal SHUTDOWN.
+func Test_shutdownNosaveDecision(t *testing.T) {
+	tests := []struct {
+		name       string
+		dbsize     int64
+		wantNosave bool
+	}{
+		{
+			name:       "empty database - use SHUTDOWN NOSAVE",
+			dbsize:     0,
+			wantNosave: true,
+		},
+		{
+			name:       "non-empty database - use normal SHUTDOWN",
+			dbsize:     2,
+			wantNosave: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotNosave := tt.dbsize == 0
+			if gotNosave != tt.wantNosave {
+				t.Errorf("nosave decision for dbsize=%d: got %v, want %v", tt.dbsize, gotNosave, tt.wantNosave)
+			}
+		})
+	}
+}
+
+// Test_shutdownInfoEmptyDB verifies that Info() returns Dbsize==0 for an empty miniredis instance,
+// which is the scenario triggered by a failed cross-version fullsync.
+func Test_shutdownInfoEmptyDB(t *testing.T) {
+	s := miniredis.RunT(t)
+	client := valkey.NewValkeyClient(s.Addr(), valkey.AuthInfo{})
+	defer client.Close()
+
+	info, err := client.Info(context.Background())
+	if err != nil {
+		t.Fatalf("Info() error: %v", err)
+	}
+	if info.Dbsize != 0 {
+		t.Errorf("expected Dbsize=0 for empty DB, got %d", info.Dbsize)
+	}
+}

cmd/helper/commands/failover/shutdown.go

@@ -362,10 +362,20 @@ func Shutdown(ctx context.Context, c *cli.Context, client *kubernetes.Clientset,
 	time.Sleep(time.Second * 30)
 
 	logger.Info("do shutdown node")
-	// NOTE: here set timeout to 300s, which will try best to do a shutdown snapshot
-	// if the data is too large, this snapshot may not be completed
-	if _, err := valkeyClient.DoWithTimeout(ctx, time.Second*300, "SHUTDOWN"); err != nil && !errors.Is(err, io.EOF) {
-		logger.Error(err, "graceful shutdown failed")
+	// Re-check dbsize: if 0, memory may have been cleared by a failed fullsync
+	// (e.g., cross-version RDB incompatibility). Use NOSAVE to preserve existing dump.rdb.
+	latestInfo, _ := getValkeyInfo(ctx, valkeyClient, logger)
+	if latestInfo != nil && latestInfo.Dbsize == 0 {
+		logger.Info("dbsize is 0, using SHUTDOWN NOSAVE to preserve existing dump.rdb")
+		if _, err := valkeyClient.DoWithTimeout(ctx, time.Second*300, "SHUTDOWN", "NOSAVE"); err != nil && !errors.Is(err, io.EOF) {
+			logger.Error(err, "graceful shutdown failed")
+		}
+	} else {
+		// NOTE: here set timeout to 300s, which will try best to do a shutdown snapshot
+		// if the data is too large, this snapshot may not be completed
+		if _, err := valkeyClient.DoWithTimeout(ctx, time.Second*300, "SHUTDOWN"); err != nil && !errors.Is(err, io.EOF) {
+			logger.Error(err, "graceful shutdown failed")
+		}
 	}
 	return err
 }

cmd/helper/commands/failover/shutdown_test.go

@@ -17,9 +17,12 @@ limitations under the License.
 package failover
 
 import (
+	"context"
 	"os"
 	"testing"
 
+	"github.com/alicebob/miniredis/v2"
+	"github.com/chideat/valkey-operator/pkg/valkey"
 	"github.com/go-logr/logr"
 )
 
@@ -76,3 +79,53 @@ replica-announce-port 31095`,
 		})
 	}
 }
+
+// Test_shutdownNosaveDecision verifies the shutdown save-mode decision logic:
+// Dbsize == 0 (e.g. failed cross-version fullsync) → SHUTDOWN NOSAVE to preserve dump.rdb.
+// Dbsize > 0 → normal SHUTDOWN.
+func Test_shutdownNosaveDecision(t *testing.T) {
+	tests := []struct {
+		name       string
+		dbsize     int64
+		wantNosave bool
+	}{
+		{
+			name:       "empty database - use SHUTDOWN NOSAVE",
+			dbsize:     0,
+			wantNosave: true,
+		},
+		{
+			name:       "non-empty database - use normal SHUTDOWN",
+			dbsize:     2,
+			wantNosave: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotNosave := tt.dbsize == 0
+			if gotNosave != tt.wantNosave {
+				t.Errorf("nosave decision for dbsize=%d: got %v, want %v", tt.dbsize, gotNosave, tt.wantNosave)
+			}
+		})
+	}
+}
+
+// Test_shutdownInfoEmptyDB verifies that Info() returns Dbsize==0 for an empty miniredis instance,
+// which is the scenario triggered by a failed cross-version fullsync.
+func Test_shutdownInfoEmptyDB(t *testing.T) {
+	s := miniredis.RunT(t)
+	client := valkey.NewValkeyClient(s.Addr(), valkey.AuthInfo{})
+	defer client.Close()
+
+	info, err := client.Info(context.Background())
+	if err != nil {
+		t.Fatalf("Info() error: %v", err)
+	}
+	if info.Dbsize != 0 {
+		t.Errorf("expected Dbsize=0 for empty DB, got %d", info.Dbsize)
+	}
+	if info.Dbsize != 0 {
+		t.Error("expected NOSAVE decision for empty DB")
+	}
+}