Merge pull request #19298 from gangli113/experimentalFlag

migrate to use max-learners flag

server/config/config.go

@@ -200,8 +200,8 @@ type ServerConfig struct {
 	// consider running defrag during bootstrap. Needs to be set to non-zero value to take effect.
 	BootstrapDefragThresholdMegabytes uint `json:"bootstrap-defrag-threshold-megabytes"`
 
-	// ExperimentalMaxLearners sets a limit to the number of learner members that can exist in the cluster membership.
-	ExperimentalMaxLearners int `json:"experimental-max-learners"`
+	// MaxLearners sets a limit to the number of learner members that can exist in the cluster membership.
+	MaxLearners int `json:"max-learners"`
 
 	// V2Deprecation defines a phase of v2store deprecation process.
 	V2Deprecation V2DeprecationEnum `json:"v2-deprecation"`

server/embed/config.go

@@ -139,6 +139,7 @@ var (
 		"experimental-watch-progress-notify-interval":       "watch-progress-notify-interval",
 		"experimental-warning-apply-duration":               "warning-apply-duration",
 		"experimental-bootstrap-defrag-threshold-megabytes": "bootstrap-defrag-threshold-megabytes",
+		"experimental-max-learners":                         "max-learners",
 	}
 )
 
@@ -421,7 +422,10 @@ type Config struct {
 	// ExperimentalWarningUnaryRequestDuration is deprecated, please use WarningUnaryRequestDuration instead.
 	ExperimentalWarningUnaryRequestDuration time.Duration `json:"experimental-warning-unary-request-duration"`
 	// ExperimentalMaxLearners sets a limit to the number of learner members that can exist in the cluster membership.
+	// Deprecated in v3.6 and will be decommissioned in v3.7.
+	// TODO: Delete in v3.7
 	ExperimentalMaxLearners int `json:"experimental-max-learners"`
+	MaxLearners             int `json:"max-learners"`
 
 	// ForceNewCluster starts a new cluster even if previously started; unsafe.
 	ForceNewCluster bool `json:"force-new-cluster"`
@@ -609,7 +613,9 @@ func NewConfig() *Config {
 		ExperimentalDowngradeCheckTime:      DefaultDowngradeCheckTime,
 		ExperimentalMemoryMlock:             false,
 		ExperimentalStopGRPCServiceOnDefrag: false,
-		ExperimentalMaxLearners:             membership.DefaultMaxLearners,
+		MaxLearners:                         membership.DefaultMaxLearners,
+		// TODO: delete in v3.7
+		ExperimentalMaxLearners: membership.DefaultMaxLearners,
 
 		CompactHashCheckTime: DefaultCompactHashCheckTime,
 		// TODO: delete in v3.7
@@ -825,7 +831,9 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
 	// TODO: delete in v3.7
 	fs.UintVar(&cfg.ExperimentalBootstrapDefragThresholdMegabytes, "experimental-bootstrap-defrag-threshold-megabytes", 0, "Enable the defrag during etcd server bootstrap on condition that it will free at least the provided threshold of disk space. Needs to be set to non-zero value to take effect. It's deprecated, and will be decommissioned in v3.7. Use --bootstrap-defrag-threshold-megabytes instead.")
 	fs.UintVar(&cfg.BootstrapDefragThresholdMegabytes, "bootstrap-defrag-threshold-megabytes", 0, "Enable the defrag during etcd server bootstrap on condition that it will free at least the provided threshold of disk space. Needs to be set to non-zero value to take effect.")
-	fs.IntVar(&cfg.ExperimentalMaxLearners, "experimental-max-learners", membership.DefaultMaxLearners, "Sets the maximum number of learners that can be available in the cluster membership.")
+	// 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.")
 
 	// unsafe

server/embed/etcd.go

@@ -235,7 +235,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
 		WarningUnaryRequestDuration:          cfg.WarningUnaryRequestDuration,
 		ExperimentalMemoryMlock:              cfg.ExperimentalMemoryMlock,
 		BootstrapDefragThresholdMegabytes:    cfg.BootstrapDefragThresholdMegabytes,
-		ExperimentalMaxLearners:              cfg.ExperimentalMaxLearners,
+		MaxLearners:                          cfg.MaxLearners,
 		V2Deprecation:                        cfg.V2DeprecationEffective(),
 		ExperimentalLocalAddress:             cfg.InferLocalAddr(),
 		ServerFeatureGate:                    cfg.ServerFeatureGate,
@@ -386,7 +386,7 @@ func print(lg *zap.Logger, ec Config, sc config.ServerConfig, memberInitialized
 		zap.String("discovery-user", sc.DiscoveryCfg.Auth.Username),
 
 		zap.String("downgrade-check-interval", sc.DowngradeCheckTime.String()),
-		zap.Int("max-learners", sc.ExperimentalMaxLearners),
+		zap.Int("max-learners", sc.MaxLearners),
 
 		zap.String("v2-deprecation", string(ec.V2Deprecation)),
 	)

server/etcdmain/config.go

@@ -70,6 +70,7 @@ var (
 		"experimental-watch-progress-notify-interval":       "--experimental-watch-progress-notify-interval is deprecated in v3.6 and will be decommissioned in v3.7. Use '--watch-progress-notify-interval' instead.",
 		"experimental-warning-apply-duration":               "--experimental-warning-apply-duration is deprecated in v3.6 and will be decommissioned in v3.7. Use '--warning-apply-duration' instead.",
 		"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.",
 	}
 )
 
@@ -199,6 +200,10 @@ func (cfg *config) parse(arguments []string) error {
 		cfg.ec.BootstrapDefragThresholdMegabytes = cfg.ec.ExperimentalBootstrapDefragThresholdMegabytes
 	}
 
+	if cfg.ec.FlagsExplicitlySet["experimental-max-learners"] {
+		cfg.ec.MaxLearners = cfg.ec.ExperimentalMaxLearners
+	}
+
 	// `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

@@ -882,6 +882,70 @@ func TestBootstrapDefragThresholdMegabytesFlagMigration(t *testing.T) {
 	}
 }
 
+// TestMaxLearnersFlagMigration tests the migration from
+// --experimental-max-learners to --max-learners
+// TODO: delete in v3.7
+func TestMaxLearnersFlagMigration(t *testing.T) {
+	testCases := []struct {
+		name                    string
+		maxLearners             int
+		experimentalMaxLearners int
+		expectErr               bool
+		expectedMaxLearners     int
+	}{
+		{
+			name:                    "cannot set both experimental flag and non experimental flag",
+			maxLearners:             1,
+			experimentalMaxLearners: 2,
+			expectErr:               true,
+		},
+		{
+			name:                    "can set experimental flag",
+			experimentalMaxLearners: 2,
+			expectedMaxLearners:     2,
+		},
+		{
+			name:                "can set non experimental flag",
+			maxLearners:         1,
+			expectedMaxLearners: 1,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cmdLineArgs := []string{}
+			yc := struct {
+				ExperimentalMaxLearners int `json:"experimental-max-learners,omitempty"`
+				MaxLearners             int `json:"max-learners,omitempty"`
+			}{}
+
+			if tc.maxLearners != 0 {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--max-learners=%d", tc.maxLearners))
+				yc.MaxLearners = tc.maxLearners
+			}
+
+			if tc.experimentalMaxLearners != 0 {
+				cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-max-learners=%d", tc.experimentalMaxLearners))
+				yc.ExperimentalMaxLearners = tc.experimentalMaxLearners
+			}
+
+			cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs)
+
+			if tc.expectErr {
+				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.expectedMaxLearners, cfgFromCmdLine.ec.MaxLearners)
+			require.Equal(t, tc.expectedMaxLearners, cfgFromFile.ec.MaxLearners)
+		})
+	}
+}
+
 // TODO delete in v3.7
 func generateCfgsFromFileAndCmdLine(t *testing.T, yc any, cmdLineArgs []string) (*config, error, *config, error) {
 	b, err := yaml.Marshal(&yc)
@@ -996,6 +1060,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 		ExperimentalWatchProgressNotifyInterval       time.Duration `json:"experimental-watch-progress-notify-interval,omitempty"`
 		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"`
 	}
 
 	testCases := []struct {
@@ -1019,6 +1084,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 				ExperimentalWatchProgressNotifyInterval:       3 * time.Minute,
 				ExperimentalWarningApplyDuration:              3 * time.Minute,
 				ExperimentalBootstrapDefragThresholdMegabytes: 100,
+				ExperimentalMaxLearners:                       1,
 			},
 			expectedFlags: map[string]struct{}{
 				"experimental-compact-hash-check-enabled":           {},
@@ -1028,6 +1094,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
 				"experimental-watch-progress-notify-interval":       {},
 				"experimental-warning-apply-duration":               {},
 				"experimental-bootstrap-defrag-threshold-megabytes": {},
+				"experimental-max-learners":                         {},
 			},
 		},
 		{

server/etcdmain/help.go

@@ -310,6 +310,8 @@ Experimental feature:
   --experimental-warning-unary-request-duration '300ms'
     Set time duration after which a warning is generated if a unary request takes more than this duration. It's deprecated, and will be decommissioned in v3.7. Use --warning-unary-request-duration instead.
   --experimental-max-learners '1'
+    Set the max number of learner members allowed in the cluster membership. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'max-learners' instead.
+  --max-learners '1'
     Set the max number of learner members allowed in the cluster membership.
   --experimental-snapshot-catch-up-entries '5000'
     Number of entries for a slow follower to catch up after compacting the raft storage entries.

server/etcdserver/bootstrap.go

@@ -290,7 +290,7 @@ func bootstrapExistingClusterNoWAL(cfg config.ServerConfig, prt http.RoundTrippe
 	if err := cfg.VerifyJoinExisting(); err != nil {
 		return nil, err
 	}
-	cl, err := membership.NewClusterFromURLsMap(cfg.Logger, cfg.InitialClusterToken, cfg.InitialPeerURLsMap, membership.WithMaxLearners(cfg.ExperimentalMaxLearners))
+	cl, err := membership.NewClusterFromURLsMap(cfg.Logger, cfg.InitialClusterToken, cfg.InitialPeerURLsMap, membership.WithMaxLearners(cfg.MaxLearners))
 	if err != nil {
 		return nil, err
 	}
@@ -305,7 +305,7 @@ func bootstrapExistingClusterNoWAL(cfg config.ServerConfig, prt http.RoundTrippe
 		return nil, fmt.Errorf("incompatible with current running cluster")
 	}
 	scaleUpLearners := false
-	if err := membership.ValidateMaxLearnerConfig(cfg.ExperimentalMaxLearners, existingCluster.Members(), scaleUpLearners); err != nil {
+	if err := membership.ValidateMaxLearnerConfig(cfg.MaxLearners, existingCluster.Members(), scaleUpLearners); err != nil {
 		return nil, err
 	}
 	remotes := existingCluster.Members()
@@ -322,7 +322,7 @@ func bootstrapNewClusterNoWAL(cfg config.ServerConfig, prt http.RoundTripper) (*
 	if err := cfg.VerifyBootstrap(); err != nil {
 		return nil, err
 	}
-	cl, err := membership.NewClusterFromURLsMap(cfg.Logger, cfg.InitialClusterToken, cfg.InitialPeerURLsMap, membership.WithMaxLearners(cfg.ExperimentalMaxLearners))
+	cl, err := membership.NewClusterFromURLsMap(cfg.Logger, cfg.InitialClusterToken, cfg.InitialPeerURLsMap, membership.WithMaxLearners(cfg.MaxLearners))
 	if err != nil {
 		return nil, err
 	}
@@ -350,7 +350,7 @@ func bootstrapNewClusterNoWAL(cfg config.ServerConfig, prt http.RoundTripper) (*
 		if config.CheckDuplicateURL(urlsmap) {
 			return nil, fmt.Errorf("discovery cluster %s has duplicate url", urlsmap)
 		}
-		if cl, err = membership.NewClusterFromURLsMap(cfg.Logger, cfg.InitialClusterToken, urlsmap, membership.WithMaxLearners(cfg.ExperimentalMaxLearners)); err != nil {
+		if cl, err = membership.NewClusterFromURLsMap(cfg.Logger, cfg.InitialClusterToken, urlsmap, membership.WithMaxLearners(cfg.MaxLearners)); err != nil {
 			return nil, err
 		}
 	}
@@ -372,10 +372,10 @@ func bootstrapClusterWithWAL(cfg config.ServerConfig, meta *snapshotMetadata) (*
 			zap.String("wal-dir", cfg.WALDir()),
 		)
 	}
-	cl := membership.NewCluster(cfg.Logger, membership.WithMaxLearners(cfg.ExperimentalMaxLearners))
+	cl := membership.NewCluster(cfg.Logger, membership.WithMaxLearners(cfg.MaxLearners))
 
 	scaleUpLearners := false
-	if err := membership.ValidateMaxLearnerConfig(cfg.ExperimentalMaxLearners, cl.Members(), scaleUpLearners); err != nil {
+	if err := membership.ValidateMaxLearnerConfig(cfg.MaxLearners, cl.Members(), scaleUpLearners); err != nil {
 		return nil, err
 	}
 
@@ -457,7 +457,7 @@ func (c *bootstrappedCluster) Finalize(cfg config.ServerConfig, s *bootstrappedS
 		}
 	}
 	scaleUpLearners := false
-	return membership.ValidateMaxLearnerConfig(cfg.ExperimentalMaxLearners, c.cl.Members(), scaleUpLearners)
+	return membership.ValidateMaxLearnerConfig(cfg.MaxLearners, c.cl.Members(), scaleUpLearners)
 }
 
 func (c *bootstrappedCluster) databaseFileMissing(s *bootstrappedStorage) bool {

server/etcdserver/bootstrap_test.go

@@ -94,10 +94,10 @@ func TestBootstrapExistingClusterNoWALMaxLearner(t *testing.T) {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			cfg := config.ServerConfig{
-				Name:                    "node0",
-				InitialPeerURLsMap:      cluster,
-				Logger:                  zaptest.NewLogger(t),
-				ExperimentalMaxLearners: tt.maxLearner,
+				Name:               "node0",
+				InitialPeerURLsMap: cluster,
+				Logger:             zaptest.NewLogger(t),
+				MaxLearners:        tt.maxLearner,
 			}
 			_, err = bootstrapExistingClusterNoWAL(cfg, mockBootstrapRoundTrip(tt.members))
 			hasError := err != nil

tests/framework/integration/cluster.go

@@ -172,7 +172,7 @@ type ClusterConfig struct {
 	LeaseCheckpointPersist  bool
 
 	WatchProgressNotifyInterval time.Duration
-	ExperimentalMaxLearners     int
+	MaxLearners                 int
 	DisableStrictReconfigCheck  bool
 	CorruptCheckTime            time.Duration
 }
@@ -289,7 +289,7 @@ func (c *Cluster) MustNewMember(t testutil.TB) *Member {
 			LeaseCheckpointInterval:     c.Cfg.LeaseCheckpointInterval,
 			LeaseCheckpointPersist:      c.Cfg.LeaseCheckpointPersist,
 			WatchProgressNotifyInterval: c.Cfg.WatchProgressNotifyInterval,
-			ExperimentalMaxLearners:     c.Cfg.ExperimentalMaxLearners,
+			MaxLearners:                 c.Cfg.MaxLearners,
 			DisableStrictReconfigCheck:  c.Cfg.DisableStrictReconfigCheck,
 			CorruptCheckTime:            c.Cfg.CorruptCheckTime,
 		})
@@ -614,7 +614,7 @@ type MemberConfig struct {
 	LeaseCheckpointInterval     time.Duration
 	LeaseCheckpointPersist      bool
 	WatchProgressNotifyInterval time.Duration
-	ExperimentalMaxLearners     int
+	MaxLearners                 int
 	DisableStrictReconfigCheck  bool
 	CorruptCheckTime            time.Duration
 }
@@ -727,9 +727,9 @@ func MustNewMember(t testutil.TB, mcfg MemberConfig) *Member {
 	}
 	m.WarningApplyDuration = embed.DefaultWarningApplyDuration
 	m.WarningUnaryRequestDuration = embed.DefaultWarningUnaryRequestDuration
-	m.ExperimentalMaxLearners = membership.DefaultMaxLearners
-	if mcfg.ExperimentalMaxLearners != 0 {
-		m.ExperimentalMaxLearners = mcfg.ExperimentalMaxLearners
+	m.MaxLearners = membership.DefaultMaxLearners
+	if mcfg.MaxLearners != 0 {
+		m.MaxLearners = mcfg.MaxLearners
 	}
 	m.V2Deprecation = config.V2_DEPR_DEFAULT
 	m.GRPCServerRecorder = &grpctesting.GRPCRecorder{}

tests/integration/clientv3/cluster_test.go

@@ -383,7 +383,7 @@ func TestMaxLearnerInCluster(t *testing.T) {
 	integration2.BeforeTest(t, integration2.WithFailpoint("raftBeforeAdvance", `sleep(100)`))
 
 	// 1. start with a cluster with 3 voting member and max learner 2
-	clus := integration2.NewCluster(t, &integration2.ClusterConfig{Size: 3, ExperimentalMaxLearners: 2, DisableStrictReconfigCheck: true})
+	clus := integration2.NewCluster(t, &integration2.ClusterConfig{Size: 3, MaxLearners: 2, DisableStrictReconfigCheck: true})
 	defer clus.Terminate(t)
 
 	// 2. adding 2 learner members should succeed