<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 8, 2015 at 12:39 PM Dan Berindei &lt;<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I&#39;m not sure about including removals/invalidations/expiration,<br></blockquote><div><br></div><div>Invalidations to me don&#39;t quite fit, since it should be passivated in that case.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
because there would be no way to say &quot;I want to be notified on<br>
creation and modification, but no removals&quot;. On the other hand, adding<br></blockquote><div><br></div><div>We could always add a parameter to the new annotation to say if you don&#39;t care about removals maybe?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
3 more methods delegating to the same implementation, while not<br>
pretty, does allow you to listen to all changes.<br></blockquote><div><br></div><div>Do we need 3 methods?  Yes I think it would be nice for people.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Or are you thinking that the 3 additional listeners would add<br>
significant overhead when clustered?<br></blockquote><div><br></div><div>I was thinking it would be 1 listener.  CacheNotifierImpl could raise the new event in addition to the existing ones.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Cheers<br>
Dan<br>
<br>
<br>
On Thu, Oct 8, 2015 at 6:06 PM, William Burns &lt;<a href="mailto:mudokonman@gmail.com" target="_blank">mudokonman@gmail.com</a>&gt; wrote:<br>
&gt; I also like #3.  However I wonder if we want to expand it to include cache<br>
&gt; removal/expiration in this new event as well.  This would simplify a user<br>
&gt; listening for data being changed in only a given listener and it could be<br>
&gt; clustered.<br>
&gt;<br>
&gt;  - Will<br>
&gt;<br>
&gt; On Thu, Oct 8, 2015 at 6:38 AM Galder Zamarreño &lt;<a href="mailto:galder@redhat.com" target="_blank">galder@redhat.com</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; I think I&#39;m leaning towards 3.<br>
&gt;&gt;<br>
&gt;&gt; I would favour 2 if all listeners were already in place for Functional<br>
&gt;&gt; API, since I think down the line we&#39;d want to offer 3 map-like APIs:<br>
&gt;&gt; ConcurrentMap, JCache and Functional Map. Hence phasing out Cache as an<br>
&gt;&gt; end-user API... Since we&#39;re not there yet, I&#39;d vote for option 3 below.<br>
&gt;&gt;<br>
&gt;&gt; Cheers,<br>
&gt;&gt; --<br>
&gt;&gt; Galder Zamarreño<br>
&gt;&gt; Infinispan, Red Hat<br>
&gt;&gt;<br>
&gt;&gt; &gt; On 5 Oct 2015, at 07:34, Dan Berindei &lt;<a href="mailto:dan.berindei@gmail.com" target="_blank">dan.berindei@gmail.com</a>&gt; wrote:<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Hi guys<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; We have two different listener types for cache entry creation<br>
&gt;&gt; &gt; (@CacheEntryCreated) and modification (of an already-existing entry,<br>
&gt;&gt; &gt; @CacheEntryModified). However, PutMapCommand and<br>
&gt;&gt; &gt; PutKeyValueCommand/IGNORE_RETURN_VALUES do not read the previous value<br>
&gt;&gt; &gt; from loaders and/or remote nodes, so we sometimes notify the<br>
&gt;&gt; &gt; @CacheEntryCreated listeners instead of the @CacheEntryModified<br>
&gt;&gt; &gt; listeners.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; PutMapCommand has a &quot;workaround&quot; for this: it also notifies the<br>
&gt;&gt; &gt; @CacheEntryModified listener, regardless of whether it found the entry<br>
&gt;&gt; &gt; or not. I&#39;d like to change this [1] and make PutMapCommand and<br>
&gt;&gt; &gt; PutKeyValueCommand behave the same way.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; These are the options I&#39;m considering:<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; 1. Replicate the PutKeyValueCommand behaviour, and document that we<br>
&gt;&gt; &gt; may sometimes notify the @CacheEntryCreated listener even though the<br>
&gt;&gt; &gt; entry already exists.<br>
&gt;&gt; &gt; It would be the simplest to implement (in fact I already have a<br>
&gt;&gt; &gt; patch), but it doesn&#39;t feel right.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; 2. Always read the previous value from loaders and/or remote nodes<br>
&gt;&gt; &gt; when a @CacheEntryCreated/Modified event listener is registered.<br>
&gt;&gt; &gt; This would give us the correct behaviour, at the expense of performance.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; 3. Same as option 2, but also add a @CacheEntryWritten listener type,<br>
&gt;&gt; &gt; which only receives the new value and is notified regardless of<br>
&gt;&gt; &gt; whether the entry was created or modified.<br>
&gt;&gt; &gt; This would give users a choice: if they don&#39;t care about the previous<br>
&gt;&gt; &gt; value, the cache will be just as fast as usual, but if they need the<br>
&gt;&gt; &gt; previous value, they need to accept a slowdown.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; 4. Add the @CacheEntryWritten listener type, but only notify these<br>
&gt;&gt; &gt; listeners instead of the @CacheEntryCreated/Modified listeners for<br>
&gt;&gt; &gt; cache.putAll() and cache.withFlags(IGNORE_RETURN_VALUES).put().<br>
&gt;&gt; &gt; This is the option Galder chose for the functional API, but the<br>
&gt;&gt; &gt; difference between write-only and read-write operations is a lot<br>
&gt;&gt; &gt; clearer there, so I&#39;m not convinced it&#39;s ok for the ConcurrentMap API.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Personally I would choose option 3, because it would be mostly<br>
&gt;&gt; &gt; backwards-compatible: old code would only need to change if existing<br>
&gt;&gt; &gt; listeners are slowing down the cache.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Cheers<br>
&gt;&gt; &gt; Dan<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; [1] <a href="https://issues.jboss.org/browse/ISPN-5752" rel="noreferrer" target="_blank">https://issues.jboss.org/browse/ISPN-5752</a><br>
&gt;&gt; &gt; _______________________________________________<br>
&gt;&gt; &gt; infinispan-dev mailing list<br>
&gt;&gt; &gt; <a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
&gt;&gt; &gt; <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; _______________________________________________<br>
&gt;&gt; infinispan-dev mailing list<br>
&gt;&gt; <a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
&gt;&gt; <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
&gt;<br>
&gt;<br>
&gt; _______________________________________________<br>
&gt; infinispan-dev mailing list<br>
&gt; <a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
&gt; <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
<br>
_______________________________________________<br>
infinispan-dev mailing list<br>
<a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a></blockquote></div></div>