Skip to content

Commit fecd920

Browse files
omerap12omerap12
committed
Merge branch 'main' into migrate-experiminal-enable-distributed-tracing
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
2 parents fcabd94 + b16b8dc commit fecd920

File tree

7 files changed

+107
-39
lines changed

7 files changed

+107
-39
lines changed

‎server/embed/config.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ var (
145145
"experimental-compact-hash-check-time": "compact-hash-check-time",
146146
"experimental-corrupt-check-time": "corrupt-check-time",
147147
"experimental-compaction-batch-limit": "compaction-batch-limit",
148+
"experimental-watch-progress-notify-interval": "watch-progress-notify-interval",
148149
}
149150
)
150151

@@ -403,8 +404,12 @@ type Config struct {
403404
ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"`
404405
CompactionBatchLimit int `json:"compaction-batch-limit"`
405406
// ExperimentalCompactionSleepInterval is the sleep interval between every etcd compaction loop.
406-
ExperimentalCompactionSleepInterval time.Duration `json:"experimental-compaction-sleep-interval"`
407+
ExperimentalCompactionSleepInterval time.Duration `json:"experimental-compaction-sleep-interval"`
408+
// ExperimentalWatchProgressNotifyInterval is the time duration of periodic watch progress notifications.
409+
// Deprecated in v3.6 and will be decommissioned in v3.7.
410+
// TODO: Delete in v3.7
407411
ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval"`
412+
WatchProgressNotifyInterval time.Duration `json:"watch-progress-notify-interval"`
408413
// ExperimentalWarningApplyDuration is the time duration after which a warning is generated if applying request
409414
// takes more time than this value.
410415
ExperimentalWarningApplyDuration time.Duration `json:"experimental-warning-apply-duration"`
@@ -821,7 +826,9 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
821826
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.")
822827
fs.IntVar(&cfg.CompactionBatchLimit, "compaction-batch-limit", cfg.CompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch.")
823828
fs.DurationVar(&cfg.ExperimentalCompactionSleepInterval, "experimental-compaction-sleep-interval", cfg.ExperimentalCompactionSleepInterval, "Sets the sleep interval between each compaction batch.")
824-
fs.DurationVar(&cfg.ExperimentalWatchProgressNotifyInterval, "experimental-watch-progress-notify-interval", cfg.ExperimentalWatchProgressNotifyInterval, "Duration of periodic watch progress notifications.")
829+
// TODO: delete in v3.7
830+
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.")
831+
fs.DurationVar(&cfg.WatchProgressNotifyInterval, "watch-progress-notify-interval", cfg.WatchProgressNotifyInterval, "Duration of periodic watch progress notifications.")
825832
fs.DurationVar(&cfg.ExperimentalDowngradeCheckTime, "experimental-downgrade-check-time", cfg.ExperimentalDowngradeCheckTime, "Duration of time between two downgrade status checks.")
826833
fs.DurationVar(&cfg.ExperimentalWarningApplyDuration, "experimental-warning-apply-duration", cfg.ExperimentalWarningApplyDuration, "Time duration after which a warning is generated if request takes more time.")
827834
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.")

‎server/etcdmain/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ var (
7272
"experimental-distributed-tracing-sampling-rate": "--experimental-distributed-tracing-sampling-rate is deprecated in 3.6 and will be decommissioned in 3.7. Use --distributed-tracing-sampling-rate instead.",
7373
"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.",
7474
"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.",
75+
"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.",
7576
}
7677
)
7778

@@ -189,6 +190,10 @@ func (cfg *config) parse(arguments []string) error {
189190
cfg.ec.CompactionBatchLimit = cfg.ec.ExperimentalCompactionBatchLimit
190191
}
191192

193+
if cfg.ec.FlagsExplicitlySet["experimental-watch-progress-notify-interval"] {
194+
cfg.ec.WatchProgressNotifyInterval = cfg.ec.ExperimentalWatchProgressNotifyInterval
195+
}
196+
192197
// `V2Deprecation` (--v2-deprecation) is deprecated and scheduled for removal in v3.8. The default value is enforced, ignoring user input.
193198
cfg.ec.V2Deprecation = cconfig.V2DeprDefault
194199

‎server/etcdmain/config_test.go

Lines changed: 85 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"time"
2727

2828
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
2930
"sigs.k8s.io/yaml"
3031

3132
"go.etcd.io/etcd/pkg/v3/featuregate"
@@ -520,18 +521,14 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) {
520521
if tc.compactHashCheckTime != "" {
521522
cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--compact-hash-check-time=%s", tc.compactHashCheckTime))
522523
compactHashCheckTime, err := time.ParseDuration(tc.compactHashCheckTime)
523-
if err != nil {
524-
t.Fatal(err)
525-
}
524+
require.NoError(t, err)
526525
yc.CompactHashCheckTime = compactHashCheckTime
527526
}
528527

529528
if tc.experimentalCompactHashCheckTime != "" {
530529
cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-compact-hash-check-time=%s", tc.experimentalCompactHashCheckTime))
531530
experimentalCompactHashCheckTime, err := time.ParseDuration(tc.experimentalCompactHashCheckTime)
532-
if err != nil {
533-
t.Fatal(err)
534-
}
531+
require.NoError(t, err)
535532
yc.ExperimentalCompactHashCheckTime = experimentalCompactHashCheckTime
536533
}
537534

@@ -547,12 +544,8 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) {
547544
t.Fatal("error parsing config")
548545
}
549546

550-
if cfgFromCmdLine.ec.CompactHashCheckTime != tc.expectedCompactHashCheckTime {
551-
t.Errorf("expected CompactHashCheckTime=%v, got %v", tc.expectedCompactHashCheckTime, cfgFromCmdLine.ec.CompactHashCheckTime)
552-
}
553-
if cfgFromFile.ec.CompactHashCheckTime != tc.expectedCompactHashCheckTime {
554-
t.Errorf("expected CompactHashCheckTime=%v, got %v", tc.expectedCompactHashCheckTime, cfgFromFile.ec.CompactHashCheckTime)
555-
}
547+
require.Equal(t, tc.expectedCompactHashCheckTime, cfgFromCmdLine.ec.CompactHashCheckTime)
548+
require.Equal(t, tc.expectedCompactHashCheckTime, cfgFromFile.ec.CompactHashCheckTime)
556549
})
557550
}
558551
}
@@ -596,18 +589,14 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) {
596589
if tc.corruptCheckTime != "" {
597590
cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--corrupt-check-time=%s", tc.corruptCheckTime))
598591
corruptCheckTime, err := time.ParseDuration(tc.corruptCheckTime)
599-
if err != nil {
600-
t.Fatal(err)
601-
}
592+
require.NoError(t, err)
602593
yc.CorruptCheckTime = corruptCheckTime
603594
}
604595

605596
if tc.experimentalCorruptCheckTime != "" {
606597
cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-corrupt-check-time=%s", tc.experimentalCorruptCheckTime))
607598
experimentalCorruptCheckTime, err := time.ParseDuration(tc.experimentalCorruptCheckTime)
608-
if err != nil {
609-
t.Fatal(err)
610-
}
599+
require.NoError(t, err)
611600
yc.ExperimentalCorruptCheckTime = experimentalCorruptCheckTime
612601
}
613602

@@ -623,12 +612,8 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) {
623612
t.Fatal("error parsing config")
624613
}
625614

626-
if cfgFromCmdLine.ec.CorruptCheckTime != tc.expectedCorruptCheckTime {
627-
t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCorruptCheckTime, cfgFromCmdLine.ec.CorruptCheckTime)
628-
}
629-
if cfgFromFile.ec.CorruptCheckTime != tc.expectedCorruptCheckTime {
630-
t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCorruptCheckTime, cfgFromFile.ec.CorruptCheckTime)
631-
}
615+
require.Equal(t, tc.expectedCorruptCheckTime, cfgFromCmdLine.ec.CorruptCheckTime)
616+
require.Equal(t, tc.expectedCorruptCheckTime, cfgFromFile.ec.CorruptCheckTime)
632617
})
633618
}
634619
}
@@ -691,22 +676,84 @@ func TestCompactionBatchLimitFlagMigration(t *testing.T) {
691676
t.Fatal("error parsing config")
692677
}
693678

694-
if cfgFromCmdLine.ec.CompactionBatchLimit != tc.expectedCompactionBatchLimit {
695-
t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCompactionBatchLimit, cfgFromCmdLine.ec.CompactionBatchLimit)
679+
require.Equal(t, tc.expectedCompactionBatchLimit, cfgFromCmdLine.ec.CompactionBatchLimit)
680+
require.Equal(t, tc.expectedCompactionBatchLimit, cfgFromFile.ec.CompactionBatchLimit)
681+
})
682+
}
683+
}
684+
685+
// TestWatchProgressNotifyInterval tests the migration from
686+
// --experimental-watch-progress-notify-interval to --watch-progress-notify-interval
687+
// TODO: delete in v3.7
688+
func TestWatchProgressNotifyInterval(t *testing.T) {
689+
testCases := []struct {
690+
name string
691+
watchProgressNotifyInterval string
692+
experimentalWatchProgressNotifyInterval string
693+
expectErr bool
694+
expectedWatchProgressNotifyInterval time.Duration
695+
}{
696+
{
697+
name: "cannot set both experimental flag and non experimental flag",
698+
watchProgressNotifyInterval: "2m",
699+
experimentalWatchProgressNotifyInterval: "3m",
700+
expectErr: true,
701+
},
702+
{
703+
name: "can set experimental flag",
704+
experimentalWatchProgressNotifyInterval: "3m",
705+
expectedWatchProgressNotifyInterval: 3 * time.Minute,
706+
},
707+
{
708+
name: "can set non experimental flag",
709+
watchProgressNotifyInterval: "2m",
710+
expectedWatchProgressNotifyInterval: 2 * time.Minute,
711+
},
712+
}
713+
for _, tc := range testCases {
714+
t.Run(tc.name, func(t *testing.T) {
715+
cmdLineArgs := []string{}
716+
yc := struct {
717+
ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval,omitempty"`
718+
WatchProgressNotifyInterval time.Duration `json:"watch-progress-notify-interval,omitempty"`
719+
}{}
720+
721+
if tc.watchProgressNotifyInterval != "" {
722+
cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--watch-progress-notify-interval=%s", tc.watchProgressNotifyInterval))
723+
watchProgressNotifyInterval, err := time.ParseDuration(tc.watchProgressNotifyInterval)
724+
require.NoError(t, err)
725+
yc.WatchProgressNotifyInterval = watchProgressNotifyInterval
726+
}
727+
728+
if tc.experimentalWatchProgressNotifyInterval != "" {
729+
cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-watch-progress-notify-interval=%s", tc.experimentalWatchProgressNotifyInterval))
730+
experimentalWatchProgressNotifyInterval, err := time.ParseDuration(tc.experimentalWatchProgressNotifyInterval)
731+
require.NoError(t, err)
732+
yc.ExperimentalWatchProgressNotifyInterval = experimentalWatchProgressNotifyInterval
733+
}
734+
735+
cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs)
736+
737+
if tc.expectErr {
738+
if errFromCmdLine == nil || errFromFile == nil {
739+
t.Fatal("expect parse error")
740+
}
741+
return
696742
}
697-
if cfgFromFile.ec.CompactionBatchLimit != tc.expectedCompactionBatchLimit {
698-
t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCompactionBatchLimit, cfgFromFile.ec.CompactionBatchLimit)
743+
if errFromCmdLine != nil || errFromFile != nil {
744+
t.Fatal("error parsing config")
699745
}
746+
747+
require.Equal(t, tc.expectedWatchProgressNotifyInterval, cfgFromCmdLine.ec.WatchProgressNotifyInterval)
748+
require.Equal(t, tc.expectedWatchProgressNotifyInterval, cfgFromFile.ec.WatchProgressNotifyInterval)
700749
})
701750
}
702751
}
703752

704753
// TODO delete in v3.7
705754
func generateCfgsFromFileAndCmdLine(t *testing.T, yc any, cmdLineArgs []string) (*config, error, *config, error) {
706755
b, err := yaml.Marshal(&yc)
707-
if err != nil {
708-
t.Fatal(err)
709-
}
756+
require.NoError(t, err)
710757

711758
tmpfile := mustCreateCfgFile(t, b)
712759
defer os.Remove(tmpfile.Name())
@@ -819,6 +866,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
819866
ExperimentalDistributedTracingServiceName string `json:"experimental-distributed-tracing-service-name,omitempty"`
820867
ExperimentalDistributedTracingInstanceID string `json:"experimental-distributed-tracing-instance-id,omitempty"`
821868
ExperimentalDistributedTracingSamplingRate int `json:"experimental-distributed-tracing-sampling-rate,omitempty"`
869+
ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval,omitempty"`
822870
}
823871

824872
testCases := []struct {
@@ -839,12 +887,14 @@ func TestConfigFileDeprecatedOptions(t *testing.T) {
839887
ExperimentalWarningUnaryRequestDuration: time.Second,
840888
ExperimentalCorruptCheckTime: time.Minute,
841889
ExperimentalCompactionBatchLimit: 1,
890+
ExperimentalWatchProgressNotifyInterval: 3 * time.Minute,
842891
},
843892
expectedFlags: map[string]struct{}{
844-
"experimental-compact-hash-check-enabled": {},
845-
"experimental-compact-hash-check-time": {},
846-
"experimental-corrupt-check-time": {},
847-
"experimental-compaction-batch-limit": {},
893+
"experimental-compact-hash-check-enabled": {},
894+
"experimental-compact-hash-check-time": {},
895+
"experimental-corrupt-check-time": {},
896+
"experimental-compaction-batch-limit": {},
897+
"experimental-watch-progress-notify-interval": {},
848898
},
849899
},
850900
{

‎server/etcdmain/help.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,8 @@ Experimental feature:
306306
--experimental-peer-skip-client-san-verification 'false'
307307
Skip verification of SAN field in client certificate for peer connections.
308308
--experimental-watch-progress-notify-interval '10m'
309+
Duration of periodical watch progress notification. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'watch-progress-notify-interval' instead.
310+
--watch-progress-notify-interval '10m'
309311
Duration of periodical watch progress notification.
310312
--experimental-warning-apply-duration '100ms'
311313
Warning is generated if requests take more than this duration.

‎tests/e2e/watch_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestWatchDelayForPeriodicProgressNotification(t *testing.T) {
9191
tc := tc
9292
cfg := e2e.DefaultConfig()
9393
cfg.ClusterSize = 1
94-
cfg.ServerConfig.ExperimentalWatchProgressNotifyInterval = watchResponsePeriod
94+
cfg.ServerConfig.WatchProgressNotifyInterval = watchResponsePeriod
9595
cfg.Client = tc.client
9696
cfg.ClientHTTPSeparate = tc.clientHTTPSeparate
9797
t.Run(tc.name, func(t *testing.T) {

‎tests/framework/e2e/cluster.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,10 @@ func WithCompactionSleepInterval(time time.Duration) EPClusterOption {
385385
}
386386

387387
func WithWatchProcessNotifyInterval(interval time.Duration) EPClusterOption {
388+
return func(c *EtcdProcessClusterConfig) { c.ServerConfig.WatchProgressNotifyInterval = interval }
389+
}
390+
391+
func WithExperimentalWatchProcessNotifyInterval(interval time.Duration) EPClusterOption {
388392
return func(c *EtcdProcessClusterConfig) { c.ServerConfig.ExperimentalWatchProgressNotifyInterval = interval }
389393
}
390394

‎tests/robustness/scenarios/scenarios.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func Exploratory(_ *testing.T) []TestScenario {
9898
e2e.WithGoFailEnabled(true),
9999
// Set low minimal compaction batch limit to allow for triggering multi batch compaction failpoints.
100100
options.WithExperimentalCompactionBatchLimit(10, 100, 1000),
101-
e2e.WithWatchProcessNotifyInterval(100 * time.Millisecond),
101+
e2e.WithExperimentalWatchProcessNotifyInterval(100 * time.Millisecond),
102102
}
103103

104104
if e2e.CouldSetSnapshotCatchupEntries(e2e.BinPath.Etcd) {

0 commit comments

Comments
 (0)