Skip to content

Commit

Permalink
Merge pull request #14803 from ahrtr/experimental_flag
Browse files Browse the repository at this point in the history
Graduate `--experimental-warning-unary-request-duration` to `--warning-unary-request-duration`
  • Loading branch information
ahrtr authored Nov 18, 2022
2 parents 07a3cfe + ebfcaae commit ddd65e7
Show file tree
Hide file tree
Showing 7 changed files with 150 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG/CHANGELOG-3.6.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0).
- Add [`etcd grpc-proxy --experimental-enable-grpc-logging`](https://github.com/etcd-io/etcd/pull/14266) flag to logging all grpc requests and responses.
- Add [`etcd --experimental-compact-hash-check-enabled --experimental-compact-hash-check-time`](https://github.com/etcd-io/etcd/issues/14039) flags to support enabling reliable corruption detection on compacted revisions.
- Add [Protection on maintenance request when auth is enabled](https://github.com/etcd-io/etcd/pull/14663).
- Graduated [`--experimental-warning-unary-request-duration` to `--warning-unary-request-duration`](https://github.com/etcd-io/etcd/pull/14414). Note the experimental flag is deprecated and will be decommissioned in v3.7.

### etcd grpc-proxy

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 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
}
6 changes: 4 additions & 2 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 @@ -261,7 +263,7 @@ Experimental feature:
--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.
Set time duration after which a warning is generated if a unary request takes more than this duration. It's 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
92 changes: 92 additions & 0 deletions tests/e2e/promote_experimental_flag_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// 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/assert"
"github.com/stretchr/testify/require"

"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.WithClusterSize(1),
e2e.WithWarningUnaryRequestDuration(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.Client, 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.WithClusterSize(1),
e2e.WithExperimentalWarningUnaryRequestDuration(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.Client, 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.WithClusterSize(1),
e2e.WithWarningUnaryRequestDuration(time.Second),
e2e.WithExperimentalWarningUnaryRequestDuration(time.Second),
)
if err == nil {
t.Fatal("Expected process to fail")
}
}
19 changes: 19 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 @@ -310,6 +313,16 @@ func WithGoFailEnabled(enabled bool) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.GoFailEnabled = enabled }
}

func WithWarningUnaryRequestDuration(time time.Duration) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.WarningUnaryRequestDuration = time }
}

// WithExperimentalWarningUnaryRequestDuration sets a value for `-experimental-warning-unary-request-duration`.
// TODO(ahrtr): remove this function when the corresponding experimental flag is decommissioned.
func WithExperimentalWarningUnaryRequestDuration(time time.Duration) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.ExperimentalWarningUnaryRequestDuration = time }
}

func WithCompactionBatchLimit(limit int) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.CompactionBatchLimit = limit }
}
Expand Down Expand Up @@ -527,6 +540,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 ddd65e7

Please sign in to comment.