34
34
* </li>
35
35
* <li>Threading approach thus thread pool size and/or implementation should be configurable</li>
36
36
* </ul>
37
+ * <p>
38
+ * Notes:
39
+ * <ul>
40
+ * <li> In implementation we have to lock since the fabric8 client event handling is multi-threaded, we can receive multiple events
41
+ * for same resource. Also we do callback from other threads.
42
+ * </li>
43
+ * <li>We don't react for delete event, since we always use finalizers and do delete on marked for deletion.</li>
44
+ * </ul>
37
45
*
38
46
* @param <R>
39
47
*/
40
48
41
- /**
42
- *
43
- **/
44
49
public class EventScheduler <R extends CustomResource > implements Watcher <R > {
45
50
46
51
private final static Logger log = LoggerFactory .getLogger (EventScheduler .class );
@@ -73,8 +78,6 @@ void scheduleEvent(CustomResourceEvent newEvent) {
73
78
log .debug ("Current queue size {}" , executor .getQueue ().size ());
74
79
log .info ("Scheduling event: {}" , newEvent .getEventInfo ());
75
80
try {
76
- // we have to lock since the fabric8 client event handling is multi-threaded,
77
- // so in the following part could be a race condition when multiple events are received for same resource.
78
81
lock .lock ();
79
82
if (eventStore .processedNewerVersionBefore (newEvent )) {
80
83
log .debug ("Skipping event processing since was processed event with newer version before. {}" , newEvent );
@@ -83,7 +86,6 @@ void scheduleEvent(CustomResourceEvent newEvent) {
83
86
if (newEvent .getAction () == Action .DELETED ) {
84
87
return ;
85
88
}
86
- // if there is an event waiting for to be scheduled we just replace that.
87
89
if (eventStore .containsOlderVersionOfNotScheduledEvent (newEvent )) {
88
90
log .debug ("Replacing event which is not scheduled yet, since incoming event is more recent. new Event:{}" , newEvent );
89
91
eventStore .addOrReplaceEventAsNotScheduledYet (newEvent );
@@ -130,8 +132,8 @@ boolean eventProcessingStarted(CustomResourceEvent event) {
130
132
lock .lock ();
131
133
EventStore .ResourceScheduleHolder res = eventStore .removeEventScheduledForProcessing (event .resourceUid ());
132
134
if (res == null ) {
133
- // if its still scheduled for processing .
134
- // note that it can happen that we scheduled an event for processing, it took some time that is was picked
135
+ // Double checking if the event is still scheduled. This is a corner case, but can actually happen .
136
+ // In detail: it can happen that we scheduled an event for processing, it took some time that is was picked
135
137
// by executor, and it was removed during that time from the schedule but not cancelled yet. So to be correct
136
138
// this should be checked also here. In other word scheduleEvent function can run in parallel with eventDispatcher.
137
139
return false ;
0 commit comments