Skip to content

Commit 5f3b287

Browse files
Addressed comments
1 parent f163a9d commit 5f3b287

File tree

4 files changed

+46
-39
lines changed

4 files changed

+46
-39
lines changed

grpc-circuitbreaker-utils/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ dependencies {
1414
implementation("org.slf4j:slf4j-api:1.7.36")
1515
implementation("io.github.resilience4j:resilience4j-circuitbreaker:1.7.1")
1616
implementation("com.typesafe:config:1.4.2")
17+
implementation("com.google.guava:guava:32.0.1-jre")
1718

1819
annotationProcessor("org.projectlombok:lombok:1.18.24")
1920
compileOnly("org.projectlombok:lombok:1.18.24")

grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class CircuitBreakerConfigParser {
2828
private static final String SLIDING_WINDOW_TYPE = "slidingWindowType";
2929
public static final String ENABLED = "enabled";
3030
public static final String DEFAULT_THRESHOLDS = "defaultThresholds";
31-
private static final Set<String> NON_THRESHOLD_KEYS = Set.of(ENABLED);
31+
private static final Set<String> NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS);
3232

3333
public static <T> CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder<T> parseConfig(
3434
Config config) {

grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ public static ResilienceCircuitBreakerInterceptor getResilienceCircuitBreakerInt
2121
resilienceCircuitBreakerRegistry,
2222
resilienceCircuitBreakerConfigMap,
2323
ResilienceCircuitBreakerConfigConverter.getDisabledKeys(
24-
circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()));
24+
circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()),
25+
circuitBreakerConfiguration.getDefaultThresholds().isEnabled());
2526
return new ResilienceCircuitBreakerInterceptor(
2627
circuitBreakerConfiguration, clock, resilienceCircuitBreakerProvider);
2728
}
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
package org.hypertrace.circuitbreaker.grpcutils.resilience;
22

3+
import com.google.common.cache.CacheBuilder;
4+
import com.google.common.cache.CacheLoader;
5+
import com.google.common.cache.LoadingCache;
36
import io.github.resilience4j.circuitbreaker.CircuitBreaker;
47
import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig;
58
import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry;
69
import java.util.List;
710
import java.util.Map;
811
import java.util.Optional;
9-
import java.util.concurrent.ConcurrentHashMap;
12+
import java.util.concurrent.TimeUnit;
1013
import lombok.extern.slf4j.Slf4j;
11-
import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser;
1214

1315
/** Utility class to provide Resilience4j CircuitBreaker */
1416
@Slf4j
@@ -17,51 +19,42 @@ class ResilienceCircuitBreakerProvider {
1719
private static final String SHARED_KEY = "SHARED_KEY";
1820
private final CircuitBreakerRegistry circuitBreakerRegistry;
1921
private final Map<String, CircuitBreakerConfig> circuitBreakerConfigMap;
20-
private final Map<String, CircuitBreaker> circuitBreakerCache = new ConcurrentHashMap<>();
2122
private final List<String> disabledKeys;
2223
private final boolean defaultEnabled;
2324

25+
// LoadingCache to manage CircuitBreaker instances with automatic loading and eviction
26+
private final LoadingCache<String, Optional<CircuitBreaker>> circuitBreakerCache =
27+
CacheBuilder.newBuilder()
28+
.expireAfterAccess(60, TimeUnit.MINUTES) // Auto-evict after 60 minutes
29+
.maximumSize(10000) // Limit max cache size
30+
.build(
31+
new CacheLoader<>() {
32+
@Override
33+
public Optional<CircuitBreaker> load(String key) {
34+
return buildNewCircuitBreaker(key);
35+
}
36+
});
37+
2438
public ResilienceCircuitBreakerProvider(
2539
CircuitBreakerRegistry circuitBreakerRegistry,
2640
Map<String, CircuitBreakerConfig> circuitBreakerConfigMap,
27-
List<String> disabledKeys) {
41+
List<String> disabledKeys,
42+
boolean defaultEnabled) {
2843
this.circuitBreakerRegistry = circuitBreakerRegistry;
2944
this.circuitBreakerConfigMap = circuitBreakerConfigMap;
3045
this.disabledKeys = disabledKeys;
31-
this.defaultEnabled = !disabledKeys.contains(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS);
46+
this.defaultEnabled = defaultEnabled;
3247
}
3348

3449
public Optional<CircuitBreaker> getCircuitBreaker(String circuitBreakerKey) {
3550
if (disabledKeys.contains(circuitBreakerKey)) {
3651
return Optional.empty();
3752
}
38-
return Optional.ofNullable(
39-
circuitBreakerCache.computeIfAbsent(
40-
circuitBreakerKey,
41-
key -> {
42-
CircuitBreaker circuitBreaker =
43-
getCircuitBreakerFromConfigMap(circuitBreakerKey, defaultEnabled);
44-
// If no circuit breaker is created return empty
45-
if (circuitBreaker == null) {
46-
return null; // Ensures cache does not store null entries
47-
}
48-
attachListeners(circuitBreaker);
49-
return circuitBreaker;
50-
}));
53+
return circuitBreakerCache.getUnchecked(circuitBreakerKey);
5154
}
5255

5356
public Optional<CircuitBreaker> getSharedCircuitBreaker() {
54-
if (!defaultEnabled) {
55-
return Optional.empty();
56-
}
57-
return Optional.of(
58-
circuitBreakerCache.computeIfAbsent(
59-
SHARED_KEY,
60-
key -> {
61-
CircuitBreaker circuitBreaker = circuitBreakerRegistry.circuitBreaker(SHARED_KEY);
62-
attachListeners(circuitBreaker);
63-
return circuitBreaker;
64-
}));
57+
return defaultEnabled ? getCircuitBreaker(SHARED_KEY) : Optional.empty();
6558
}
6659

6760
private static void attachListeners(CircuitBreaker circuitBreaker) {
@@ -86,14 +79,26 @@ private static void attachListeners(CircuitBreaker circuitBreaker) {
8679
event.getCircuitBreakerName()));
8780
}
8881

89-
private CircuitBreaker getCircuitBreakerFromConfigMap(
90-
String circuitBreakerKey, boolean defaultEnabled) {
82+
private Optional<CircuitBreaker> buildNewCircuitBreaker(String circuitBreakerKey) {
9183
return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey))
92-
.map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config))
93-
.orElseGet(
94-
() ->
95-
defaultEnabled
96-
? circuitBreakerRegistry.circuitBreaker(circuitBreakerKey)
97-
: null); // Return null if default is disabled
84+
.map(
85+
config -> {
86+
CircuitBreaker circuitBreaker =
87+
circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config);
88+
attachListeners(circuitBreaker); // Attach listeners here
89+
return circuitBreaker;
90+
})
91+
.or(
92+
() -> {
93+
if (defaultEnabled) {
94+
CircuitBreaker circuitBreaker =
95+
circuitBreakerRegistry.circuitBreaker(circuitBreakerKey);
96+
attachListeners(
97+
circuitBreaker); // Attach listeners here for default circuit breaker
98+
return Optional.of(circuitBreaker);
99+
} else {
100+
return Optional.empty();
101+
}
102+
});
98103
}
99104
}

0 commit comments

Comments
 (0)