Skip to content

Commit 1ea9e68

Browse files
csviricsviri
authored
feat: reading cache just on reconciliation dispatching (#1640)
1 parent 6e9c531 commit 1ea9e68

File tree

4 files changed

+45
-30
lines changed

4 files changed

+45
-30
lines changed

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,11 @@ private void submitReconciliationExecution(ResourceState state) {
151151
}
152152
state.setUnderProcessing(true);
153153
final var latest = maybeLatest.get();
154-
ExecutionScope<P> executionScope = new ExecutionScope<>(latest, state.getRetry());
154+
ExecutionScope<P> executionScope = new ExecutionScope<>(state.getRetry());
155155
state.unMarkEventReceived();
156156
metrics.reconcileCustomResource(latest, state.getRetry(), metricsMetadata);
157157
log.debug("Executing events for custom resource. Scope: {}", executionScope);
158-
executor.execute(new ReconcilerExecutor(executionScope));
158+
executor.execute(new ReconcilerExecutor(resourceID, executionScope));
159159
} else {
160160
log.debug(
161161
"Skipping executing controller for resource id: {}. Controller in execution: {}. Latest Resource present: {}",
@@ -388,9 +388,11 @@ private void handleAlreadyMarkedEvents() {
388388

389389
private class ReconcilerExecutor implements Runnable {
390390
private final ExecutionScope<P> executionScope;
391+
private final ResourceID resourceID;
391392

392-
private ReconcilerExecutor(ExecutionScope<P> executionScope) {
393+
private ReconcilerExecutor(ResourceID resourceID, ExecutionScope<P> executionScope) {
393394
this.executionScope = executionScope;
395+
this.resourceID = resourceID;
394396
}
395397

396398
@Override
@@ -399,6 +401,13 @@ public void run() {
399401
final var thread = Thread.currentThread();
400402
final var name = thread.getName();
401403
try {
404+
var actualResource = cache.get(resourceID);
405+
if (actualResource.isEmpty()) {
406+
log.debug("Skipping execution; primary resource missing from cache: {}",
407+
resourceID);
408+
return;
409+
}
410+
actualResource.ifPresent(executionScope::setResource);
402411
MDCUtils.addResourceInfo(executionScope.getResource());
403412
thread.setName("ReconcilerExecutor-" + controllerName() + "-" + thread.getId());
404413
PostExecutionControl<P> postExecutionControl =

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,18 @@
66
class ExecutionScope<R extends HasMetadata> {
77

88
// the latest custom resource from cache
9-
private final R resource;
9+
private R resource;
1010
private final RetryInfo retryInfo;
1111

12-
ExecutionScope(R resource, RetryInfo retryInfo) {
13-
this.resource = resource;
12+
ExecutionScope(RetryInfo retryInfo) {
1413
this.retryInfo = retryInfo;
1514
}
1615

16+
public ExecutionScope<R> setResource(R resource) {
17+
this.resource = resource;
18+
return this;
19+
}
20+
1721
public R getResource() {
1822
return resource;
1923
}

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ void ifExecutionInProgressWaitsUntilItsFinished() {
120120
void schedulesAnEventRetryOnException() {
121121
TestCustomResource customResource = testCustomResource();
122122

123-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
123+
ExecutionScope executionScope =
124+
new ExecutionScope(null);
125+
executionScope.setResource(customResource);
124126
PostExecutionControl postExecutionControl =
125127
PostExecutionControl.exceptionDuringExecution(new RuntimeException("test"));
126128

@@ -254,7 +256,7 @@ void cancelScheduleOnceEventsOnSuccessfulExecution() {
254256
var crID = new ResourceID("test-cr", TEST_NAMESPACE);
255257
var cr = testCustomResource(crID);
256258

257-
eventProcessor.eventProcessingFinished(new ExecutionScope(cr, null),
259+
eventProcessor.eventProcessingFinished(new ExecutionScope(null).setResource(cr),
258260
PostExecutionControl.defaultDispatch());
259261

260262
verify(retryTimerEventSourceMock, times(1)).cancelOnceSchedule(eq(crID));
@@ -283,7 +285,8 @@ void startProcessedMarkedEventReceivedBefore() {
283285
@Test
284286
void updatesEventSourceHandlerIfResourceUpdated() {
285287
TestCustomResource customResource = testCustomResource();
286-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
288+
ExecutionScope executionScope =
289+
new ExecutionScope(null).setResource(customResource);
287290
PostExecutionControl postExecutionControl =
288291
PostExecutionControl.customResourceUpdated(customResource);
289292

@@ -297,7 +300,8 @@ void updatesEventSourceHandlerIfResourceUpdated() {
297300
@Test
298301
void notUpdatesEventSourceHandlerIfResourceUpdated() {
299302
TestCustomResource customResource = testCustomResource();
300-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
303+
ExecutionScope executionScope =
304+
new ExecutionScope(null).setResource(customResource);
301305
PostExecutionControl postExecutionControl =
302306
PostExecutionControl.customResourceStatusPatched(customResource);
303307

@@ -311,7 +315,8 @@ void notUpdatesEventSourceHandlerIfResourceUpdated() {
311315
void notReschedulesAfterTheFinalizerRemoveProcessed() {
312316
TestCustomResource customResource = testCustomResource();
313317
markForDeletion(customResource);
314-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
318+
ExecutionScope executionScope =
319+
new ExecutionScope(null).setResource(customResource);
315320
PostExecutionControl postExecutionControl =
316321
PostExecutionControl.customResourceFinalizerRemoved(customResource);
317322

@@ -324,7 +329,8 @@ void notReschedulesAfterTheFinalizerRemoveProcessed() {
324329
void skipEventProcessingIfFinalizerRemoveProcessed() {
325330
TestCustomResource customResource = testCustomResource();
326331
markForDeletion(customResource);
327-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
332+
ExecutionScope executionScope =
333+
new ExecutionScope(null).setResource(customResource);
328334
PostExecutionControl postExecutionControl =
329335
PostExecutionControl.customResourceFinalizerRemoved(customResource);
330336

@@ -341,7 +347,8 @@ void skipEventProcessingIfFinalizerRemoveProcessed() {
341347
void newResourceAfterMissedDeleteEvent() {
342348
TestCustomResource customResource = testCustomResource();
343349
markForDeletion(customResource);
344-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
350+
ExecutionScope executionScope =
351+
new ExecutionScope(null).setResource(customResource);
345352
PostExecutionControl postExecutionControl =
346353
PostExecutionControl.customResourceFinalizerRemoved(customResource);
347354
var newResource = testCustomResource();
@@ -377,7 +384,8 @@ void rateLimitsReconciliationSubmission() {
377384
@Test
378385
void schedulesRetryForMarReconciliationInterval() {
379386
TestCustomResource customResource = testCustomResource();
380-
ExecutionScope executionScope = new ExecutionScope(customResource, null);
387+
ExecutionScope executionScope =
388+
new ExecutionScope(null).setResource(customResource);
381389
PostExecutionControl postExecutionControl =
382390
PostExecutionControl.defaultDispatch();
383391

@@ -398,7 +406,8 @@ void schedulesRetryForMarReconciliationIntervalIfRetryExhausted() {
398406
eventSourceManagerMock,
399407
metricsMock));
400408
eventProcessorWithRetry.start();
401-
ExecutionScope executionScope = new ExecutionScope(testCustomResource(), null);
409+
ExecutionScope executionScope =
410+
new ExecutionScope(null).setResource(testCustomResource());
402411
PostExecutionControl postExecutionControl =
403412
PostExecutionControl.exceptionDuringExecution(new RuntimeException());
404413
when(eventProcessorWithRetry.retryEventSource()).thenReturn(retryTimerEventSourceMock);

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,6 @@ void propagatesRetryInfoToContextIfFinalizerSet() {
374374

375375
reconciliationDispatcher.handleExecution(
376376
new ExecutionScope(
377-
testCustomResource,
378377
new RetryInfo() {
379378
@Override
380379
public int getAttemptCount() {
@@ -385,7 +384,7 @@ public int getAttemptCount() {
385384
public boolean isLastAttempt() {
386385
return true;
387386
}
388-
}));
387+
}).setResource(testCustomResource));
389388

390389
ArgumentCaptor<Context> contextArgumentCaptor =
391390
ArgumentCaptor.forClass(Context.class);
@@ -504,7 +503,6 @@ void callErrorStatusHandlerIfImplemented() {
504503

505504
reconciliationDispatcher.handleExecution(
506505
new ExecutionScope(
507-
testCustomResource,
508506
new RetryInfo() {
509507
@Override
510508
public int getAttemptCount() {
@@ -515,7 +513,7 @@ public int getAttemptCount() {
515513
public boolean isLastAttempt() {
516514
return true;
517515
}
518-
}));
516+
}).setResource(testCustomResource));
519517

520518
verify(customResourceFacade, times(1)).updateStatus(testCustomResource);
521519
verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource),
@@ -535,8 +533,7 @@ void callErrorStatusHandlerEvenOnFirstError() {
535533
};
536534

537535
var postExecControl = reconciliationDispatcher.handleExecution(
538-
new ExecutionScope(
539-
testCustomResource, null));
536+
new ExecutionScope(null).setResource(testCustomResource));
540537
verify(customResourceFacade, times(1)).updateStatus(testCustomResource);
541538
verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource),
542539
any(), any());
@@ -555,8 +552,7 @@ void errorHandlerCanInstructNoRetryWithUpdate() {
555552
};
556553

557554
var postExecControl = reconciliationDispatcher.handleExecution(
558-
new ExecutionScope(
559-
testCustomResource, null));
555+
new ExecutionScope(null).setResource(testCustomResource));
560556

561557
verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource),
562558
any(), any());
@@ -576,8 +572,7 @@ void errorHandlerCanInstructNoRetryNoUpdate() {
576572
};
577573

578574
var postExecControl = reconciliationDispatcher.handleExecution(
579-
new ExecutionScope(
580-
testCustomResource, null));
575+
new ExecutionScope(null).setResource(testCustomResource));
581576

582577
verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource),
583578
any(), any());
@@ -596,8 +591,7 @@ void errorStatusHandlerCanPatchResource() {
596591

597592

598593
reconciliationDispatcher.handleExecution(
599-
new ExecutionScope(
600-
testCustomResource, null));
594+
new ExecutionScope(null).setResource(testCustomResource));
601595

602596
verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any());
603597
verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource),
@@ -622,8 +616,7 @@ void ifRetryLimitedToZeroMaxAttemptsErrorHandlerGetsCorrectLastAttempt() {
622616
reconciler.errorHandler = mockErrorHandler;
623617

624618
reconciliationDispatcher.handleExecution(
625-
new ExecutionScope(
626-
testCustomResource, null));
619+
new ExecutionScope(null).setResource(testCustomResource));
627620

628621
verify(mockErrorHandler, times(1)).updateErrorStatus(any(),
629622
ArgumentMatchers.argThat((ArgumentMatcher<Context<TestCustomResource>>) context -> {
@@ -667,7 +660,7 @@ private void removeFinalizers(CustomResource customResource) {
667660
}
668661

669662
public <T extends HasMetadata> ExecutionScope<T> executionScopeWithCREvent(T resource) {
670-
return new ExecutionScope<>(resource, null);
663+
return (ExecutionScope<T>) new ExecutionScope<>(null).setResource(resource);
671664
}
672665

673666
private class TestReconciler

0 commit comments

Comments
 (0)