On Wed, Aug 24, 2016 at 4:14 PM, Adrian Nistor <anis...@redhat.com> wrote: > I remember there was some discussion to refactor listeners to drop the > concept of pre-events and have the previous value available in the > post-event (where applicable). I do not remember what decision was made > about this. But if we do it, that would be already a big backwards > incompatible change, so modifying some defaults regarding the behavior of > the Listener annotation is just mildly disturbing. We'll have to > apologize/document this anyway and also provide migration advice. >
That discussion died out because we couldn't decide whether we really need to maintain the current listeners or we could switch to supporting only JCache and FunctionalMap listener APIs. FWIW I'm all for modernizing the core listeners, removing the pre events, and allowing listeners to receive the previous value in the post event. I'd also change the API to be interface-based instead of annotation-based. > > On 08/22/2016 09:15 PM, William Burns wrote: > > I like the idea of having a variable to set on the listener annotation. > This way we can know for sure if we need to force previous values for some > listeners and not for others. > > It seems the default should be to force the previous value to be more inline > with the current behavior, but I fear no one will use the opposite in this > case though. What do you guys think? Actually, the current behaviour is *not* to force the previous value. If you have the entry in the data container, yes, you'll see it in the listener, but if the entry is in a store, you won't. Clustered listeners do get the previous value even if it's remote, but not if the entry is passivated. > > On Mon, Aug 22, 2016 at 4:31 AM Adrian Nistor <anis...@redhat.com> wrote: >> >> Hi Radim, >> >> Continuous query is built on top of these listeners. CQ _always_ needs >> the previous value and it is very convenient in this case that the >> command is forced to load the previous value. I imagine there may be >> other use cases where we cannot live without the prev value. Unfortunately, if a command is retried in a non-tx cache (event.isCommandRetried() == true), the listener may receive the new value as the previous value. So CQ needs to support this case, or we'll have to finally fix it in core. I'd mention versioning/tombstones, but I fear Sanne is going to read this and derail the thread ;) >> >> I think the listener should be able to state if it needs the prev value >> at registration time. Maybe add a new attribute in the Listener >> annotation? Similar to how we handled Observation. >> Actually, with the current API, the only way to get the previous value is with the pre event, so we could interpret @Listener(observation = POST) as a sign that the listener doesn't need the previous value. >> Adrian >> >> On 08/19/2016 11:34 PM, Radim Vansa wrote: >> > Hi, >> > >> > as I am trying to simplify current entry wrapping and distribution code, >> > I often find that listeners can get wrong previous value in the event, >> > and it sometimes forces the command to load the value even if it is not >> > needed for the command. >> > >> > I am wondering if we should change the previous value in events to >> > Optional - we can usually at least detect that we cannot provide a >> > reliable value (e.g. after retry due to topology change, or because the >> > command did not bothered to load the previous value from cache loader) >> > and return empty Optional. >> > It would be a breaking change without a lot of benefit, so I would not make this change. If we added a getPreviousValue() method for post events, then yes, it could return an Optional. I'm not yet sure it's a good fit for isCommandRetried() == true, since in that case we usually have a previous value, we're just not sure it's the correct one. >> > WDYT? >> > >> > Radim >> > >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev