<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 8, 2014 at 6:14 PM, William Burns <span dir="ltr"><<a href="mailto:mudokonman@gmail.com" target="_blank">mudokonman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Wed, Oct 8, 2014 at 10:57 AM, Dan Berindei <<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>> wrote:<br>
><br>
><br>
> On Wed, Oct 8, 2014 at 5:42 PM, William Burns <<a href="mailto:mudokonman@gmail.com">mudokonman@gmail.com</a>> wrote:<br>
>><br>
>> So it seems we would want to change this for 7.0 if possible since it<br>
>> would be a bigger change for something like 7.1 and 8.0 would be even<br>
>> further out. I should be able to put this together for CR2.<br>
><br>
><br>
> I'm not 100% convinced that we need it for 7.x. For 8.0 I would recommend<br>
> removing the size() method altogether, and providing some looser<br>
> "statistics" instead.<br>
<br>
</span>Yeah I guess I don't know enough about the demand for these methods or<br>
what people wanted to use them for to know what kind of priority they<br>
should be given.<br>
<br>
It sounds like you are talking about decoupling from the<br>
Map/ConcurrentMap interface completely then, right? So we would also<br>
eliminate the other bulk methods (keySet, values, entrySet)?<br></blockquote><div><br></div><div>Yes, I would base the Cache interface on JSR-107's Cache, which doesn't have size() or the other methods.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
><br>
>><br>
>><br>
>> It seems that we want to implement keySet, values and entrySet methods<br>
>> using the entry iterator approach.<br>
>><br>
>> It is however unclear for the size method if we want to use MR entry<br>
>> counting and not worry about the rehash and passivation issues since<br>
>> it is just an estimation anyways. Or if we want to also use the entry<br>
>> iterator which should be closer approximation but will require more<br>
>> network overhead and memory usage.<br>
><br>
><br>
> +1 to use the entry iterator from me, ignoring state transfer we can get<br>
> some pretty wild fluctuations in the size of the cache.<br>
<br>
</span>That is personally my feeling as well, but I tend to err more on the<br>
side of correctness to begin with.<br>
<span class=""><br>
> We could use a distributed task for Cache.isEmpty() instead of size() == 0,<br>
> though.<br>
<br>
</span>Yes that should be a good optimization either way.<br>
<span class=""><br>
><br>
>><br>
>><br>
>> Also we didn't really talk about the fact that these methods would<br>
>> ignore ongoing transactions and if that is a concern or not.<br>
>><br>
><br>
> It might be a concern for the Hibernate 2LC impl, it was their TCK that<br>
> prompted the last round of discussions about clear().<br>
<br>
</span>Although I wonder how much these methods are even used since they only<br>
work for Local, Replication or Invalidation caches in their current<br>
state (and didn't even use loaders until 6.0).<br></blockquote><div><br></div><div>There is some more information about the test in the mailing list discussion [1]</div><div>There's also a JIRA for clear() [2]</div><div><br></div><div>I think 2LC almost never uses distribution, so size() being local-only didn't matter, but making it non-tx could cause problems - at least for that particular test.</div><div><br></div><div>[1] <a href="http://lists.jboss.org/pipermail/infinispan-dev/2013-October/013914.html">http://lists.jboss.org/pipermail/infinispan-dev/2013-October/013914.html</a></div><div>[2] <a href="https://issues.jboss.org/browse/ISPN-3656">https://issues.jboss.org/browse/ISPN-3656</a></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
><br>
> We haven't talked about what size(), keySet() and values() should return for<br>
> an invalidation cache either... I forget, does the distributed entry<br>
> iterator work with invalidation caches?<br>
<br>
</span>It works the same as a local cache so only the local node contents are<br>
returned. Replicated does the same thing, distributed is the only<br>
special case. This was the only thing that made sense to me, but if<br>
you have any ideas that would be great to hear for possibly enhancing<br>
Invalidation iteration.<br></blockquote><div><br></div><div>Sounds good to me. </div><div><br></div><div>cache.get(k) will search on all the nodes via ClusterLoader, so there is a certain appeal in making the entry iterator do the same. But invalidation caches are used with an external (non-CacheLoader) source of data anyway, so we can never return "all the entries".</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
><br>
><br>
>><br>
>> - Will<br>
>><br>
>> On Wed, Oct 8, 2014 at 10:13 AM, Mircea Markus <<a href="mailto:mmarkus@redhat.com">mmarkus@redhat.com</a>> wrote:<br>
>> ><br>
>> > On Oct 8, 2014, at 15:11, Dan Berindei <<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>> wrote:<br>
>> ><br>
>> >><br>
>> >> On Wed, Oct 8, 2014 at 5:03 PM, Mircea Markus <<a href="mailto:mmarkus@redhat.com">mmarkus@redhat.com</a>><br>
>> >> wrote:<br>
>> >> On Oct 3, 2014, at 9:30, Radim Vansa <<a href="mailto:rvansa@redhat.com">rvansa@redhat.com</a>> wrote:<br>
>> >><br>
>> >> > Hi,<br>
>> >> ><br>
>> >> > recently we had a discussion about what size() returns, but I've<br>
>> >> > realized there are more things that users would like to know. My<br>
>> >> > question is whether you think that they would really appreciate it,<br>
>> >> > or<br>
>> >> > whether it's just my QA point of view where I sometimes compute the<br>
>> >> > 'checksums' of cache to see if I didn't lost anything.<br>
>> >> ><br>
>> >> > There are those sizes:<br>
>> >> > A) number of owned entries<br>
>> >> > B) number of entries stored locally in memory<br>
>> >> > C) number of entries stored in each local cache store<br>
>> >> > D) number of entries stored in each shared cache store<br>
>> >> > E) total number of entries in cache<br>
>> >> ><br>
>> >> > So far, we can get<br>
>> >> > B via withFlags(SKIP_CACHE_LOAD).size()<br>
>> >> > (passivation ? B : 0) + firstNonZero(C, D) via size()<br>
>> >> > E via distributed iterators / MR<br>
>> >> > A via data container iteration + distribution manager query, but only<br>
>> >> > without cache store<br>
>> >> > C or D through<br>
>> >> ><br>
>> >> > getComponentRegistry().getLocalComponent(PersistenceManager.class).getStores()<br>
>> >> ><br>
>> >> > I think that it would go along with users' expectations if size()<br>
>> >> > returned E and for the rest we should have special methods on<br>
>> >> > AdvancedCache. That would of course change the meaning of size(), but<br>
>> >> > I'd say that finally to something that has firm meaning.<br>
>> >> ><br>
>> >> > WDYT?<br>
>> >><br>
>> >> There was a lot of arguments in past whether size() and other methods<br>
>> >> that operate over all the elements (keySet, values) are useful because:<br>
>> >> - they are approximate (data changes during iteration)<br>
>> >> - they are very resource consuming and might be miss-used (this is the<br>
>> >> reason we chosen to use size() with its current local semantic)<br>
>> >><br>
>> >> These methods (size, keys, values) are useful for people and I think we<br>
>> >> were not wise to implement them only on top of the local data: this is like<br>
>> >> preferring efficiency over correctness. This also created a lot of confusion<br>
>> >> with our users, question like size() doesn't return the correct value being<br>
>> >> asked regularly. I totally agree that size() returns E (i.e. everything that<br>
>> >> is stored within the grid, including persistence) and it's performance<br>
>> >> implications to be documented accordingly. For keySet and values - we should<br>
>> >> stop implementing them (throw exception) and point users to Will's<br>
>> >> distributed iterator which is a nicer way to achieve the desired behavior.<br>
>> >><br>
>> >> We can also implement keySet() and values() on top of the distributed<br>
>> >> entry iterator and document that using the iterator directly is better.<br>
>> ><br>
>> > Yes, that's what I meant as well.<br>
>> ><br>
>> > Cheers,<br>
>> > --<br>
>> > Mircea Markus<br>
>> > Infinispan lead (<a href="http://www.infinispan.org" target="_blank">www.infinispan.org</a>)<br>
>> ><br>
>> ><br>
>> ><br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > infinispan-dev mailing list<br>
>> > <a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
>> > <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" 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">infinispan-dev@lists.jboss.org</a><br>
>> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> infinispan-dev mailing list<br>
> <a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
_______________________________________________<br>
infinispan-dev mailing list<br>
<a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
</div></div></blockquote></div><br></div></div>