Skip to content

Commit

Permalink
Skip configuration validation if the circuit breaker is disabled
Browse files Browse the repository at this point in the history
Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>
  • Loading branch information
damnever committed Jan 29, 2024
1 parent 808bdc9 commit b00e6ed
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 27 deletions.
12 changes: 7 additions & 5 deletions pkg/cacheutil/memcached_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
29 changes: 12 additions & 17 deletions pkg/cacheutil/memcached_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -46,8 +44,6 @@ func TestMemcachedClientConfig_validate(t *testing.T) {
Addresses: []string{},
MaxAsyncConcurrency: 1,
DNSProviderUpdateInterval: time.Second,
SetAsyncCircuitBreakerConsecutiveFailures: 1,
SetAsyncCircuitBreakerFailurePercent: 0.5,
},
expected: errMemcachedConfigNoAddrs,
},
Expand All @@ -56,44 +52,43 @@ 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,
},
"should fail on dns_provider_update_interval <= 0": {
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,
},
expected: errCircuitBreakerFailurePercentInvalid,
},
"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,
},
Expand Down
12 changes: 7 additions & 5 deletions pkg/cacheutil/redis_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/cacheutil/redis_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,23 @@ 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.Addr = "127.0.0.1:6789"
cfg.SetAsyncCircuitBreakerEnabled = true
cfg.SetAsyncCircuitBreakerConsecutiveFailures = 0
return cfg
},
Expand All @@ -210,6 +223,8 @@ func TestValidateRedisConfig(t *testing.T) {
name: "invalidCircuitBreakerFailurePercent",
config: func() RedisClientConfig {
cfg := DefaultRedisClientConfig
cfg.Addr = "127.0.0.1:6789"
cfg.SetAsyncCircuitBreakerEnabled = true
cfg.SetAsyncCircuitBreakerFailurePercent = 0
return cfg
},
Expand Down

0 comments on commit b00e6ed

Please sign in to comment.