On 08/24/2016 04:32 PM, Dan Berindei wrote: > 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.
So prev values are not working as doc states (silently returning wrong values) in case that: - topology changes (command retry) - persistence is used These are quite non-obvious "if"s for users :-/ I'd call listeners a non-reliable feature. > >> 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. But pre events are also unreliable as they just notify that something may or may not happen in the future. > >>> 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. What value has a "previous value" when you can't know it's correct? I see that as the main problem. I wouldn't introduce any complexities (listener flags telling if it needs or does not need previous value) at this point - the less code the better, until we fix what we already have. > > >>>> 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 -- Radim Vansa <rva...@redhat.com> JBoss Performance Team _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
