Skip configuration validation if the circuit breaker is disabled

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>
thanos-io · Jan 29, 2024 · 1d56ca5 · 1d56ca5
1 parent 808bdc9
commit 1d56ca5
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 27 deletions.
diff --git 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
@@ -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,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,
 			},

diff --git 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
@@ -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
 			},