<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 8, 2014 at 7:53 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Oct 8, 2014 at 12:41 PM, Dan Berindei <<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>> wrote:<br>
><br>
><br>
> On Wed, Oct 8, 2014 at 6:19 PM, Radim Vansa <<a href="mailto:rvansa@redhat.com">rvansa@redhat.com</a>> wrote:<br>
>><br>
>> Users expect that size() will be constant-time (or linear to cluster<br>
>> size), and generally fast operation. I'd prefer to keep it that way.<br>
>> Though, even the MR way (used for HotRod size() now) needs to crawl<br>
>> through all the entries locally.<br>
><br>
><br>
> They might expect that, but there is nothing in the Map API suggesting it.<br>
><br>
>><br>
>><br>
>> 'Heretic, not very well though of and changing too many things' idea:<br>
>> what about having data container segment-aware? Then you'd just bcast<br>
>> SizeCommand with given topologyId and sum up sizes of primary-owned<br>
>> segments... It's not a complete solution, but at least that would enable<br>
>> to get the number of locally owned entries quite fast. Though, you can't<br>
>> do that easily with cache stores (without changing SPI).<br>
><br>
><br>
> We could create a separate DataContainer for each segment. But would it<br>
> really be worth the trouble? I don't know of anyone using size() for<br>
> something other than checking that their data was properly loaded into the<br>
> cache, and they don't need a super-fast size() for that.<br>
<br>
</span>Having a DataContainer per segment would actually reduce required<br>
memory usage for the distributed iterator as well, since we can query<br>
data by segment much more efficiently and close out segments one by<br>
one per node instead of having to keep multiple open at once. When I<br>
asked about this before it was kind of a we can deal with it later<br>
kind thing. I would think this would increase ST operation time as<br>
well.<br></blockquote><div><br></div><div>You mean it would improve ST performance, because it wouldn't have to compute the hash of each key in the data container?</div><div><br></div><div>I don't think we have ever considered splitting the data container for ST, as it didn't seem worth the trouble. OTOH we wanted to add a segment-based query to the cache loader SPI every since we started designing NBST :)</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
><br>
>><br>
>><br>
>> Regarding cache stores, IMO we're damned anyway: when calling<br>
>> cacheStore.size(), it can report more entries as those haven't been<br>
>> expired yet, it can report less entries as those can be expired due to<br>
>> [1]. Or, we'll enumerate all the entries, and that's going to be slow<br>
>> (btw., [1] reminded me that we should enumerate both datacontainer AND<br>
>> cachestores even if passivation is not enabled).<br>
><br>
><br>
> Exactly, we need to iterate all the entries from the stores if we want<br>
> something remotely accurate (although I had forgotten about expiration also<br>
> being a problem). Otherwise we could just leave size() as it is now, it's<br>
> pretty fast :)<br>
><br>
>><br>
>><br>
>> Radim<br>
>><br>
>> [1] <a href="https://issues.jboss.org/browse/ISPN-3202" target="_blank">https://issues.jboss.org/browse/ISPN-3202</a><br>
>><br>
>> On 10/08/2014 04:42 PM, William Burns wrote:<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>
>> > 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>
>> > 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>
>> > - Will<br>
>> ><br>
>> > On Wed, Oct 8, 2014 at 10:13 AM, Mircea Markus <<a href="mailto:mmarkus@redhat.com">mmarkus@redhat.com</a>><br>
>> > wrote:<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>
>> >>> 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>
>> >>> 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<br>
>> >>> we were not wise to implement them only on top of the local data: this is<br>
>> >>> like preferring efficiency over correctness. This also created a lot of<br>
>> >>> confusion with our users, question like size() doesn't return the correct<br>
>> >>> value being asked regularly. I totally agree that size() returns E (i.e.<br>
>> >>> everything that is stored within the grid, including persistence) and it's<br>
>> >>> performance implications to be documented accordingly. For keySet and values<br>
>> >>> - we should stop implementing them (throw exception) and point users to<br>
>> >>> Will's distributed iterator which is a nicer way to achieve the desired<br>
>> >>> 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>
>> >> 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>
>> > 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>
>> Radim Vansa <<a href="mailto:rvansa@redhat.com">rvansa@redhat.com</a>><br>
>> JBoss DataGrid QA<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>