migrate experimental-snapshot-catchup-entries flag to snapshot-catchu…

…p-entries

Signed-off-by: Ajay Sundar Karuppasamy <ajaysundar@google.com>

server/embed/config.go

@@ -148,6 +148,7 @@ var (
 		"experimental-bootstrap-defrag-threshold-megabytes": "bootstrap-defrag-threshold-megabytes",
 		"experimental-max-learners":                         "max-learners",
 		"experimental-memory-mlock":                         "memory-mlock",
+		"experimental-snapshot-catchup-entries":             "snapshot-catchup-entries",
 	}
 )
 
@@ -183,12 +184,21 @@ type Config struct {
 	// TODO: remove it in 3.7.
 	SnapshotCount uint64 `json:"snapshot-count"`
 
-	// SnapshotCatchUpEntries is the number of entries for a slow follower
+	// ExperimentalSnapshotCatchUpEntries is the number of entries for a slow follower
 	// to catch-up after compacting the raft storage entries.
 	// We expect the follower has a millisecond level latency with the leader.
 	// The max throughput is around 10K. Keep a 5K entries is enough for helping
 	// follower to catch up.
-	SnapshotCatchUpEntries uint64 `json:"experimental-snapshot-catch-up-entries"`
+	// Deprecated in v3.6 and will be removed in v3.7.
+	// TODO: remove in v3.7.
+	ExperimentalSnapshotCatchUpEntries uint64 `json:"experimental-snapshot-catch-up-entries"`
+
+	// SnapshotCatchUpEntries is the number of entires for a slow follower
+	// to catch-up after compacting the raft storage entries.
+	// We expect the follower has a millisecond level latency with the leader.
+	// The max throughput is around 10K. Keep a 5K entries is enough for helping
+	// follower to catch up.
+	SnapshotCatchUpEntries uint64 `json:"snapshot-catchup-entries"`
 
 	// MaxSnapFiles is deprecated in v3.6 and will be decommissioned in v3.7.
 	// TODO: remove it in 3.7.
@@ -573,8 +583,9 @@ func NewConfig() *Config {
 
 		Name: DefaultName,
 
-		SnapshotCount:          etcdserver.DefaultSnapshotCount,
-		SnapshotCatchUpEntries: etcdserver.DefaultSnapshotCatchUpEntries,
+		SnapshotCount:                      etcdserver.DefaultSnapshotCount,
+		ExperimentalSnapshotCatchUpEntries: etcdserver.DefaultSnapshotCatchUpEntries,
+		SnapshotCatchUpEntries:             etcdserver.DefaultSnapshotCatchUpEntries,
 
 		MaxTxnOps:            DefaultMaxTxnOps,
 		MaxRequestBytes:      DefaultMaxRequestBytes,
@@ -861,7 +872,8 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
 	// TODO: delete in v3.7
 	fs.IntVar(&cfg.ExperimentalMaxLearners, "experimental-max-learners", membership.DefaultMaxLearners, "Sets the maximum number of learners that can be available in the cluster membership. Deprecated in v3.6 and will be decommissioned in v3.7. Use --max-learners instead.")
 	fs.IntVar(&cfg.MaxLearners, "max-learners", membership.DefaultMaxLearners, "Sets the maximum number of learners that can be available in the cluster membership.")
-	fs.Uint64Var(&cfg.SnapshotCatchUpEntries, "experimental-snapshot-catchup-entries", cfg.SnapshotCatchUpEntries, "Number of entries for a slow follower to catch up after compacting the raft storage entries.")
+	fs.Uint64Var(&cfg.ExperimentalSnapshotCatchUpEntries, "experimental-snapshot-catchup-entries", cfg.ExperimentalSnapshotCatchUpEntries, "Number of entries for a slow follower to catch up after compacting the raft storage entries. Deprecated in v3.6 and will be decommissioned in v3.7. Use --snapshot-catchup-entries instead.")
+	fs.Uint64Var(&cfg.SnapshotCatchUpEntries, "snapshot-catchup-entries", cfg.SnapshotCatchUpEntries, "Number of entries for a slow follower to catch up after compacting the raft storage entries.")
 
 	// unsafe
 	fs.BoolVar(&cfg.UnsafeNoFsync, "unsafe-no-fsync", false, "Disables fsync, unsafe, will cause data loss.")
@@ -910,6 +922,15 @@ func (cfg *configYAML) configFromFile(path string) error {
 		cfg.FlagsExplicitlySet[flg] = true
 	}
 
+	// attempt to fix a bug introduced in https://github.com/etcd-io/etcd/pull/15033
+	// both `experimental-snapshot-catch-up-entries` and `experimental-snapshot-catchup-entries` refer to the same field,
+	// 	map the YAML field "experimental-snapshot-catch-up-entries" to the flag "experimental-snapshot-catchup-entries".
+	if val, ok := cfgMap["experimental-snapshot-catch-up-entries"]; ok {
+		cfgMap["experimental-snapshot-catchup-entries"] = val
+		cfg.ExperimentalSnapshotCatchUpEntries = uint64(val.(float64))
+		cfg.FlagsExplicitlySet["experimental-snapshot-catchup-entries"] = true
+	}
+
 	getBoolFlagVal := func(flagName string) *bool {
 		flagVal, ok := cfgMap[flagName]
 		if !ok {

server/etcdmain/config.go

@@ -72,6 +72,7 @@ var (
 		"experimental-bootstrap-defrag-threshold-megabytes": "--experimental-bootstrap-defrag-threshold-megabytes is deprecated in v3.6 and will be decommissioned in v3.7. Use '--bootstrap-defrag-threshold-megabytes' instead.",
 		"experimental-max-learners":                         "--experimental-max-learners is deprecated in v3.6 and will be decommissioned in v3.7. Use '--max-learners' instead.",
 		"experimental-memory-mlock":                         "--experimental-memory-mlock is deprecated in v3.6 and will be decommissioned in v3.7. Use '--memory-mlock' instead.",
+		"experimental-snapshot-catchup-entries":             "--experimental-snapshot-catchup-entries is deprecated in v3.6 and will be decommissioned in v3.7. Use '--snapshot-catchup-entries' instead.",
 	}
 )
 
@@ -209,6 +210,10 @@ func (cfg *config) parse(arguments []string) error {
 		cfg.ec.MemoryMlock = cfg.ec.ExperimentalMemoryMlock
 	}
 
+	if cfg.ec.FlagsExplicitlySet["experimental-snapshot-catchup-entries"] {
+		cfg.ec.SnapshotCatchUpEntries = cfg.ec.ExperimentalSnapshotCatchUpEntries
+	}
+
 	// `V2Deprecation` (--v2-deprecation) is deprecated and scheduled for removal in v3.8. The default value is enforced, ignoring user input.
 	cfg.ec.V2Deprecation = cconfig.V2DeprDefault
 

server/etcdmain/config_test.go

@@ -32,6 +32,7 @@ import (
 	"go.etcd.io/etcd/pkg/v3/featuregate"
 	"go.etcd.io/etcd/pkg/v3/flags"
 	"go.etcd.io/etcd/server/v3/embed"
+	"go.etcd.io/etcd/server/v3/etcdserver"
 	"go.etcd.io/etcd/server/v3/features"
 )
 
@@ -66,7 +67,7 @@ func TestConfigFileMemberFields(t *testing.T) {
 		MaxWALFiles            uint   `json:"max-wals"`
 		Name                   string `json:"name"`
 		SnapshotCount          uint64 `json:"snapshot-count"`
-		SnapshotCatchUpEntries uint64 `json:"experimental-snapshot-catch-up-entries"`
+		SnapshotCatchUpEntries uint64 `json:"snapshot-catchup-entries"`
 		ListenPeerURLs         string `json:"listen-peer-urls"`
 		ListenClientURLs       string `json:"listen-client-urls"`
 		ListenClientHTTPURLs   string `json:"listen-client-http-urls"`
@@ -320,6 +321,73 @@ func TestConfigParsingMissedAdvertiseClientURLsFlag(t *testing.T) {
 	}
 }
 
+// TestExperimentalSnapshotCatchUpEntriesFlagMigration tests the migration from
+// --experimental-snapshot-catch-up-entries to --snapshot-catch-up-entries
+func TestExperimentalSnapshotCatchUpEntriesFlagMigration(t *testing.T) {
+	testCases := []struct {
+		name                               string
+		snapshotCatchUpEntries             uint64
+		experimentalSnapshotCatchUpEntries uint64
+		wantErr                            bool
+		wantConfig                         uint64
+	}{
+		{
+			name:       "default",
+			wantConfig: etcdserver.DefaultSnapshotCatchUpEntries,
+		},
+		{
+			name:                               "cannot set both experimental flag and non experimental flag",
+			experimentalSnapshotCatchUpEntries: 1000,
+			snapshotCatchUpEntries:             2000,
+			wantErr:                            true,
+		},
+		{
+			name:                               "can set experimental flag",
+			experimentalSnapshotCatchUpEntries: 1000,
+			wantConfig:                         1000,
+		},
+		{
+			name:                   "can set non-experimental flag",
+			snapshotCatchUpEntries: 2000,
+			wantConfig:             2000,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cmdLineArgs := []string{}
+			yc := struct {
+				ExperimentalSnapshotCatchUpEntries uint64 `json:"experimental-snapshot-catch-up-entries,omitempty"`
+				SnapshotCatchUpEntries             uint64 `json:"snapshot-catchup-entries,omitempty"`
+			}{}
+
+			if tc.snapshotCatchUpEntries > 0 {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--snapshot-catchup-entries=%d", tc.snapshotCatchUpEntries))
+				yc.SnapshotCatchUpEntries = tc.snapshotCatchUpEntries
+			}
+
+			if tc.experimentalSnapshotCatchUpEntries > 0 {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-snapshot-catchup-entries=%d", tc.experimentalSnapshotCatchUpEntries))
+				yc.ExperimentalSnapshotCatchUpEntries = tc.experimentalSnapshotCatchUpEntries
+			}
+
+			cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs)
+
+			if tc.wantErr {
+				if errFromCmdLine == nil || errFromFile == nil {
+					t.Fatal("expect parse error")
+				}
+				return
+			}
+			if errFromCmdLine != nil || errFromFile != nil {
+				t.Fatal("error parsing config")
+			}
+
+			require.Equal(t, tc.wantConfig, cfgFromCmdLine.ec.SnapshotCatchUpEntries)
+			require.Equal(t, tc.wantConfig, cfgFromFile.ec.SnapshotCatchUpEntries)
+		})
+	}
+}
+
 func TestConfigIsNewCluster(t *testing.T) {
 	tests := []struct {
 		state  string
@@ -1131,6 +1199,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 		ExperimentalWarningApplyDuration              time.Duration `json:"experimental-warning-apply-duration,omitempty"`
 		ExperimentalBootstrapDefragThresholdMegabytes uint          `json:"experimental-bootstrap-defrag-threshold-megabytes,omitempty"`
 		ExperimentalMaxLearners                       int           `json:"experimental-max-learners,omitempty"`
+		ExperimentalSnapshotCatchUpEntries            uint64        `json:"experimental-snapshot-catch-up-entries,omitempty"`
 	}
 
 	testCases := []struct {
@@ -1155,6 +1224,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 				ExperimentalWarningApplyDuration:              3 * time.Minute,
 				ExperimentalBootstrapDefragThresholdMegabytes: 100,
 				ExperimentalMaxLearners:                       1,
+				ExperimentalSnapshotCatchUpEntries:            1000,
 			},
 			expectedFlags: map[string]struct{}{
 				"experimental-compact-hash-check-enabled":           {},
@@ -1165,6 +1235,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 				"experimental-warning-apply-duration":               {},
 				"experimental-bootstrap-defrag-threshold-megabytes": {},
 				"experimental-max-learners":                         {},
+				"experimental-snapshot-catchup-entries":             {},
 			},
 		},
 		{

server/etcdmain/help.go

@@ -326,6 +326,8 @@ Experimental feature:
   --experimental-memory-mlock
     Enable to enforce etcd pages (in particular bbolt) to stay in RAM. Deprecated in v3.6 and will be decommissioned in v3.7. Use '--memory-mlock' instead.
   --experimental-snapshot-catchup-entries
+    Number of entries for a slow follower to catch up after compacting the raft storage entries. Deprecated in v3.6 and will be decommissioned in v3.7. Use '--snapshot-catchup-entries' instead.
+  --snapshot-catchup-entries
     Number of entries for a slow follower to catch up after compacting the raft storage entries.
   --experimental-stop-grpc-service-on-defrag
     Enable etcd gRPC service to stop serving client requests on defragmentation. It's deprecated, and will be decommissioned in v3.7. Use '--feature-gates=StopGRPCServiceOnDefrag=true' instead.