migrate experimental-compact-hash-check-enabled to feature gate.

Signed-off-by: Siyuan Zhang <[email protected]>

pkg/featuregate/feature_gate.go

@@ -91,6 +91,8 @@ type FeatureGate interface {
 	// set on the copy without mutating the original. This is useful for validating
 	// config against potential feature gate changes before committing those changes.
 	DeepCopy() MutableFeatureGate
+	// String returns a string containing all enabled feature gates, formatted as "key1=value1,key2=value2,...".
+	String() string
 }
 
 // MutableFeatureGate parses and stores flag gates for known features from

server/config/config.go

@@ -147,10 +147,9 @@ type ServerConfig struct {
 
 	// InitialCorruptCheck is true to check data corruption on boot
 	// before serving any peer/client traffic.
-	InitialCorruptCheck     bool
-	CorruptCheckTime        time.Duration
-	CompactHashCheckEnabled bool
-	CompactHashCheckTime    time.Duration
+	InitialCorruptCheck  bool
+	CorruptCheckTime     time.Duration
+	CompactHashCheckTime time.Duration
 
 	// PreVote is true to enable Raft Pre-Vote.
 	PreVote bool

server/embed/config.go

@@ -58,21 +58,21 @@ const (
 	ClusterStateFlagNew      = "new"
 	ClusterStateFlagExisting = "existing"
 
-	DefaultName                             = "default"
-	DefaultMaxSnapshots                     = 5
-	DefaultMaxWALs                          = 5
-	DefaultMaxTxnOps                        = uint(128)
-	DefaultWarningApplyDuration             = 100 * time.Millisecond
-	DefaultWarningUnaryRequestDuration      = 300 * time.Millisecond
-	DefaultMaxRequestBytes                  = 1.5 * 1024 * 1024
-	DefaultMaxConcurrentStreams             = math.MaxUint32
-	DefaultGRPCKeepAliveMinTime             = 5 * time.Second
-	DefaultGRPCKeepAliveInterval            = 2 * time.Hour
-	DefaultGRPCKeepAliveTimeout             = 20 * time.Second
-	DefaultDowngradeCheckTime               = 5 * time.Second
-	DefaultAutoCompactionMode               = "periodic"
-	DefaultAuthToken                        = "simple"
-	DefaultExperimentalCompactHashCheckTime = time.Minute
+	DefaultName                        = "default"
+	DefaultMaxSnapshots                = 5
+	DefaultMaxWALs                     = 5
+	DefaultMaxTxnOps                   = uint(128)
+	DefaultWarningApplyDuration        = 100 * time.Millisecond
+	DefaultWarningUnaryRequestDuration = 300 * time.Millisecond
+	DefaultMaxRequestBytes             = 1.5 * 1024 * 1024
+	DefaultMaxConcurrentStreams        = math.MaxUint32
+	DefaultGRPCKeepAliveMinTime        = 5 * time.Second
+	DefaultGRPCKeepAliveInterval       = 2 * time.Hour
+	DefaultGRPCKeepAliveTimeout        = 20 * time.Second
+	DefaultDowngradeCheckTime          = 5 * time.Second
+	DefaultAutoCompactionMode          = "periodic"
+	DefaultAuthToken                   = "simple"
+	DefaultCompactHashCheckTime        = time.Minute
 
 	DefaultDiscoveryDialTimeout      = 2 * time.Second
 	DefaultDiscoveryRequestTimeOut   = 5 * time.Second
@@ -128,6 +128,13 @@ var (
 
 	// indirection for testing
 	getCluster = srv.GetCluster
+
+	// in 3.6, we are migration all the --experimental flags to feature gate and flags without the prefix.
+	// This is the mapping from the non boolean `experimental-` to the new flags.
+	// TODO: delete in v3.7
+	experimentalNonBoolFlagMigrationMap = map[string]string{
+		"experimental-compact-hash-check-time": "compact-hash-check-time",
+	}
 )
 
 var (
@@ -356,10 +363,17 @@ type Config struct {
 	// AuthTokenTTL in seconds of the simple token
 	AuthTokenTTL uint `json:"auth-token-ttl"`
 
-	ExperimentalInitialCorruptCheck     bool          `json:"experimental-initial-corrupt-check"`
-	ExperimentalCorruptCheckTime        time.Duration `json:"experimental-corrupt-check-time"`
-	ExperimentalCompactHashCheckEnabled bool          `json:"experimental-compact-hash-check-enabled"`
-	ExperimentalCompactHashCheckTime    time.Duration `json:"experimental-compact-hash-check-time"`
+	ExperimentalInitialCorruptCheck bool          `json:"experimental-initial-corrupt-check"`
+	ExperimentalCorruptCheckTime    time.Duration `json:"experimental-corrupt-check-time"`
+	// ExperimentalCompactHashCheckEnabled enables leader to periodically check followers compaction hashes.
+	// Deprecated in v3.6 and will be decommissioned in v3.7.
+	// TODO: delete in v3.7
+	ExperimentalCompactHashCheckEnabled bool `json:"experimental-compact-hash-check-enabled"`
+	// ExperimentalCompactHashCheckTime is the duration of time between leader checks followers compaction hashes.
+	// Deprecated in v3.6 and will be decommissioned in v3.7.
+	// TODO: delete in v3.7
+	ExperimentalCompactHashCheckTime time.Duration `json:"experimental-compact-hash-check-time"`
+	CompactHashCheckTime             time.Duration `json:"compact-hash-check-time"`
 
 	// ExperimentalEnableLeaseCheckpoint enables leader to send regular checkpoints to other members to prevent reset of remaining TTL on leader change.
 	ExperimentalEnableLeaseCheckpoint bool `json:"experimental-enable-lease-checkpoint"`
@@ -468,6 +482,8 @@ type Config struct {
 
 	// ServerFeatureGate is a server level feature gate
 	ServerFeatureGate featuregate.FeatureGate
+	// FlagsExplicitlySet stores if a flag is explicitly set from the cmd line or config file.
+	FlagsExplicitlySet map[string]bool
 }
 
 // configYAML holds the config suitable for yaml parsing
@@ -573,8 +589,9 @@ func NewConfig() *Config {
 		ExperimentalStopGRPCServiceOnDefrag:      false,
 		ExperimentalMaxLearners:                  membership.DefaultMaxLearners,
 
-		ExperimentalCompactHashCheckEnabled: false,
-		ExperimentalCompactHashCheckTime:    DefaultExperimentalCompactHashCheckTime,
+		CompactHashCheckTime: DefaultCompactHashCheckTime,
+		// TODO: delete in v3.7
+		ExperimentalCompactHashCheckTime: DefaultCompactHashCheckTime,
 
 		V2Deprecation: config.V2DeprDefault,
 
@@ -592,6 +609,7 @@ func NewConfig() *Config {
 
 		AutoCompactionMode: DefaultAutoCompactionMode,
 		ServerFeatureGate:  features.NewDefaultServerFeatureGate(DefaultName, nil),
+		FlagsExplicitlySet: map[string]bool{},
 	}
 	cfg.InitialCluster = cfg.InitialClusterFromName(cfg.Name)
 	return cfg
@@ -755,8 +773,11 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
 	// experimental
 	fs.BoolVar(&cfg.ExperimentalInitialCorruptCheck, "experimental-initial-corrupt-check", cfg.ExperimentalInitialCorruptCheck, "Enable to check data corruption before serving any client/peer traffic.")
 	fs.DurationVar(&cfg.ExperimentalCorruptCheckTime, "experimental-corrupt-check-time", cfg.ExperimentalCorruptCheckTime, "Duration of time between cluster corruption check passes.")
-	fs.BoolVar(&cfg.ExperimentalCompactHashCheckEnabled, "experimental-compact-hash-check-enabled", cfg.ExperimentalCompactHashCheckEnabled, "Enable leader to periodically check followers compaction hashes.")
-	fs.DurationVar(&cfg.ExperimentalCompactHashCheckTime, "experimental-compact-hash-check-time", cfg.ExperimentalCompactHashCheckTime, "Duration of time between leader checks followers compaction hashes.")
+	// TODO: delete in v3.7
+	fs.BoolVar(&cfg.ExperimentalCompactHashCheckEnabled, "experimental-compact-hash-check-enabled", cfg.ExperimentalCompactHashCheckEnabled, "Enable leader to periodically check followers compaction hashes. Deprecated in v3.6 and will be decommissioned in v3.7. Use '--feature-gates=CompactHashCheck=true' instead")
+	fs.DurationVar(&cfg.ExperimentalCompactHashCheckTime, "experimental-compact-hash-check-time", cfg.ExperimentalCompactHashCheckTime, "Duration of time between leader checks followers compaction hashes. Deprecated in v3.6 and will be decommissioned in v3.7. Use --compact-hash-check-time instead.")
+
+	fs.DurationVar(&cfg.CompactHashCheckTime, "compact-hash-check-time", cfg.CompactHashCheckTime, "Duration of time between leader checks followers compaction hashes.")
 
 	fs.BoolVar(&cfg.ExperimentalEnableLeaseCheckpoint, "experimental-enable-lease-checkpoint", false, "Enable leader to send regular checkpoints to other members to prevent reset of remaining TTL on leader change.")
 	// TODO: delete in v3.7
@@ -817,6 +838,11 @@ func (cfg *configYAML) configFromFile(path string) error {
 	if err != nil {
 		return err
 	}
+
+	for flg := range cfgMap {
+		cfg.FlagsExplicitlySet[flg] = true
+	}
+
 	getBoolFlagVal := func(flagName string) *bool {
 		flagVal, ok := cfgMap[flagName]
 		if !ok {
@@ -979,6 +1005,14 @@ func updateMinMaxVersions(info *transport.TLSInfo, min, max string) {
 
 // Validate ensures that '*embed.Config' fields are properly configured.
 func (cfg *Config) Validate() error {
+	// make sure there is no conflict in the flag settings in the ExperimentalNonBoolFlagMigrationMap
+	// TODO: delete in v3.7
+	for oldFlag, newFlag := range experimentalNonBoolFlagMigrationMap {
+		if cfg.FlagsExplicitlySet[oldFlag] && cfg.FlagsExplicitlySet[newFlag] {
+			return fmt.Errorf("cannot set %s and %s at the same time in the config file, please use %s only", oldFlag, newFlag, newFlag)
+		}
+	}
+
 	if err := cfg.setupLogging(); err != nil {
 		return err
 	}
@@ -1083,9 +1117,12 @@ func (cfg *Config) Validate() error {
 	if cfg.ExperimentalEnableLeaseCheckpointPersist && !cfg.ExperimentalEnableLeaseCheckpoint {
 		return fmt.Errorf("setting experimental-enable-lease-checkpoint-persist requires experimental-enable-lease-checkpoint")
 	}
-
+	// TODO: delete in v3.7
 	if cfg.ExperimentalCompactHashCheckTime <= 0 {
-		return fmt.Errorf("--experimental-compact-hash-check-time must be >0 (set to %v)", cfg.ExperimentalCompactHashCheckTime)
+		return fmt.Errorf("--experimental-compact-hash-check-time must be >=0 (set to %v)", cfg.ExperimentalCompactHashCheckTime)
+	}
+	if cfg.CompactHashCheckTime <= 0 {
+		return fmt.Errorf("--compact-hash-check-time must be >0 (set to %v)", cfg.CompactHashCheckTime)
 	}
 
 	// If `--name` isn't configured, then multiple members may have the same "default" name.

server/embed/config_test.go

@@ -98,6 +98,7 @@ func TestConfigFileFeatureGates(t *testing.T) {
 		serverFeatureGatesJSON              string
 		experimentalStopGRPCServiceOnDefrag string
 		experimentalInitialCorruptCheck     string
+		experimentalCompactHashCheckEnabled string
 		expectErr                           bool
 		expectedFeatures                    map[featuregate.Feature]bool
 	}{
@@ -194,12 +195,46 @@ func TestConfigFileFeatureGates(t *testing.T) {
 				features.InitialCorruptCheck:     false,
 			},
 		},
+		{
+			name:                                "cannot set both experimental flag and feature gate flag for ExperimentalCompactHashCheckEnabled",
+			serverFeatureGatesJSON:              "CompactHashCheck=true",
+			experimentalCompactHashCheckEnabled: "false",
+			expectErr:                           true,
+		},
+		{
+			name:                                "can set feature gate experimentalCompactHashCheckEnabled to true from experimental flag",
+			experimentalCompactHashCheckEnabled: "true",
+			expectedFeatures: map[featuregate.Feature]bool{
+				features.StopGRPCServiceOnDefrag: false,
+				features.DistributedTracing:      false,
+				features.CompactHashCheck:        true,
+			},
+		},
+		{
+			name:                                "can set feature gate experimentalCompactHashCheckEnabled to false from experimental flag",
+			experimentalCompactHashCheckEnabled: "false",
+			expectedFeatures: map[featuregate.Feature]bool{
+				features.StopGRPCServiceOnDefrag: false,
+				features.DistributedTracing:      false,
+				features.CompactHashCheck:        false,
+			},
+		},
+		{
+			name:                   "can set feature gate CompactHashCheck to true from feature gate flag",
+			serverFeatureGatesJSON: "CompactHashCheck=true",
+			expectedFeatures: map[featuregate.Feature]bool{
+				features.StopGRPCServiceOnDefrag: false,
+				features.DistributedTracing:      false,
+				features.CompactHashCheck:        true,
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			yc := struct {
 				ExperimentalStopGRPCServiceOnDefrag *bool  `json:"experimental-stop-grpc-service-on-defrag,omitempty"`
 				ExperimentalInitialCorruptCheck     *bool  `json:"experimental-initial-corrupt-check,omitempty"`
+				ExperimentalCompactHashCheckEnabled *bool  `json:"experimental-compact-hash-check-enabled,omitempty"`
 				ServerFeatureGatesJSON              string `json:"feature-gates"`
 			}{
 				ServerFeatureGatesJSON: tc.serverFeatureGatesJSON,
@@ -221,6 +256,14 @@ func TestConfigFileFeatureGates(t *testing.T) {
 				yc.ExperimentalStopGRPCServiceOnDefrag = &experimentalStopGRPCServiceOnDefrag
 			}
 
+			if tc.experimentalCompactHashCheckEnabled != "" {
+				experimentalCompactHashCheckEnabled, err := strconv.ParseBool(tc.experimentalCompactHashCheckEnabled)
+				if err != nil {
+					t.Fatal(err)
+				}
+				yc.ExperimentalCompactHashCheckEnabled = &experimentalCompactHashCheckEnabled
+			}
+
 			b, err := yaml.Marshal(&yc)
 			if err != nil {
 				t.Fatal(err)

server/embed/etcd.go

@@ -205,8 +205,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
 		CORS:                                     cfg.CORS,
 		HostWhitelist:                            cfg.HostWhitelist,
 		CorruptCheckTime:                         cfg.ExperimentalCorruptCheckTime,
-		CompactHashCheckEnabled:                  cfg.ExperimentalCompactHashCheckEnabled,
-		CompactHashCheckTime:                     cfg.ExperimentalCompactHashCheckTime,
+		CompactHashCheckTime:                     cfg.CompactHashCheckTime,
 		PreVote:                                  cfg.PreVote,
 		Logger:                                   cfg.logger,
 		ForceNewCluster:                          cfg.ForceNewCluster,
@@ -351,9 +350,9 @@ func print(lg *zap.Logger, ec Config, sc config.ServerConfig, memberInitialized
 		zap.Uint32("max-concurrent-streams", sc.MaxConcurrentStreams),
 
 		zap.Bool("pre-vote", sc.PreVote),
+		zap.String(ServerFeatureGateFlagName, sc.ServerFeatureGate.String()),
 		zap.Bool("initial-corrupt-check", sc.InitialCorruptCheck),
 		zap.String("corrupt-check-time-interval", sc.CorruptCheckTime.String()),
-		zap.Bool("compact-check-time-enabled", sc.CompactHashCheckEnabled),
 		zap.Duration("compact-check-time-interval", sc.CompactHashCheckTime),
 		zap.String("auto-compaction-mode", sc.AutoCompactionMode),
 		zap.Duration("auto-compaction-retention", sc.AutoCompactionRetention),

server/etcdmain/config.go

@@ -62,6 +62,8 @@ var (
 		"snapshot-count": "--snapshot-count is deprecated in 3.6 and will be decommissioned in 3.7.",
 		"max-snapshots":  "--max-snapshots is deprecated in 3.6 and will be decommissioned in 3.7.",
 		"v2-deprecation": "--v2-deprecation is deprecated and scheduled for removal in v3.8. The default value is enforced, ignoring user input.",
+		"experimental-compact-hash-check-enabled": "--experimental-compact-hash-check-enabled is deprecated in 3.6 and will be decommissioned in 3.7.",
+		"experimental-compact-hash-check-time":    "--experimental-compact-hash-check-time is deprecated in 3.6 and will be decommissioned in 3.7.",
 	}
 )
 
@@ -165,6 +167,12 @@ func (cfg *config) parse(arguments []string) error {
 		err = cfg.configFromCmdLine()
 	}
 
+	// params related to experimental flag deprecation
+	// TODO: delete in v3.7
+	if cfg.ec.FlagsExplicitlySet["experimental-compact-hash-check-time"] {
+		cfg.ec.CompactHashCheckTime = cfg.ec.ExperimentalCompactHashCheckTime
+	}
+
 	// `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
 
@@ -260,6 +268,10 @@ func (cfg *config) configFromCmdLine() error {
 		cfg.ec.InitialCluster = ""
 	}
 
+	cfg.cf.flagSet.Visit(func(f *flag.Flag) {
+		cfg.ec.FlagsExplicitlySet[f.Name] = true
+	})
+
 	getBoolFlagVal := func(flagName string) *bool {
 		boolVal, parseErr := flags.GetBoolFlagVal(cfg.cf.flagSet, flagName)
 		if parseErr != nil {