Skip to content

Commit 60660ed

Browse files
csvirimetacosm
authored andcommitted
feat: Only one scheduled event (re-schedul / retry) at one time (#609)
1 parent 7fc1c64 commit 60660ed

File tree

4 files changed

+32
-18
lines changed

4 files changed

+32
-18
lines changed

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -199,18 +199,15 @@ void eventProcessingFinished(
199199
// If a delete event present at this phase, it was received during reconciliation.
200200
// So we either removed the finalizer during reconciliation or we don't use finalizers.
201201
// Either way we don't want to retry.
202-
if (retry != null && postExecutionControl.exceptionDuringExecution() &&
202+
if (isRetryConfigured() && postExecutionControl.exceptionDuringExecution() &&
203203
!eventMarker.deleteEventPresent(customResourceID)) {
204204
handleRetryOnException(executionScope);
205205
// todo revisit monitoring since events are not present anymore
206206
// final var monitor = monitor(); executionScope.getEvents().forEach(e ->
207207
// monitor.failedEvent(executionScope.getCustomResourceID(), e));
208208
return;
209209
}
210-
211-
if (retry != null) {
212-
handleSuccessfulExecutionRegardingRetry(executionScope);
213-
}
210+
cleanupOnSuccessfulExecution(executionScope);
214211
if (eventMarker.deleteEventPresent(customResourceID)) {
215212
cleanupForDeletedEvent(executionScope.getCustomResourceID());
216213
} else {
@@ -265,7 +262,7 @@ private boolean isCacheReadyForInstantReconciliation(ExecutionScope<R> execution
265262
private void reScheduleExecutionIfInstructed(PostExecutionControl<R> postExecutionControl,
266263
R customResource) {
267264
postExecutionControl.getReScheduleDelay().ifPresent(delay -> eventSourceManager
268-
.getRetryTimerEventSource()
265+
.getRetryAndRescheduleTimerEventSource()
269266
.scheduleOnce(customResource, delay));
270267
}
271268

@@ -295,19 +292,21 @@ private void handleRetryOnException(ExecutionScope<R> executionScope) {
295292
delay,
296293
customResourceID);
297294
eventSourceManager
298-
.getRetryTimerEventSource()
295+
.getRetryAndRescheduleTimerEventSource()
299296
.scheduleOnce(executionScope.getCustomResource(), delay);
300297
},
301298
() -> log.error("Exhausted retries for {}", executionScope));
302299
}
303300

304-
private void handleSuccessfulExecutionRegardingRetry(ExecutionScope<R> executionScope) {
301+
private void cleanupOnSuccessfulExecution(ExecutionScope<R> executionScope) {
305302
log.debug(
306-
"Marking successful execution for resource: {}",
303+
"Cleanup for successful execution for resource: {}",
307304
getName(executionScope.getCustomResource()));
308-
retryState.remove(executionScope.getCustomResourceID());
305+
if (isRetryConfigured()) {
306+
retryState.remove(executionScope.getCustomResourceID());
307+
}
309308
eventSourceManager
310-
.getRetryTimerEventSource()
309+
.getRetryAndRescheduleTimerEventSource()
311310
.cancelOnceSchedule(executionScope.getCustomResourceID());
312311
}
313312

@@ -337,6 +336,10 @@ private void unsetUnderExecution(CustomResourceID customResourceUid) {
337336
underProcessing.remove(customResourceUid);
338337
}
339338

339+
private boolean isRetryConfigured() {
340+
return retry != null;
341+
}
342+
340343
@Override
341344
public void close() {
342345
lock.lock();

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class DefaultEventSourceManager<R extends CustomResource<?, ?>>
2222
private final ReentrantLock lock = new ReentrantLock();
2323
private final Set<EventSource> eventSources = Collections.synchronizedSet(new HashSet<>());
2424
private DefaultEventHandler<R> defaultEventHandler;
25-
private TimerEventSource<R> retryTimerEventSource;
25+
private TimerEventSource<R> retryAndRescheduleTimerEventSource;
2626
private CustomResourceEventSource customResourceEventSource;
2727

2828
DefaultEventSourceManager(DefaultEventHandler<R> defaultEventHandler) {
@@ -39,8 +39,8 @@ private void init(DefaultEventHandler<R> defaultEventHandler) {
3939
this.defaultEventHandler = defaultEventHandler;
4040
defaultEventHandler.setEventSourceManager(this);
4141

42-
this.retryTimerEventSource = new TimerEventSource<>();
43-
registerEventSource(retryTimerEventSource);
42+
this.retryAndRescheduleTimerEventSource = new TimerEventSource<>();
43+
registerEventSource(retryAndRescheduleTimerEventSource);
4444
}
4545

4646
@Override
@@ -98,8 +98,8 @@ public void cleanupForCustomResource(CustomResourceID customResourceUid) {
9898
}
9999
}
100100

101-
public TimerEventSource getRetryTimerEventSource() {
102-
return retryTimerEventSource;
101+
public TimerEventSource getRetryAndRescheduleTimerEventSource() {
102+
return retryAndRescheduleTimerEventSource;
103103
}
104104

105105
@Override

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ private T clone(T customResource) {
188188
/**
189189
* This will ensure that the next event received after this method is called will not be filtered
190190
* out.
191-
*
191+
*
192192
* @param customResourceID - to which the event is related
193193
*/
194194
public void whitelistNextEvent(CustomResourceID customResourceID) {

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class DefaultEventHandlerTest {
5353

5454
@BeforeEach
5555
public void setup() {
56-
when(defaultEventSourceManagerMock.getRetryTimerEventSource())
56+
when(defaultEventSourceManagerMock.getRetryAndRescheduleTimerEventSource())
5757
.thenReturn(retryTimerEventSourceMock);
5858
defaultEventHandler.setEventSourceManager(defaultEventSourceManagerMock);
5959
defaultEventHandlerWithRetry.setEventSourceManager(defaultEventSourceManagerMock);
@@ -280,6 +280,17 @@ public void dontWhitelistsEventIfUpdatedEventInCache() {
280280
verify(mockCREventSource, times(0)).whitelistNextEvent(eq(crID));
281281
}
282282

283+
@Test
284+
public void cancelScheduleOnceEventsOnSuccessfulExecution() {
285+
var crID = new CustomResourceID("test-cr", TEST_NAMESPACE);
286+
var cr = testCustomResource(crID);
287+
288+
defaultEventHandler.eventProcessingFinished(new ExecutionScope(cr, null),
289+
PostExecutionControl.defaultDispatch());
290+
291+
verify(retryTimerEventSourceMock, times(1)).cancelOnceSchedule(eq(crID));
292+
}
293+
283294
private CustomResourceID eventAlreadyUnderProcessing() {
284295
when(eventDispatcherMock.handleExecution(any()))
285296
.then(

0 commit comments

Comments
 (0)