[infinispan-dev] Feedback and requests on clustered and remote listeners

[Emmanuel Bernard]

On 19 Sep 2014, at 17:09, William Burns <mudokonman at gmail.com> wrote:

> Comments regarding embedded usage are inline.  I am not quite sure on
> the hot rod client ones.
> 
> On Thu, Sep 18, 2014 at 12:24 PM, Emmanuel Bernard
> <emmanuel at hibernate.org> wrote:
>> 
>> 
>> That requires us to be able to provide the old and new value to the KeyValueFilter and the Converter interface as well as the type of event (creation, update, deletion).
> 
> I agree the oldValue is required for most efficient usage.  From the
> oldValue though it seems you can infer what operation it is.  Create
> has null oldValue and delete has null newValue I would think.

well except when I do cache.put(key, null) but that might not matter.
The other use case is the includeInitialState where the old value would be either null or the same as the new one? Could a user detect that state based on old == new? 
At any rate the programming model becomes quite awkward and rely on strong understanding, I’d prefer to stick an enum showing the transition explicitly to make things easier.

> 
> This also came up here
> http://markmail.org/search/?q=infinispan#query:infinispan%20list%3Aorg.jboss.lists.infinispan-dev%20order%3Adate-backward+page:1+mid:nn6r3uuabq3hyzmd+state:results
> and I am debating if this interface should be separate or just an
> extension from KeyValueFilter etc.
> 
> The thing is the new interface is mostly beneficial only to clustered
> listeners since non cluster listeners get both the pre and post event
> which makes the old value accessible.  I may have to just try to write
> it up and see how it goes unless anyone has any suggestions.

+1, feel free to send even gists of your progresses, I’m happy to provide feedback.

> 
>> 
>> With the use case I describe, I think the best approach is to merge the KVF and Converter into a single Listener like interface that is able to send or silence an event payload. But that’s guestimate.
>> Because oldValue / newValue implies an unmarshalling overhead we might want to make it an annotation based flag on the class that is executed on each node (somewhat similar to the settings hosted on @Listener).
> 
> We actually have an interface that combines the 2 interfaces, it is
> called KeyValueFilterConverter.  It was added to more efficiently
> perform indexless querying using entry retriever.  This interface is
> not supported for cluster listeners at this time though.

That interface would do - assuming we get the old / new values and the transition.
But then it begs the question, do we really want to keep the KeyFilter, KeyValueFilter and Converter interfaces around. That’s a lot of interface for features quite interrelated.
I can see why they can speed things up (esp KeyFilter that does not require to unmarshal the value).
> 
>> 
>> ## includeCurrentState and very narrow filtering
>> 
>> The existing approach is fine (send a create event notif for all existing keys and queue changes in the mean time) as long as the listener plans to consume most of these events.
>> But in case of a big data grid, with a lot of passivated entries, the cost would become non negligible.
> 
> The filter and converter are applied while doing the current state so
> it should be performant in that case.  

I don’t understand, the code still has to look all key/value pairs of a given node (at least the primary ones) and send them through the KVF / Converter logic. So you need to unmarshal all of them as well as load from cachestore the passivated ones. Correct? That’s the cost I am describing here.

> Also to note while the current
> state operation is ongoing any new notifications are enqueued until
> the current state is applied.  These new events will not cause
> blocking as you mentioned earlier with sync since they are immediately
> enqueued.  The queueing may be something we have to add blocking
> though possibly to prevent memory exhaustion in the case when the
> initial iteration is extremely slow and there are a lot of updates
> during that period.  The code currently has code to release queued
> events by segment as the segments are completed, I have thought about
> also releasing events by key instead which should relieve a lot of
> possible memory usage.
> 
>> 
>> An alternative approach is to first do a query matching the elements the listener is interested in and queue up the events until the query is fully processed. Can a listener access a cache and do a query? Should we offer such option in a more packaged way?
> 
> The provided filter be doing this already.
> 
> But maybe more info on what you are proposing.  Either way it seems we
> have to have the listener installed before we can run the query so we
> can properly tell what events should be raised in the event of
> concurrent events while the query is running.

You lost me here :)