Is ResourceEventHandler thread-safe?

[ivanchuchulski]

Hello,

We're using ResourceEventHandler to get notified about changes for K8s resources, which can get changed frequently (EndpointSlices) and we're concerned if the underlying implementation is thread-safe. We're concerned what happens in the following cases:

• can multiple executions of onAdd method for example can happen simultaneously by different threads
• execution of onAdd, onUpdate or onDelete can happen simultaneously by different threads (i.e. "add" and "update" events are to be processed and methods get do the methods the triggered one after another or in parallel)

We've consulted the docs, but there concerns about blocking are listed from which we can't answer our questions.

Best Regards,
Ivan Chuchuslki

[shawkins]

can multiple executions of onAdd method for example can happen simultaneously by different threads
execution of onAdd, onUpdate or onDelete can happen simultaneously by different threads (i.e. "add" and "update" events are to be processed and methods get do the methods the triggered one after another or in parallel)

No, all event processing is serialized, but it's not guarenteed to be the same thread each time.

[ivanchuchulski]

Hello
Thank you for the fast reply!
I forgot to specify in my initial message, I didn't specify, but we're using 6.14.0.

I'm wondering if this lines from the docs is relevant to the concurrency or it's regarding something else:

To ease developer burden the callbacks on Watchers and ResourceEventHandlers will not be done directly by an HTTP client thread, but the order of calls will be guaranteed. This will make the logic you include in those callbacks tolerant to some blocking without compromising the on-going work at the HTTP client level. However, you should avoid truly long running operations as this will cause further events to queue and may eventually case memory issues.

It is recommended with any HTTP client implementation that logic you supply via Watchers, ExecListeners, ResourceEventHandlers, Predicates, Interceptors, LeaderCallbacks, etc. does not execute long running tasks.

If the methods are indeed thread-safe, can this be stated in the docs and/or in the Javadoc so we can know for sure?

Best Regards,
Ivan Chuchuslki

[shawkins]

I'm wondering if this lines from the docs is relevant to the concurrency or it's regarding something else:

http worker threads are a separately managed pool of threads, which you don't want to slow down with long running work.

This is saying that all callbacks will happen in some other thread, not an http worker thread.

If the methods are indeed thread-safe, can this be stated in the docs and/or in the Javadoc so we can know for sure?

Sure, feel free to open a pr for any change that might help. Saying thread-safe though is may not be appropriately descriptive. Just because the callbacks are serialized does not mean that write visibility across threads is guarenteed - if you are doing something stateful and you need that guarentee, then you should still use volatile or locking.

[ivanchuchulski]

Hello,
Thanks for the information, we're considering a thread-safe collection since we have to use a keep a collection of ready target pod's hostnames (we're watching an EnpointSlice targeting a StatefulSet's pods).

With relation to that scenario I have another question - as suggested by the K8s docs, we should always consider the full list of EndpointSlice objects from the cluster. We initially implemented that with a call to the API server, but instead there should be a cache in the shared index informer which we when creating the class that implements ResourceEventHandler. Can we utilize that, should we use another informer or can we reuse the original one? Another thing I see in some articles about Go informers that there should be some waiting for the cache to be synched, how should that be done?

[csviri]

Another thing I see in some articles about Go informers that there should be some waiting for the cache to be synched, how should that be done?

See it done in JOSDK:

https://github.com/operator-framework/java-operator-sdk/blob/1c881f55757fa768b085bbff234ecf0dc80e9a87/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java#L90-L100

[ivanchuchulski]

Hello @csviri, @shawkins ,

Thanks for pointing out a particular example. We're not concerned with using the non-blocking start method, as the logic for starting the Informer is going to be in the starting process in our controller. We're opting to use the run blocking method instead.

One thing that I'm concerned is that should we wait for the informer cache to be synched (i.e. the initial list to be performed)? I'm seeing that there is a hasSynced method in SharedIndexInformer interface(I'm not sure if that's the same as WaitForCacheSync function in the client-go impl), which maybe should be used to check that? I suppose for it we'd have to wait to return true maybe?

Also are you aware if there is a situation where the cache could be become empty and we'd have to fallback to fetching data from the API server with a client instance or we'd not worry about that?

Best Regards,
Ivan Chuchulski

[shawkins]

There if you use one of blocking methods, then the informer won't be returned until after it has synced.

Also are you aware if there is a situation where the cache could be become empty and we'd have to fallback to fetching data from the API server with a client instance or we'd not worry about that?

The cache can lag the state of the api server, but it won't simply become empty - there is no client side expiration of entries. Items are only removed via a delete event or they are absent from a relist.