Skip to content

Commit 58bab12

Browse files
committed
fix: create state only on resource event
In general we cleanup caches on delete event, so although in practice this probably not causing issues in theory it can happen that we receive events where there is no related custom resource. In that case we would create state, what can lead to a memory leak. Signed-off-by: Attila Mészáros <[email protected]>
1 parent a32f4c7 commit 58bab12

File tree

3 files changed

+52
-2
lines changed

3 files changed

+52
-2
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,16 @@ public synchronized void handleEvent(Event event) {
102102
try {
103103
log.debug("Received event: {}", event);
104104

105+
final var optionalState = resourceStateManager.getOrCreateOnResourceEvent(event);
106+
if (optionalState.isEmpty()) {
107+
log.debug(
108+
"Skipping event, since no state present and it is not a resource event. Resource ID:"
109+
+ " {}",
110+
event.getRelatedCustomResourceID());
111+
return;
112+
}
113+
var state = optionalState.orElseThrow();
105114
final var resourceID = event.getRelatedCustomResourceID();
106-
final var state = resourceStateManager.getOrCreate(event.getRelatedCustomResourceID());
107115
MDCUtils.addResourceIDInfo(resourceID);
108116
metrics.receivedEvent(event, metricsMetadata);
109117
handleEventMarking(event, state);

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceStateManager.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,33 @@
22

33
import java.util.List;
44
import java.util.Map;
5+
import java.util.Optional;
56
import java.util.concurrent.ConcurrentHashMap;
67
import java.util.stream.Collectors;
78

9+
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent;
10+
811
class ResourceStateManager {
912
// maybe we should have a way for users to specify a hint on the amount of CRs their reconciler
1013
// will process to avoid under- or over-sizing the state maps and avoid too many resizing that
1114
// take time and memory?
1215
private final Map<ResourceID, ResourceState> states = new ConcurrentHashMap<>(100);
1316

17+
public Optional<ResourceState> getOrCreateOnResourceEvent(Event event) {
18+
var resourceId = event.getRelatedCustomResourceID();
19+
var state = states.get(event.getRelatedCustomResourceID());
20+
if (state != null) {
21+
return Optional.of(state);
22+
}
23+
if (event instanceof ResourceEvent) {
24+
state = new ResourceState(resourceId);
25+
states.put(resourceId, state);
26+
return Optional.of(state);
27+
} else {
28+
return Optional.empty();
29+
}
30+
}
31+
1432
public ResourceState getOrCreate(ResourceID resourceID) {
1533
return states.computeIfAbsent(resourceID, ResourceState::new);
1634
}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,30 @@ void cancelScheduleOnceEventsOnSuccessfulExecution() {
276276
verify(retryTimerEventSourceMock, times(1)).cancelOnceSchedule(eq(crID));
277277
}
278278

279+
@Test
280+
void skipsGenericEventIfNoResourceEventReceivedBefore() {
281+
var crID = new ResourceID("test-cr", TEST_NAMESPACE);
282+
eventProcessor =
283+
spy(
284+
new EventProcessor(
285+
controllerConfiguration(null, LinearRateLimiter.deactivatedRateLimiter()),
286+
reconciliationDispatcherMock,
287+
eventSourceManagerMock,
288+
metricsMock));
289+
290+
verify(reconciliationDispatcherMock, timeout(100).times(0)).handleExecution(any());
291+
292+
eventProcessor.start();
293+
eventProcessor.handleEvent(new Event(crID));
294+
295+
await()
296+
.pollDelay(Duration.ofMillis(100))
297+
.untilAsserted(
298+
() -> {
299+
verify(reconciliationDispatcherMock, never()).handleExecution(any());
300+
});
301+
}
302+
279303
@Test
280304
void startProcessedMarkedEventReceivedBefore() {
281305
var crID = new ResourceID("test-cr", TEST_NAMESPACE);
@@ -287,7 +311,7 @@ void startProcessedMarkedEventReceivedBefore() {
287311
eventSourceManagerMock,
288312
metricsMock));
289313
when(controllerEventSourceMock.get(eq(crID))).thenReturn(Optional.of(testCustomResource()));
290-
eventProcessor.handleEvent(new Event(crID));
314+
eventProcessor.handleEvent(new ResourceEvent(ResourceAction.ADDED, crID, testCustomResource()));
291315

292316
verify(reconciliationDispatcherMock, timeout(100).times(0)).handleExecution(any());
293317

0 commit comments

Comments
 (0)