Skip to content

Commit

Permalink
server: added duplicate warning-unary-request-duration flag
Browse files Browse the repository at this point in the history
--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 #13783

Signed-off-by: Bogdan Kanivets <bkanivets@apple.com>
  • Loading branch information
Bogdan Kanivets committed Nov 18, 2022
1 parent 6d8139d commit 663581b
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG/CHANGELOG-3.6.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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).
Expand Down
6 changes: 3 additions & 3 deletions server/embed/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -474,8 +476,6 @@ func NewConfig() *Config {
MaxConcurrentStreams: DefaultMaxConcurrentStreams,
ExperimentalWarningApplyDuration: DefaultWarningApplyDuration,

ExperimentalWarningUnaryRequestDuration: DefaultWarningUnaryRequestDuration,

GRPCKeepAliveMinTime: DefaultGRPCKeepAliveMinTime,
GRPCKeepAliveInterval: DefaultGRPCKeepAliveInterval,
GRPCKeepAliveTimeout: DefaultGRPCKeepAliveTimeout,
Expand Down
2 changes: 1 addition & 1 deletion server/embed/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
31 changes: 30 additions & 1 deletion server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
8 changes: 5 additions & 3 deletions server/etcdmain/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down
94 changes: 94 additions & 0 deletions tests/e2e/warning_logging_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
9 changes: 9 additions & 0 deletions tests/framework/e2e/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ type EtcdProcessClusterConfig struct {
CompactHashCheckTime time.Duration
GoFailEnabled bool
CompactionBatchLimit int

WarningUnaryRequestDuration time.Duration
ExperimentalWarningUnaryRequestDuration time.Duration
}

func DefaultConfig() *EtcdProcessClusterConfig {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 663581b

Please sign in to comment.