diff --git a/server/embed/config.go b/server/embed/config.go index 9ddddaf00f0..dbdd07693aa 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -135,6 +135,7 @@ var ( 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", } ) @@ -387,7 +388,11 @@ type Config struct { // Deprecated in v3.6. // TODO: Delete in v3.7 ExperimentalEnableLeaseCheckpointPersist bool `json:"experimental-enable-lease-checkpoint-persist"` - ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit"` + // ExperimentalCompactionBatchLimit Sets the maximum revisions deleted in each compaction batch. + // Deprecated in v3.6 and will be decommissioned in v3.7. + // TODO: Delete in v3.7 + 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"` ExperimentalWatchProgressNotifyInterval time.Duration `json:"experimental-watch-progress-notify-interval"` @@ -788,7 +793,9 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) { 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 fs.BoolVar(&cfg.ExperimentalEnableLeaseCheckpointPersist, "experimental-enable-lease-checkpoint-persist", false, "Enable persisting remainingTTL to prevent indefinite auto-renewal of long lived leases. Always enabled in v3.6. Should be used to ensure smooth upgrade from v3.5 clusters with this feature enabled. Requires experimental-enable-lease-checkpoint to be enabled.") - fs.IntVar(&cfg.ExperimentalCompactionBatchLimit, "experimental-compaction-batch-limit", cfg.ExperimentalCompactionBatchLimit, "Sets the maximum revisions deleted in each compaction batch.") + // TODO: delete in v3.7 + 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.") fs.DurationVar(&cfg.ExperimentalDowngradeCheckTime, "experimental-downgrade-check-time", cfg.ExperimentalDowngradeCheckTime, "Duration of time between two downgrade status checks.") diff --git a/server/embed/etcd.go b/server/embed/etcd.go index 4018a14d409..f5fa2c2c692 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -215,7 +215,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { UnsafeNoFsync: cfg.UnsafeNoFsync, EnableLeaseCheckpoint: cfg.ExperimentalEnableLeaseCheckpoint, LeaseCheckpointPersist: cfg.ExperimentalEnableLeaseCheckpointPersist, - CompactionBatchLimit: cfg.ExperimentalCompactionBatchLimit, + CompactionBatchLimit: cfg.CompactionBatchLimit, CompactionSleepInterval: cfg.ExperimentalCompactionSleepInterval, WatchProgressNotifyInterval: cfg.ExperimentalWatchProgressNotifyInterval, DowngradeCheckTime: cfg.ExperimentalDowngradeCheckTime, diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index f60277e1044..99cc279a1be 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -66,6 +66,7 @@ var ( "experimental-compact-hash-check-time": "--experimental-compact-hash-check-time is deprecated in 3.6 and will be decommissioned in 3.7. Use '--compact-hash-check-time' instead.", "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.", } ) @@ -179,6 +180,10 @@ func (cfg *config) parse(arguments []string) error { cfg.ec.CorruptCheckTime = cfg.ec.ExperimentalCorruptCheckTime } + if cfg.ec.FlagsExplicitlySet["experimental-compaction-batch-limit"] { + cfg.ec.CompactionBatchLimit = cfg.ec.ExperimentalCompactionBatchLimit + } + // `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 85cdc69eb08..1b9d5f49062 100644 --- a/server/etcdmain/config_test.go +++ b/server/etcdmain/config_test.go @@ -485,7 +485,6 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) { name string compactHashCheckTime string experimentalCompactHashCheckTime string - useConfigFile bool expectErr bool expectedCompactHashCheckTime time.Duration }{ @@ -536,19 +535,7 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) { yc.ExperimentalCompactHashCheckTime = experimentalCompactHashCheckTime } - b, err := yaml.Marshal(&yc) - if err != nil { - t.Fatal(err) - } - - tmpfile := mustCreateCfgFile(t, b) - defer os.Remove(tmpfile.Name()) - - cfgFromCmdLine := newConfig() - errFromCmdLine := cfgFromCmdLine.parse(cmdLineArgs) - - cfgFromFile := newConfig() - errFromFile := cfgFromFile.parse([]string{fmt.Sprintf("--config-file=%s", tmpfile.Name())}) + cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs) if tc.expectErr { if errFromCmdLine == nil || errFromFile == nil { @@ -557,7 +544,7 @@ func TestCompactHashCheckTimeFlagMigration(t *testing.T) { return } if errFromCmdLine != nil || errFromFile != nil { - t.Fatal(err) + t.Fatal("error parsing config") } if cfgFromCmdLine.ec.CompactHashCheckTime != tc.expectedCompactHashCheckTime { @@ -578,7 +565,6 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) { name string corruptCheckTime string experimentalCorruptCheckTime string - useConfigFile bool expectErr bool expectedCorruptCheckTime time.Duration }{ @@ -625,19 +611,7 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) { yc.ExperimentalCorruptCheckTime = experimentalCorruptCheckTime } - b, err := yaml.Marshal(&yc) - if err != nil { - t.Fatal(err) - } - - tmpfile := mustCreateCfgFile(t, b) - defer os.Remove(tmpfile.Name()) - - cfgFromCmdLine := newConfig() - errFromCmdLine := cfgFromCmdLine.parse(cmdLineArgs) - - cfgFromFile := newConfig() - errFromFile := cfgFromFile.parse([]string{fmt.Sprintf("--config-file=%s", tmpfile.Name())}) + cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs) if tc.expectErr { if errFromCmdLine == nil || errFromFile == nil { @@ -646,7 +620,7 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) { return } if errFromCmdLine != nil || errFromFile != nil { - t.Fatal(err) + t.Fatal("error parsing config") } if cfgFromCmdLine.ec.CorruptCheckTime != tc.expectedCorruptCheckTime { @@ -659,6 +633,92 @@ func TestCorruptCheckTimeFlagMigration(t *testing.T) { } } +// TestCompactionBatchLimitFlagMigration tests the migration from +// --experimental-compaction-batch-limit to --compaction-batch-limit +// TODO: delete in v3.7 +func TestCompactionBatchLimitFlagMigration(t *testing.T) { + testCases := []struct { + name string + compactionBatchLimit int + experimentalCompactionBatchLimit int + expectErr bool + expectedCompactionBatchLimit int + }{ + { + name: "cannot set both experimental flag and non experimental flag", + compactionBatchLimit: 1, + experimentalCompactionBatchLimit: 2, + expectErr: true, + }, + { + name: "can set experimental flag", + experimentalCompactionBatchLimit: 2, + expectedCompactionBatchLimit: 2, + }, + { + name: "can set non experimental flag", + compactionBatchLimit: 1, + expectedCompactionBatchLimit: 1, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cmdLineArgs := []string{} + yc := struct { + ExperimentalCompactionBatchLimit int `json:"experimental-compaction-batch-limit,omitempty"` + CompactionBatchLimit int `json:"compaction-batch-limit,omitempty"` + }{} + + if tc.compactionBatchLimit != 0 { + cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--compaction-batch-limit=%d", tc.compactionBatchLimit)) + yc.CompactionBatchLimit = tc.compactionBatchLimit + } + + if tc.experimentalCompactionBatchLimit != 0 { + cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-compaction-batch-limit=%d", tc.experimentalCompactionBatchLimit)) + yc.ExperimentalCompactionBatchLimit = tc.experimentalCompactionBatchLimit + } + + 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.CompactionBatchLimit != tc.expectedCompactionBatchLimit { + t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCompactionBatchLimit, cfgFromCmdLine.ec.CompactionBatchLimit) + } + if cfgFromFile.ec.CompactionBatchLimit != tc.expectedCompactionBatchLimit { + t.Errorf("expected CorruptCheckTime=%v, got %v", tc.expectedCompactionBatchLimit, cfgFromFile.ec.CompactionBatchLimit) + } + }) + } +} + +// TODO delete in v3.7 +func generateCfgsFromFileAndCmdLine(t *testing.T, yc interface{}, cmdLineArgs []string) (*config, error, *config, error) { + b, err := yaml.Marshal(&yc) + if err != nil { + t.Fatal(err) + } + + tmpfile := mustCreateCfgFile(t, b) + defer os.Remove(tmpfile.Name()) + + cfgFromCmdLine := newConfig() + errFromCmdLine := cfgFromCmdLine.parse(cmdLineArgs) + + cfgFromFile := newConfig() + errFromFile := cfgFromFile.parse([]string{fmt.Sprintf("--config-file=%s", tmpfile.Name())}) + return cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile +} + func mustCreateCfgFile(t *testing.T, b []byte) *os.File { tmpfile, err := os.CreateTemp("", "servercfg") if err != nil { @@ -753,6 +813,7 @@ func TestConfigFileDeprecatedOptions(t *testing.T) { ExperimentalCompactHashCheckTime time.Duration `json:"experimental-compact-hash-check-time,omitempty"` 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"` } testCases := []struct { @@ -772,11 +833,13 @@ func TestConfigFileDeprecatedOptions(t *testing.T) { ExperimentalCompactHashCheckTime: 2 * time.Minute, ExperimentalWarningUnaryRequestDuration: time.Second, ExperimentalCorruptCheckTime: time.Minute, + ExperimentalCompactionBatchLimit: 1, }, expectedFlags: map[string]struct{}{ "experimental-compact-hash-check-enabled": {}, "experimental-compact-hash-check-time": {}, "experimental-corrupt-check-time": {}, + "experimental-compaction-batch-limit": {}, }, }, { diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index 3dc98f29400..832716fb24f 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -288,7 +288,9 @@ Experimental feature: --experimental-enable-lease-checkpoint 'false' ExperimentalEnableLeaseCheckpoint enables primary lessor to persist lease remainingTTL to prevent indefinite auto-renewal of long lived leases. --experimental-compaction-batch-limit 1000 - ExperimentalCompactionBatchLimit sets the maximum revisions deleted in each compaction batch. + 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. + --compaction-batch-limit 1000 + CompactionBatchLimit sets the maximum revisions deleted in each compaction batch. --experimental-peer-skip-client-san-verification 'false' Skip verification of SAN field in client certificate for peer connections. --experimental-watch-progress-notify-interval '10m' diff --git a/tests/e2e/corrupt_test.go b/tests/e2e/corrupt_test.go index e1250206dcb..86535345e99 100644 --- a/tests/e2e/corrupt_test.go +++ b/tests/e2e/corrupt_test.go @@ -294,14 +294,18 @@ func testCompactHashCheckDetectCorruption(t *testing.T, useFeatureGate bool) { } func TestCompactHashCheckDetectCorruptionInterrupt(t *testing.T) { - testCompactHashCheckDetectCorruptionInterrupt(t, false) + testCompactHashCheckDetectCorruptionInterrupt(t, false, false) } func TestCompactHashCheckDetectCorruptionInterruptWithFeatureGate(t *testing.T) { - testCompactHashCheckDetectCorruptionInterrupt(t, true) + testCompactHashCheckDetectCorruptionInterrupt(t, true, false) } -func testCompactHashCheckDetectCorruptionInterrupt(t *testing.T, useFeatureGate bool) { +func TestCompactHashCheckDetectCorruptionInterruptWithExperimentalFlag(t *testing.T) { + testCompactHashCheckDetectCorruptionInterrupt(t, true, true) +} + +func testCompactHashCheckDetectCorruptionInterrupt(t *testing.T, useFeatureGate bool, useExperimentalFlag bool) { checkTime := time.Second e2e.BeforeTest(t) ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) @@ -325,6 +329,12 @@ func testCompactHashCheckDetectCorruptionInterrupt(t *testing.T, useFeatureGate } else { opts = append(opts, e2e.WithCompactHashCheckEnabled(true)) } + var compactionBatchLimit e2e.EPClusterOption + if useExperimentalFlag { + compactionBatchLimit = e2e.WithExperimentalCompactionBatchLimit(1) + } else { + compactionBatchLimit = e2e.WithCompactionBatchLimit(1) + } cfg := e2e.NewConfig(opts...) epc, err := e2e.InitEtcdProcessCluster(t, cfg) @@ -332,7 +342,7 @@ func testCompactHashCheckDetectCorruptionInterrupt(t *testing.T, useFeatureGate // Assign a node a very slow compaction speed, so that its compaction can be interrupted. err = epc.UpdateProcOptions(slowCompactionNodeIndex, t, - e2e.WithCompactionBatchLimit(1), + compactionBatchLimit, e2e.WithCompactionSleepInterval(1*time.Hour), ) require.NoError(t, err) diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go index 3b8ecf984c3..3da0ff9a8f2 100644 --- a/tests/framework/e2e/cluster.go +++ b/tests/framework/e2e/cluster.go @@ -373,6 +373,10 @@ func WithServerFeatureGate(featureName string, val bool) EPClusterOption { } func WithCompactionBatchLimit(limit int) EPClusterOption { + return func(c *EtcdProcessClusterConfig) { c.ServerConfig.CompactionBatchLimit = limit } +} + +func WithExperimentalCompactionBatchLimit(limit int) EPClusterOption { return func(c *EtcdProcessClusterConfig) { c.ServerConfig.ExperimentalCompactionBatchLimit = limit } } diff --git a/tests/robustness/failpoint/trigger.go b/tests/robustness/failpoint/trigger.go index 9ae9a808481..0f863fb54a6 100644 --- a/tests/robustness/failpoint/trigger.go +++ b/tests/robustness/failpoint/trigger.go @@ -75,7 +75,7 @@ func (t triggerCompact) Trigger(ctx context.Context, _ *testing.T, member e2e.Et } rev = resp.Header.Revision - if !t.multiBatchCompaction || rev > int64(clus.Cfg.ServerConfig.ExperimentalCompactionBatchLimit) { + if !t.multiBatchCompaction || rev > int64(clus.Cfg.ServerConfig.CompactionBatchLimit) { break } time.Sleep(50 * time.Millisecond) @@ -99,7 +99,7 @@ func (t triggerCompact) Available(config e2e.EtcdProcessClusterConfig, _ e2e.Etc // For multiBatchCompaction we need to guarantee that there are enough revisions between two compaction requests. // With addition of compaction requests to traffic this might be hard if experimental-compaction-batch-limit is too high. if t.multiBatchCompaction { - return config.ServerConfig.ExperimentalCompactionBatchLimit <= 10 + return config.ServerConfig.CompactionBatchLimit <= 10 } return true } diff --git a/tests/robustness/options/server_config_options.go b/tests/robustness/options/server_config_options.go index a0502a1f7dc..99fe2acef4a 100644 --- a/tests/robustness/options/server_config_options.go +++ b/tests/robustness/options/server_config_options.go @@ -28,7 +28,7 @@ func WithSnapshotCount(input ...uint64) e2e.EPClusterOption { func WithCompactionBatchLimit(input ...int) e2e.EPClusterOption { return func(c *e2e.EtcdProcessClusterConfig) { - c.ServerConfig.ExperimentalCompactionBatchLimit = input[internalRand.Intn(len(input))] + c.ServerConfig.CompactionBatchLimit = input[internalRand.Intn(len(input))] } }