Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate flag experimental-watch-progress-notify-interval to use watch-progress-notify-interval #19248

Merged
merged 2 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions server/embed/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
)

Expand Down Expand Up @@ -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"`
Expand Down Expand Up @@ -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.")
Expand Down
2 changes: 1 addition & 1 deletion server/embed/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
}
)

Expand Down Expand Up @@ -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

Expand Down
120 changes: 85 additions & 35 deletions server/etcdmain/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand All @@ -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)
})
}
}
Expand Down Expand Up @@ -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
}

Expand All @@ -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)
})
}
}
Expand Down Expand Up @@ -691,22 +676,84 @@ 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)
})
}
}

// 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())
Expand Down Expand Up @@ -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 {
Expand All @@ -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": {},
},
},
{
Expand Down
2 changes: 2 additions & 0 deletions server/etcdmain/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions tests/framework/e2e/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}

Expand Down
2 changes: 1 addition & 1 deletion tests/robustness/scenarios/scenarios.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading