diff --git a/server/embed/config.go b/server/embed/config.go index a66adf97c2b..94306637235 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -133,9 +133,10 @@ var ( // 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", - "experimental-corrupt-check-time": "corrupt-check-time", - "experimental-compaction-batch-limit": "compaction-batch-limit", + "experimental-compact-hash-check-time": "compact-hash-check-time", + "experimental-corrupt-check-time": "corrupt-check-time", + "experimental-compaction-batch-limit": "compaction-batch-limit", + "experimental-watch-progress-notify-interval": "watch-progress-notify-interval", } ) @@ -394,8 +395,12 @@ type Config struct { ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"` CompactionBatchLimit int `json:"compaction-batch-limit"` // ExperimentalCompactionSleepInterval is the sleep interval between every etcd compaction loop. - ExperimentalCompactionSleepInterval time.Duration `json:"experimental-compaction-sleep-interval"` + ExperimentalCompactionSleepInterval time.Duration `json:"experimental-compaction-sleep-interval"` + // ExperimentalWatchProgressNotifyInterval is the time duration of periodic watch progress notifications. + // Deprecated in v3.6 and will be decommissioned in v3.7. + // TODO: Delete in v3.7 ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval"` + WatchProgressNotifyInterval time.Duration `json:"watch-progress-notify-interval"` // ExperimentalWarningApplyDuration is the time duration after which a warning is generated if applying request // takes more time than this value. ExperimentalWarningApplyDuration time.Duration `json:"experimental-warning-apply-duration"` @@ -797,7 +802,9 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) { fs.IntVar(&cfg.ExperimentalCompactionBatchLimit, "experimental-compaction-batch-limit", cfg.ExperimentalCompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch. Deprecated in v3.6 and will be decommissioned in v3.7. Use --compaction-batch-limit instead.") fs.IntVar(&cfg.CompactionBatchLimit, "compaction-batch-limit", cfg.CompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch.") fs.DurationVar(&cfg.ExperimentalCompactionSleepInterval, "experimental-compaction-sleep-interval", cfg.ExperimentalCompactionSleepInterval, "Sets the sleep interval between each compaction batch.") - fs.DurationVar(&cfg.ExperimentalWatchProgressNotifyInterval, "experimental-watch-progress-notify-interval", cfg.ExperimentalWatchProgressNotifyInterval, "Duration of periodic watch progress notifications.") + // TODO: delete in v3.7 + fs.DurationVar(&cfg.ExperimentalWatchProgressNotifyInterval, "experimental-watch-progress-notify-interval", cfg.ExperimentalWatchProgressNotifyInterval, "Duration of periodic watch progress notifications. Deprecated in v3.6 and will be decommissioned in v3.7. Use --watch-progress-notify-interval instead.") + fs.DurationVar(&cfg.WatchProgressNotifyInterval, "watch-progress-notify-interval", cfg.WatchProgressNotifyInterval, "Duration of periodic watch progress notifications.") fs.DurationVar(&cfg.ExperimentalDowngradeCheckTime, "experimental-downgrade-check-time", cfg.ExperimentalDowngradeCheckTime, "Duration of time between two downgrade status checks.") fs.DurationVar(&cfg.ExperimentalWarningApplyDuration, "experimental-warning-apply-duration", cfg.ExperimentalWarningApplyDuration, "Time duration after which a warning is generated if request takes more time.") fs.DurationVar(&cfg.WarningUnaryRequestDuration, "warning-unary-request-duration", cfg.WarningUnaryRequestDuration, "Time duration after which a warning is generated if a unary request takes more time.") diff --git a/server/embed/etcd.go b/server/embed/etcd.go index 18811624854..c3491588989 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -229,7 +229,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { LeaseCheckpointPersist: cfg.ExperimentalEnableLeaseCheckpointPersist, CompactionBatchLimit: cfg.CompactionBatchLimit, CompactionSleepInterval: cfg.ExperimentalCompactionSleepInterval, - WatchProgressNotifyInterval: cfg.ExperimentalWatchProgressNotifyInterval, + WatchProgressNotifyInterval: cfg.WatchProgressNotifyInterval, DowngradeCheckTime: cfg.ExperimentalDowngradeCheckTime, WarningApplyDuration: cfg.ExperimentalWarningApplyDuration, WarningUnaryRequestDuration: cfg.WarningUnaryRequestDuration, diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index 99cc279a1be..633bf053bd4 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -67,6 +67,7 @@ var ( "experimental-txn-mode-write-with-shared-buffer": "--experimental-txn-mode-write-with-shared-buffer is deprecated in v3.6 and will be decommissioned in v3.7. Use '--feature-gates=TxnModeWriteWithSharedBuffer=true' instead.", "experimental-corrupt-check-time": "--experimental-corrupt-check-time is deprecated in v3.6 and will be decommissioned in v3.7. Use '--corrupt-check-time' instead.", "experimental-compaction-batch-limit": "--experimental-compaction-batch-limit is deprecated in v3.6 and will be decommissioned in v3.7. Use '--compaction-batch-limit' instead.", + "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.", } ) @@ -184,6 +185,10 @@ func (cfg *config) parse(arguments []string) error { cfg.ec.CompactionBatchLimit = cfg.ec.ExperimentalCompactionBatchLimit } + if cfg.ec.FlagsExplicitlySet["experimental-watch-progress-notify-interval"] { + cfg.ec.WatchProgressNotifyInterval = cfg.ec.ExperimentalWatchProgressNotifyInterval + } + // `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 diff --git a/server/etcdmain/config_test.go b/server/etcdmain/config_test.go index 634921b66b7..0cbaa9c1a97 100644 --- a/server/etcdmain/config_test.go +++ b/server/etcdmain/config_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" "go.etcd.io/etcd/pkg/v3/featuregate" @@ -520,18 +521,14 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) { if tc.compactHashCheckTime != "" { cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--compact-hash-check-time=%s", tc.compactHashCheckTime)) compactHashCheckTime, err := time.ParseDuration(tc.compactHashCheckTime) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) yc.CompactHashCheckTime = compactHashCheckTime } if tc.experimentalCompactHashCheckTime != "" { cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-compact-hash-check-time=%s", tc.experimentalCompactHashCheckTime)) experimentalCompactHashCheckTime, err := time.ParseDuration(tc.experimentalCompactHashCheckTime) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) yc.ExperimentalCompactHashCheckTime = experimentalCompactHashCheckTime } @@ -547,12 +544,8 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) { t.Fatal("error parsing config") } - if cfgFromCmdLine.ec.CompactHashCheckTime != tc.expectedCompactHashCheckTime { - t.Errorf("expected CompactHashCheckTime=%v, got %v", tc.expectedCompactHashCheckTime, cfgFromCmdLine.ec.CompactHashCheckTime) - } - if cfgFromFile.ec.CompactHashCheckTime != tc.expectedCompactHashCheckTime { - t.Errorf("expected CompactHashCheckTime=%v, got %v", tc.expectedCompactHashCheckTime, cfgFromFile.ec.CompactHashCheckTime) - } + require.Equal(t, tc.expectedCompactHashCheckTime, cfgFromCmdLine.ec.CompactHashCheckTime) + require.Equal(t, tc.expectedCompactHashCheckTime, cfgFromFile.ec.CompactHashCheckTime) }) } } @@ -596,18 +589,14 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) { if tc.corruptCheckTime != "" { cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--corrupt-check-time=%s", tc.corruptCheckTime)) corruptCheckTime, err := time.ParseDuration(tc.corruptCheckTime) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) yc.CorruptCheckTime = corruptCheckTime } if tc.experimentalCorruptCheckTime != "" { cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-corrupt-check-time=%s", tc.experimentalCorruptCheckTime)) experimentalCorruptCheckTime, err := time.ParseDuration(tc.experimentalCorruptCheckTime) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) yc.ExperimentalCorruptCheckTime = experimentalCorruptCheckTime } @@ -623,12 +612,8 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) { t.Fatal("error parsing config") } - if cfgFromCmdLine.ec.CorruptCheckTime != tc.expectedCorruptCheckTime { - t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCorruptCheckTime, cfgFromCmdLine.ec.CorruptCheckTime) - } - if cfgFromFile.ec.CorruptCheckTime != tc.expectedCorruptCheckTime { - t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCorruptCheckTime, cfgFromFile.ec.CorruptCheckTime) - } + require.Equal(t, tc.expectedCorruptCheckTime, cfgFromCmdLine.ec.CorruptCheckTime) + require.Equal(t, tc.expectedCorruptCheckTime, cfgFromFile.ec.CorruptCheckTime) }) } } @@ -691,12 +676,76 @@ func TestCompactionBatchLimitFlagMigration(t *testing.T) { t.Fatal("error parsing config") } - if cfgFromCmdLine.ec.CompactionBatchLimit != tc.expectedCompactionBatchLimit { - t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCompactionBatchLimit, cfgFromCmdLine.ec.CompactionBatchLimit) + require.Equal(t, tc.expectedCompactionBatchLimit, cfgFromCmdLine.ec.CompactionBatchLimit) + require.Equal(t, tc.expectedCompactionBatchLimit, cfgFromFile.ec.CompactionBatchLimit) + }) + } +} + +// TestWatchProgressNotifyInterval tests the migration from +// --experimental-watch-progress-notify-interval to --watch-progress-notify-interval +// TODO: delete in v3.7 +func TestWatchProgressNotifyInterval(t *testing.T) { + testCases := []struct { + name string + watchProgressNotifyInterval string + experimentalWatchProgressNotifyInterval string + expectErr bool + expectedWatchProgressNotifyInterval time.Duration + }{ + { + name: "cannot set both experimental flag and non experimental flag", + watchProgressNotifyInterval: "2m", + experimentalWatchProgressNotifyInterval: "3m", + expectErr: true, + }, + { + name: "can set experimental flag", + experimentalWatchProgressNotifyInterval: "3m", + expectedWatchProgressNotifyInterval: 3 * time.Minute, + }, + { + name: "can set non experimental flag", + watchProgressNotifyInterval: "2m", + expectedWatchProgressNotifyInterval: 2 * time.Minute, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cmdLineArgs := []string{} + yc := struct { + ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval,omitempty"` + WatchProgressNotifyInterval time.Duration `json:"watch-progress-notify-interval,omitempty"` + }{} + + if tc.watchProgressNotifyInterval != "" { + cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--watch-progress-notify-interval=%s", tc.watchProgressNotifyInterval)) + watchProgressNotifyInterval, err := time.ParseDuration(tc.watchProgressNotifyInterval) + require.NoError(t, err) + yc.WatchProgressNotifyInterval = watchProgressNotifyInterval + } + + if tc.experimentalWatchProgressNotifyInterval != "" { + cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-watch-progress-notify-interval=%s", tc.experimentalWatchProgressNotifyInterval)) + experimentalWatchProgressNotifyInterval, err := time.ParseDuration(tc.experimentalWatchProgressNotifyInterval) + require.NoError(t, err) + yc.ExperimentalWatchProgressNotifyInterval = experimentalWatchProgressNotifyInterval + } + + cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs) + + if tc.expectErr { + if errFromCmdLine == nil || errFromFile == nil { + t.Fatal("expect parse error") + } + return } - if cfgFromFile.ec.CompactionBatchLimit != tc.expectedCompactionBatchLimit { - t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCompactionBatchLimit, cfgFromFile.ec.CompactionBatchLimit) + if errFromCmdLine != nil || errFromFile != nil { + t.Fatal("error parsing config") } + + require.Equal(t, tc.expectedWatchProgressNotifyInterval, cfgFromCmdLine.ec.WatchProgressNotifyInterval) + require.Equal(t, tc.expectedWatchProgressNotifyInterval, cfgFromFile.ec.WatchProgressNotifyInterval) }) } } @@ -704,9 +753,7 @@ func TestCompactionBatchLimitFlagMigration(t *testing.T) { // TODO delete in v3.7 func generateCfgsFromFileAndCmdLine(t *testing.T, yc any, cmdLineArgs []string) (*config, error, *config, error) { b, err := yaml.Marshal(&yc) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) tmpfile := mustCreateCfgFile(t, b) defer os.Remove(tmpfile.Name()) @@ -814,6 +861,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) { ExperimentalWarningUnaryRequestDuration time.Duration `json:"experimental-warning-unary-request-duration,omitempty"` ExperimentalCorruptCheckTime time.Duration `json:"experimental-corrupt-check-time,omitempty"` ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit,omitempty"` + ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval,omitempty"` } testCases := []struct { @@ -834,12 +882,14 @@ func TestConfigFileDeprecatedOptions(t *testing.T) { ExperimentalWarningUnaryRequestDuration: time.Second, ExperimentalCorruptCheckTime: time.Minute, ExperimentalCompactionBatchLimit: 1, + ExperimentalWatchProgressNotifyInterval: 3 * time.Minute, }, expectedFlags: map[string]struct{}{ - "experimental-compact-hash-check-enabled": {}, - "experimental-compact-hash-check-time": {}, - "experimental-corrupt-check-time": {}, - "experimental-compaction-batch-limit": {}, + "experimental-compact-hash-check-enabled": {}, + "experimental-compact-hash-check-time": {}, + "experimental-corrupt-check-time": {}, + "experimental-compaction-batch-limit": {}, + "experimental-watch-progress-notify-interval": {}, }, }, { diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index 832716fb24f..1cd88e98607 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -294,6 +294,8 @@ Experimental feature: --experimental-peer-skip-client-san-verification 'false' Skip verification of SAN field in client certificate for peer connections. --experimental-watch-progress-notify-interval '10m' + Duration of periodical watch progress notification. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'watch-progress-notify-interval' instead. + --watch-progress-notify-interval '10m' Duration of periodical watch progress notification. --experimental-warning-apply-duration '100ms' Warning is generated if requests take more than this duration. diff --git a/tests/e2e/watch_test.go b/tests/e2e/watch_test.go index 5405d82d6ab..758177c6489 100644 --- a/tests/e2e/watch_test.go +++ b/tests/e2e/watch_test.go @@ -91,7 +91,7 @@ func TestWatchDelayForPeriodicProgressNotification(t *testing.T) { tc := tc cfg := e2e.DefaultConfig() cfg.ClusterSize = 1 - cfg.ServerConfig.ExperimentalWatchProgressNotifyInterval = watchResponsePeriod + cfg.ServerConfig.WatchProgressNotifyInterval = watchResponsePeriod cfg.Client = tc.client cfg.ClientHTTPSeparate = tc.clientHTTPSeparate t.Run(tc.name, func(t *testing.T) { diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go index b273041a999..3a2f8388835 100644 --- a/tests/framework/e2e/cluster.go +++ b/tests/framework/e2e/cluster.go @@ -385,6 +385,10 @@ func WithCompactionSleepInterval(time time.Duration) EPClusterOption { } func WithWatchProcessNotifyInterval(interval time.Duration) EPClusterOption { + return func(c *EtcdProcessClusterConfig) { c.ServerConfig.WatchProgressNotifyInterval = interval } +} + +func WithExperimentalWatchProcessNotifyInterval(interval time.Duration) EPClusterOption { return func(c *EtcdProcessClusterConfig) { c.ServerConfig.ExperimentalWatchProgressNotifyInterval = interval } } diff --git a/tests/robustness/scenarios/scenarios.go b/tests/robustness/scenarios/scenarios.go index e4e7378d822..0c4aac2fb93 100644 --- a/tests/robustness/scenarios/scenarios.go +++ b/tests/robustness/scenarios/scenarios.go @@ -98,7 +98,7 @@ func Exploratory(_ *testing.T) []TestScenario { e2e.WithGoFailEnabled(true), // Set low minimal compaction batch limit to allow for triggering multi batch compaction failpoints. options.WithExperimentalCompactionBatchLimit(10, 100, 1000), - e2e.WithWatchProcessNotifyInterval(100 * time.Millisecond), + e2e.WithExperimentalWatchProcessNotifyInterval(100 * time.Millisecond), } if e2e.CouldSetSnapshotCatchupEntries(e2e.BinPath.Etcd) {