--
Galder Zamarreño
Infinispan, Red Hat
On 20 Oct 2015, at 15:05, Dan Berindei <dan.berindei(a)gmail.com>
wrote:
On Mon, Oct 19, 2015 at 5:34 PM, Galder Zamarreño <galder(a)redhat.com> wrote:
>
>
> --
> Galder Zamarreño
> Infinispan, Red Hat
>
>> On 14 Oct 2015, at 16:43, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>>
>> There is one more thing to consider: the interface for acting on the
>> previous value of an event is very clunky even today. It basically
>> requires a thread-local to keep track of the previous value from the
>> pre=true invocation to the pre=false invocation, and even that won't
>> work once the reactive interceptor stack lands (so that pre and post
>> events are triggered on different threads).
>>
>> So I'm starting to think we should go with option 1 for now, and start
>> a bigger redesign of the cache notifications for 9.0:
>> * Remove the pre=true events
>> * Add explicit event properties for the previous value/metadata
>
> Why redesign cache notifications? As I mentioned in a previous reply, I see Cache and
cache notifications being phased out in favour of: JCache (and its events), ConcurrentMap
and Functional Map (and its events) :|.
How is the ConcurrentMap API different from the Cache API?
Cache is ConcurrentMap + a mixed bag of other things which includes all sorts of things,
from put operations that take extra parameters, to ways to retrieve the Batch container
:|
ConcurrentMap is what is given to use by the JDK whereas the rest of Cache API is what we
want to have, or legacy stuff we've had since day 1.
Where possible, we should promote users to use standard APIs, so that's ConcurrentMap
(as-is, no Cache) and JCache. If the users find limitations on those, we should try to
fill those gaps with Functional Map.
It's true that today, using ConcurrentMap is done by getting a Cache, which IMO is
really confusing in how we expose our APIs. So, yeah, if people want to use ConcurrentMap,
they need Cache, but that should not be the case.
>
> So, no need to redesign Cache notifications IMO :|
I was assuming JCache listeners are implemented on top of the Cache
listeners, so they would need fixes in the Cache listeners to work
properly.
^ Yeah, currently that is the case.
We could implement the JCache listeners directly in 9.0 and remove
the
Cache listeners, I guess. But looking at the JCache API it looks like
their listeners need the previous value for all writes, unlike the
FunctionalMap listeners, so I think there will still be a need for a
lower-level implementation.
Depends. JCache API impl could decide, based on whether any listeners are attached,
whether those write-only operations should become read-write operations because listeners
attach demand it, as opposed for the actual function requiring to read previous value.
Cheers,
>
>>
>> Without backwards compatibility requirements, we could even add a
>> skipPreviousValue parameter to all listener annotations -- except for
>> @CacheEntryCreated, I guess -- making the new listener type
>> superfluous.
>>
>> Cheers
>> Dan
>>
>>
>> On Mon, Oct 12, 2015 at 11:02 AM, Dan Berindei <dan.berindei(a)gmail.com>
wrote:
>>> On Fri, Oct 9, 2015 at 4:39 PM, William Burns <mudokonman(a)gmail.com>
wrote:
>>>>
>>>>
>>>> On Thu, Oct 8, 2015 at 12:39 PM Dan Berindei
<dan.berindei(a)gmail.com> wrote:
>>>>>
>>>>> I'm not sure about including removals/invalidations/expiration,
>>>>
>>>>
>>>> Invalidations to me don't quite fit, since it should be passivated in
that
>>>> case.
>>>
>>> Passivations have a different listener, I didn't include
>>> @CacheEntryPassivated here :)
>>>
>>> Perhaps invalidation doesn't fit, because it's used to remove stale
>>> entries either after a rebalance, or after a write (for L1 entries, or
>>> in invalidation mode).
>>>
>>> But then I would say expiration also doesn't fit, because the all the
>>> others are direct actions by the user, and expiration happens in the
>>> background.
>>>
>>>>
>>>>>
>>>>> because there would be no way to say "I want to be notified on
>>>>> creation and modification, but no removals". On the other hand,
adding
>>>>
>>>>
>>>> We could always add a parameter to the new annotation to say if you
don't
>>>> care about removals maybe?
>>>
>>> Yes, that should work.
>>>
>>>>
>>>>>
>>>>> 3 more methods delegating to the same implementation, while not
>>>>> pretty, does allow you to listen to all changes.
>>>>
>>>>
>>>> Do we need 3 methods? Yes I think it would be nice for people.
>>>
>>> I'm assuming the listener methods are checked to receive the right
>>> event types. If we allow supertypes (and make CacheEntryWrittenEvent a
>>> supertype of the others) it could be just 1 method with 4 annotations.
>>>
>>>>
>>>>>
>>>>>
>>>>> Or are you thinking that the 3 additional listeners would add
>>>>> significant overhead when clustered?
>>>>
>>>>
>>>> I was thinking it would be 1 listener. CacheNotifierImpl could raise
the
>>>> new event in addition to the existing ones.
>>>
>>> Right, if we include the removals in this annotation there would be
>>> just one listener. But would it be faster than having 4 listeners, one
>>> for create/update, and one each for remove/invalidation/expiration?
>>>
>>> Cheers
>>> Dan
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev