From 1d56ca58b4ab74a76699e2c15005ab96012c0002 Mon Sep 17 00:00:00 2001 From: "Xiaochao Dong (@damnever)" Date: Mon, 29 Jan 2024 18:55:16 +0800 Subject: [PATCH] Skip configuration validation if the circuit breaker is disabled Signed-off-by: Xiaochao Dong (@damnever) --- pkg/cacheutil/memcached_client.go | 12 ++++++----- pkg/cacheutil/memcached_client_test.go | 29 +++++++++++--------------- pkg/cacheutil/redis_client.go | 12 ++++++----- pkg/cacheutil/redis_client_test.go | 13 ++++++++++++ 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/pkg/cacheutil/memcached_client.go b/pkg/cacheutil/memcached_client.go index 57547099323..1b6a681b1b2 100644 --- a/pkg/cacheutil/memcached_client.go +++ b/pkg/cacheutil/memcached_client.go @@ -184,11 +184,13 @@ func (c *MemcachedClientConfig) validate() error { return errMemcachedMaxAsyncConcurrencyNotPositive } - if c.SetAsyncCircuitBreakerConsecutiveFailures == 0 { - return errCircuitBreakerConsecutiveFailuresNotPositive - } - if c.SetAsyncCircuitBreakerFailurePercent <= 0 || c.SetAsyncCircuitBreakerFailurePercent > 1 { - return errCircuitBreakerFailurePercentInvalid + if c.SetAsyncCircuitBreakerEnabled { + if c.SetAsyncCircuitBreakerConsecutiveFailures == 0 { + return errCircuitBreakerConsecutiveFailuresNotPositive + } + if c.SetAsyncCircuitBreakerFailurePercent <= 0 || c.SetAsyncCircuitBreakerFailurePercent > 1 { + return errCircuitBreakerFailurePercentInvalid + } } return nil } diff --git a/pkg/cacheutil/memcached_client_test.go b/pkg/cacheutil/memcached_client_test.go index 297ff7d2815..e6e81ebb7e9 100644 --- a/pkg/cacheutil/memcached_client_test.go +++ b/pkg/cacheutil/memcached_client_test.go @@ -36,8 +36,6 @@ func TestMemcachedClientConfig_validate(t *testing.T) { Addresses: []string{"127.0.0.1:11211"}, MaxAsyncConcurrency: 1, DNSProviderUpdateInterval: time.Second, - SetAsyncCircuitBreakerConsecutiveFailures: 1, - SetAsyncCircuitBreakerFailurePercent: 0.5, }, expected: nil, }, @@ -46,8 +44,6 @@ func TestMemcachedClientConfig_validate(t *testing.T) { Addresses: []string{}, MaxAsyncConcurrency: 1, DNSProviderUpdateInterval: time.Second, - SetAsyncCircuitBreakerConsecutiveFailures: 1, - SetAsyncCircuitBreakerFailurePercent: 0.5, }, expected: errMemcachedConfigNoAddrs, }, @@ -56,8 +52,6 @@ func TestMemcachedClientConfig_validate(t *testing.T) { Addresses: []string{"127.0.0.1:11211"}, MaxAsyncConcurrency: 0, DNSProviderUpdateInterval: time.Second, - SetAsyncCircuitBreakerConsecutiveFailures: 1, - SetAsyncCircuitBreakerFailurePercent: 0.5, }, expected: errMemcachedMaxAsyncConcurrencyNotPositive, }, @@ -65,25 +59,25 @@ func TestMemcachedClientConfig_validate(t *testing.T) { config: MemcachedClientConfig{ Addresses: []string{"127.0.0.1:11211"}, MaxAsyncConcurrency: 1, - SetAsyncCircuitBreakerConsecutiveFailures: 1, - SetAsyncCircuitBreakerFailurePercent: 0.5, }, expected: errMemcachedDNSUpdateIntervalNotPositive, }, "should fail on circuit_breaker_consecutive_failures = 0": { config: MemcachedClientConfig{ - Addresses: []string{"127.0.0.1:11211"}, - MaxAsyncConcurrency: 1, - DNSProviderUpdateInterval: time.Second, + Addresses: []string{"127.0.0.1:11211"}, + MaxAsyncConcurrency: 1, + DNSProviderUpdateInterval: time.Second, + SetAsyncCircuitBreakerEnabled: true, SetAsyncCircuitBreakerConsecutiveFailures: 0, }, expected: errCircuitBreakerConsecutiveFailuresNotPositive, }, "should fail on circuit_breaker_failure_percent <= 0": { config: MemcachedClientConfig{ - Addresses: []string{"127.0.0.1:11211"}, - MaxAsyncConcurrency: 1, - DNSProviderUpdateInterval: time.Second, + Addresses: []string{"127.0.0.1:11211"}, + MaxAsyncConcurrency: 1, + DNSProviderUpdateInterval: time.Second, + SetAsyncCircuitBreakerEnabled: true, SetAsyncCircuitBreakerConsecutiveFailures: 1, SetAsyncCircuitBreakerFailurePercent: 0, }, @@ -91,9 +85,10 @@ func TestMemcachedClientConfig_validate(t *testing.T) { }, "should fail on circuit_breaker_failure_percent >= 1": { config: MemcachedClientConfig{ - Addresses: []string{"127.0.0.1:11211"}, - MaxAsyncConcurrency: 1, - DNSProviderUpdateInterval: time.Second, + Addresses: []string{"127.0.0.1:11211"}, + MaxAsyncConcurrency: 1, + DNSProviderUpdateInterval: time.Second, + SetAsyncCircuitBreakerEnabled: true, SetAsyncCircuitBreakerConsecutiveFailures: 1, SetAsyncCircuitBreakerFailurePercent: 1.1, }, diff --git a/pkg/cacheutil/redis_client.go b/pkg/cacheutil/redis_client.go index 6a0a9aedc67..b6c63bc619b 100644 --- a/pkg/cacheutil/redis_client.go +++ b/pkg/cacheutil/redis_client.go @@ -155,11 +155,13 @@ func (c *RedisClientConfig) validate() error { } } - if c.SetAsyncCircuitBreakerConsecutiveFailures == 0 { - return errCircuitBreakerConsecutiveFailuresNotPositive - } - if c.SetAsyncCircuitBreakerFailurePercent <= 0 || c.SetAsyncCircuitBreakerFailurePercent > 1 { - return errCircuitBreakerFailurePercentInvalid + if c.SetAsyncCircuitBreakerEnabled { + if c.SetAsyncCircuitBreakerConsecutiveFailures == 0 { + return errCircuitBreakerConsecutiveFailuresNotPositive + } + if c.SetAsyncCircuitBreakerFailurePercent <= 0 || c.SetAsyncCircuitBreakerFailurePercent > 1 { + return errCircuitBreakerFailurePercentInvalid + } } return nil } diff --git a/pkg/cacheutil/redis_client_test.go b/pkg/cacheutil/redis_client_test.go index 6e755235fd7..5a6299ca8ad 100644 --- a/pkg/cacheutil/redis_client_test.go +++ b/pkg/cacheutil/redis_client_test.go @@ -197,10 +197,22 @@ func TestValidateRedisConfig(t *testing.T) { }, expect_err: true, }, + { + name: "SetAsyncCircuitBreakerDisabled", + config: func() RedisClientConfig { + cfg := DefaultRedisClientConfig + cfg.Addr = "127.0.0.1:6789" + cfg.SetAsyncCircuitBreakerEnabled = false + cfg.SetAsyncCircuitBreakerConsecutiveFailures = 0 + return cfg + }, + expect_err: false, + }, { name: "invalidCircuitBreakerFailurePercent", config: func() RedisClientConfig { cfg := DefaultRedisClientConfig + cfg.SetAsyncCircuitBreakerEnabled = true cfg.SetAsyncCircuitBreakerConsecutiveFailures = 0 return cfg }, @@ -210,6 +222,7 @@ func TestValidateRedisConfig(t *testing.T) { name: "invalidCircuitBreakerFailurePercent", config: func() RedisClientConfig { cfg := DefaultRedisClientConfig + cfg.SetAsyncCircuitBreakerEnabled = true cfg.SetAsyncCircuitBreakerFailurePercent = 0 return cfg },