Squashed commit of the following:

commit 4fc53fa2536b4ca911c0d2c24a475e22c9df4564 Author: Gang Li <[email protected]> Date: Thu Jan 23 10:32:01 2025 -0800 use require.NoError in error check Signed-off-by: Gang Li <[email protected]> commit d3c595a Author: Gang Li <[email protected]> Date: Wed Jan 22 14:41:50 2025 -0800 fix name typo Signed-off-by: Gang Li <[email protected]> commit 4a848cd Author: Gang Li <[email protected]> Date: Tue Jan 21 14:35:50 2025 -0800 use experimental flag in robustness scenario Signed-off-by: Gang Li <[email protected]> commit 346020b Author: Gang Li <[email protected]> Date: Tue Jan 21 10:50:58 2025 -0800 add TODO Signed-off-by: Gang Li <[email protected]> commit edf6845 Author: Gang Li <[email protected]> Date: Tue Jan 21 10:44:54 2025 -0800 migrate to use watch-progress-notify-interval flag Signed-off-by: Gang Li <[email protected]> Signed-off-by: Gang Li <[email protected]>
etcd-io · Jan 23, 2025 · c893658 · c893658
1 parent 32cfd45
commit c893658
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
 			}
 
@@ -596,18 +593,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
 			}
 
@@ -701,12 +694,82 @@ func TestCompactionBatchLimitFlagMigration(t *testing.T) {
 	}
 }
 
+// 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 errFromCmdLine != nil || errFromFile != nil {
+				t.Fatal("error parsing config")
+			}
+
+			if cfgFromCmdLine.ec.WatchProgressNotifyInterval != tc.expectedWatchProgressNotifyInterval {
+				t.Errorf("expected WatchProgressNotifyInterval=%v, got %v", tc.expectedWatchProgressNotifyInterval, cfgFromCmdLine.ec.WatchProgressNotifyInterval)
+			}
+			if cfgFromFile.ec.WatchProgressNotifyInterval != tc.expectedWatchProgressNotifyInterval {
+				t.Errorf("expected WatchProgressNotifyInterval=%v, got %v", 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())
@@ -814,6 +877,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 +898,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
@@ -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
@@ -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
@@ -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
@@ -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) {