From 7e8ebf7727809a701e112310f68ef0e323dcb97c Mon Sep 17 00:00:00 2001 From: Bogdan Kanivets Date: Thu, 1 Sep 2022 11:00:18 -0700 Subject: [PATCH] server: added duplicate warning-unary-request-duration flag --warning-unary-request-duration is a duplicate of --experimental-warning-unary-request-duration experimental-warning-unary-request-duration will be removed in v3.7. fixes https://github.com/etcd-io/etcd/issues/13783 Signed-off-by: Bogdan Kanivets --- CHANGELOG/CHANGELOG-3.6.md | 2 +- server/embed/config.go | 6 +- server/embed/etcd.go | 2 +- server/etcdmain/config.go | 31 +++++++++- server/etcdmain/help.go | 8 ++- tests/e2e/warning_logging_test.go | 94 +++++++++++++++++++++++++++++++ tests/framework/e2e/cluster.go | 9 +++ 7 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 tests/e2e/warning_logging_test.go diff --git a/CHANGELOG/CHANGELOG-3.6.md b/CHANGELOG/CHANGELOG-3.6.md index e51c81985..b3e24b10d 100644 --- a/CHANGELOG/CHANGELOG-3.6.md +++ b/CHANGELOG/CHANGELOG-3.6.md @@ -19,7 +19,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0). - Deprecated [V2 discovery](https://etcd.io/docs/v3.5/dev-internal/discovery_protocol/). - Deprecated [SetKeepAlive and SetKeepAlivePeriod in limitListenerConn](https://github.com/etcd-io/etcd/pull/14356). -- Removed [etcdctl defrag --data-dir](https://github.com/etcd-io/etcd/pull/13793). +- Promoted [`--experimental-warning-unary-request-duration` to `--warning-unary-request-duration`](https://github.com/etcd-io/etcd/pull/14414). Note the experimental has already been deprecated and will be decommissioned in v3.7.- Removed [etcdctl defrag --data-dir](https://github.com/etcd-io/etcd/pull/13793). - Removed [etcdctl snapshot status](https://github.com/etcd-io/etcd/pull/13809). - Removed [etcdctl snapshot restore](https://github.com/etcd-io/etcd/pull/13809). - Removed [etcdutl snapshot save](https://github.com/etcd-io/etcd/pull/13809). diff --git a/server/embed/config.go b/server/embed/config.go index b6aa8da57..fc192519d 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -342,8 +342,10 @@ type Config struct { // ExperimentalBootstrapDefragThresholdMegabytes is the minimum number of megabytes needed to be freed for etcd server to // consider running defrag during bootstrap. Needs to be set to non-zero value to take effect. ExperimentalBootstrapDefragThresholdMegabytes uint `json:"experimental-bootstrap-defrag-threshold-megabytes"` - // ExperimentalWarningUnaryRequestDuration is the time duration after which a warning is generated if applying + // WarningUnaryRequestDuration is the time duration after which a warning is generated if applying // unary request takes more time than this value. + WarningUnaryRequestDuration time.Duration `json:"warning-unary-request-duration"` + // ExperimentalWarningUnaryRequestDuration is deprecated, please use WarningUnaryRequestDuration instead. ExperimentalWarningUnaryRequestDuration time.Duration `json:"experimental-warning-unary-request-duration"` // ExperimentalMaxLearners sets a limit to the number of learner members that can exist in the cluster membership. ExperimentalMaxLearners int `json:"experimental-max-learners"` @@ -474,8 +476,6 @@ func NewConfig() *Config { MaxConcurrentStreams: DefaultMaxConcurrentStreams, ExperimentalWarningApplyDuration: DefaultWarningApplyDuration, - ExperimentalWarningUnaryRequestDuration: DefaultWarningUnaryRequestDuration, - GRPCKeepAliveMinTime: DefaultGRPCKeepAliveMinTime, GRPCKeepAliveInterval: DefaultGRPCKeepAliveInterval, GRPCKeepAliveTimeout: DefaultGRPCKeepAliveTimeout, diff --git a/server/embed/etcd.go b/server/embed/etcd.go index 88178afc6..e3863ee62 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -217,7 +217,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { WatchProgressNotifyInterval: cfg.ExperimentalWatchProgressNotifyInterval, DowngradeCheckTime: cfg.ExperimentalDowngradeCheckTime, WarningApplyDuration: cfg.ExperimentalWarningApplyDuration, - WarningUnaryRequestDuration: cfg.ExperimentalWarningUnaryRequestDuration, + WarningUnaryRequestDuration: cfg.WarningUnaryRequestDuration, ExperimentalMemoryMlock: cfg.ExperimentalMemoryMlock, ExperimentalTxnModeWriteWithSharedBuffer: cfg.ExperimentalTxnModeWriteWithSharedBuffer, ExperimentalBootstrapDefragThresholdMegabytes: cfg.ExperimentalBootstrapDefragThresholdMegabytes, diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index b14191a95..42b0da694 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -17,10 +17,12 @@ package etcdmain import ( + "errors" "flag" "fmt" "os" "runtime" + "time" "go.etcd.io/etcd/api/v3/version" "go.etcd.io/etcd/client/pkg/v3/logutil" @@ -270,7 +272,8 @@ func newConfig() *config { fs.DurationVar(&cfg.ec.ExperimentalWatchProgressNotifyInterval, "experimental-watch-progress-notify-interval", cfg.ec.ExperimentalWatchProgressNotifyInterval, "Duration of periodic watch progress notifications.") fs.DurationVar(&cfg.ec.ExperimentalDowngradeCheckTime, "experimental-downgrade-check-time", cfg.ec.ExperimentalDowngradeCheckTime, "Duration of time between two downgrade status check.") fs.DurationVar(&cfg.ec.ExperimentalWarningApplyDuration, "experimental-warning-apply-duration", cfg.ec.ExperimentalWarningApplyDuration, "Time duration after which a warning is generated if request takes more time.") - fs.DurationVar(&cfg.ec.ExperimentalWarningUnaryRequestDuration, "experimental-warning-unary-request-duration", cfg.ec.ExperimentalWarningUnaryRequestDuration, "Time duration after which a warning is generated if a unary request takes more time.") + fs.DurationVar(&cfg.ec.WarningUnaryRequestDuration, "warning-unary-request-duration", cfg.ec.WarningUnaryRequestDuration, "Time duration after which a warning is generated if a unary request takes more time.") + fs.DurationVar(&cfg.ec.ExperimentalWarningUnaryRequestDuration, "experimental-warning-unary-request-duration", cfg.ec.ExperimentalWarningUnaryRequestDuration, "Time duration after which a warning is generated if a unary request takes more time. It's already deprecated, and will be decommissioned in v3.7. Use --warning-unary-request-duration instead.") fs.BoolVar(&cfg.ec.ExperimentalMemoryMlock, "experimental-memory-mlock", cfg.ec.ExperimentalMemoryMlock, "Enable to enforce etcd pages (in particular bbolt) to stay in RAM.") fs.BoolVar(&cfg.ec.ExperimentalTxnModeWriteWithSharedBuffer, "experimental-txn-mode-write-with-shared-buffer", true, "Enable the write transaction to use a shared buffer in its readonly check operations.") fs.UintVar(&cfg.ec.ExperimentalBootstrapDefragThresholdMegabytes, "experimental-bootstrap-defrag-threshold-megabytes", 0, "Enable the defrag during etcd server bootstrap on condition that it will free at least the provided threshold of disk space. Needs to be set to non-zero value to take effect.") @@ -335,6 +338,11 @@ func (cfg *config) parse(arguments []string) error { cfg.ec.V2Deprecation = cconfig.V2_DEPR_DEFAULT } + cfg.ec.WarningUnaryRequestDuration, perr = cfg.parseWarningUnaryRequestDuration() + if perr != nil { + return perr + } + // now logger is set up return err } @@ -422,3 +430,24 @@ func (cfg *config) validate() error { } return cfg.ec.Validate() } + +func (cfg *config) parseWarningUnaryRequestDuration() (time.Duration, error) { + if cfg.ec.ExperimentalWarningUnaryRequestDuration != 0 && cfg.ec.WarningUnaryRequestDuration != 0 { + return 0, errors.New( + "both --experimental-warning-unary-request-duration and --warning-unary-request-duration flags are set. " + + "Use only --warning-unary-request-duration") + } + + if cfg.ec.WarningUnaryRequestDuration != 0 { + return cfg.ec.WarningUnaryRequestDuration, nil + } + + if cfg.ec.ExperimentalWarningUnaryRequestDuration != 0 { + cfg.ec.GetLogger().Warn( + "--experimental-warning-unary-request-duration is deprecated, and will be decommissioned in v3.7. " + + "Use --warning-unary-request-duration instead.") + return cfg.ec.ExperimentalWarningUnaryRequestDuration, nil + } + + return embed.DefaultWarningUnaryRequestDuration, nil +} diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index a7f569a8e..612e9f7cc 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -227,7 +227,9 @@ Logging: --enable-log-rotation 'false' Enable log rotation of a single log-outputs file target. --log-rotation-config-json '{"maxsize": 100, "maxage": 0, "maxbackups": 0, "localtime": false, "compress": false}' - Configures log rotation if enabled with a JSON logger config. MaxSize(MB), MaxAge(days,0=no limit), MaxBackups(0=no limit), LocalTime(use computers local time), Compress(gzip)". + Configures log rotation if enabled with a JSON logger config. MaxSize(MB), MaxAge(days,0=no limit), MaxBackups(0=no limit), LocalTime(use computers local time), Compress(gzip)". + --warning-unary-request-duration '300ms' + Set time duration after which a warning is logged if a unary request takes more than this duration. Experimental distributed tracing: --experimental-enable-distributed-tracing 'false' @@ -260,8 +262,8 @@ Experimental feature: Enable the write transaction to use a shared buffer in its readonly check operations. --experimental-bootstrap-defrag-threshold-megabytes Enable the defrag during etcd server bootstrap on condition that it will free at least the provided threshold of disk space. Needs to be set to non-zero value to take effect. - --experimental-warning-unary-request-duration '300ms' - Set time duration after which a warning is generated if a unary request takes more than this duration. + --experimental-warning-unary-request-duration '0s' + Set time duration after which a warning is generated if a unary request takes more than this duration. It's already deprecated, and will be decommissioned in v3.7. Use --warning-unary-request-duration instead. --experimental-max-learners '1' Set the max number of learner members allowed in the cluster membership. --experimental-wait-cluster-ready-timeout '5s' diff --git a/tests/e2e/warning_logging_test.go b/tests/e2e/warning_logging_test.go new file mode 100644 index 000000000..139a6a4af --- /dev/null +++ b/tests/e2e/warning_logging_test.go @@ -0,0 +1,94 @@ +// Copyright 2022 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package e2e + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/stretchr/testify/assert" + "go.etcd.io/etcd/tests/v3/framework/config" + "go.etcd.io/etcd/tests/v3/framework/e2e" +) + +func TestWarningApplyDuration(t *testing.T) { + e2e.BeforeTest(t) + + epc, err := e2e.NewEtcdProcessCluster(context.TODO(), t, &e2e.EtcdProcessClusterConfig{ + ClusterSize: 1, + //Set very small duration to trigger warning + WarningUnaryRequestDuration: time.Microsecond, + }) + if err != nil { + t.Fatalf("could not start etcd process cluster (%v)", err) + } + t.Cleanup(func() { + if errC := epc.Close(); errC != nil { + t.Fatalf("error closing etcd processes (%v)", errC) + } + }) + + cc, err := e2e.NewEtcdctl(epc.Cfg, epc.EndpointsV3()) + require.NoError(t, err) + err = cc.Put(context.TODO(), "foo", "bar", config.PutOptions{}) + assert.NoError(t, err, "error on put") + + // verify warning + e2e.AssertProcessLogs(t, epc.Procs[0], "request stats") +} + +// TODO: this test is a duplicate of TestWarningApplyDuration except it uses --experimental-warning-unary-request-duration +// Remove this test after --experimental-warning-unary-request-duration flag is removed. +func TestExperimentalWarningApplyDuration(t *testing.T) { + e2e.BeforeTest(t) + + epc, err := e2e.NewEtcdProcessCluster(context.TODO(), t, &e2e.EtcdProcessClusterConfig{ + ClusterSize: 1, + //Set very small duration to trigger warning + ExperimentalWarningUnaryRequestDuration: time.Microsecond, + }) + if err != nil { + t.Fatalf("could not start etcd process cluster (%v)", err) + } + t.Cleanup(func() { + if errC := epc.Close(); errC != nil { + t.Fatalf("error closing etcd processes (%v)", errC) + } + }) + + cc, err := e2e.NewEtcdctl(epc.Cfg, epc.EndpointsV3()) + require.NoError(t, err) + err = cc.Put(context.TODO(), "foo", "bar", config.PutOptions{}) + assert.NoError(t, err, "error on put") + + // verify warning + e2e.AssertProcessLogs(t, epc.Procs[0], "request stats") +} + +func TestBothWarningApplyDurationFlagsFail(t *testing.T) { + e2e.BeforeTest(t) + + _, err := e2e.NewEtcdProcessCluster(context.TODO(), t, &e2e.EtcdProcessClusterConfig{ + ClusterSize: 1, + WarningUnaryRequestDuration: time.Second, + ExperimentalWarningUnaryRequestDuration: time.Second, + }) + if err == nil { + t.Fatal("Expected process to fail") + } +} diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go index e69081f19..2650e156c 100644 --- a/tests/framework/e2e/cluster.go +++ b/tests/framework/e2e/cluster.go @@ -179,6 +179,9 @@ type EtcdProcessClusterConfig struct { CompactHashCheckTime time.Duration GoFailEnabled bool CompactionBatchLimit int + + WarningUnaryRequestDuration time.Duration + ExperimentalWarningUnaryRequestDuration time.Duration } func DefaultConfig() *EtcdProcessClusterConfig { @@ -527,6 +530,12 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in if cfg.CompactionBatchLimit != 0 { args = append(args, "--experimental-compaction-batch-limit", fmt.Sprintf("%d", cfg.CompactionBatchLimit)) } + if cfg.WarningUnaryRequestDuration != 0 { + args = append(args, "--warning-unary-request-duration", cfg.WarningUnaryRequestDuration.String()) + } + if cfg.ExperimentalWarningUnaryRequestDuration != 0 { + args = append(args, "--experimental-warning-unary-request-duration", cfg.ExperimentalWarningUnaryRequestDuration.String()) + } envVars := map[string]string{} for key, value := range cfg.EnvVars { envVars[key] = value