Why listeners are not invoked? JCache iterator() notifies the listeners.

Can the iterator remove entries?

I assume there's no ordering guarantee, but behaviour under concurrent changes would be rather tricky. I don't like the idea of iterating over changing structure.

If you need to iterate through all entries, I'd rather introduce the snapshot ability and then iterate over the snapshot. Then, you wouldn't have to mess with tx and introduce non-tx operation on tx cache.

My 2c Radim On 03/17/2014 02:30 PM, William Burns wrote: > While working on ISPN-4068 to add the current state to listeners that > were added I found that what I essentially needed was a way to iterate > over the entries of the cache. I am thinking of adding this to the > public API available on the AdvancedCache interface. > > I wanted to get your guy's opinions if you don't think we should add > it or any changes you might suggest. > > My thought was to add 2 overloaded methods: > > <C> Iterator<Map.Entry<K, C>> entryIterator(KeyValueFilter<? super K, > ? super V> filter, Converter<? super K, ? super V, C> converter); > > and > > Iterator<Map.Entry<K, V>> entryIterator(KeyValueFilter<? super K, ? > super V> filter); > > The method would return almost immediately after invocation and the > iterator would queue entries and block as entries are required to be > returned. The filter and converter are applied on each of the remote > nodes and are required to be serializable or have an externalizer > registered. > > Internally the iterator would use chunking to help prevent memory > saturation. The max memory usage would be (chunkSize * N) + local > entries where N is the number of nodes. > > These methods would be different than other methods on the > Cache/AdvancedCache in the following things: > > 1. This operation is treated as nontx and thus won't store them into > the context and thus repeatable read semantics would not be > guaranteed. This doesn't preclude manually adding values to the > context. Also prior writes in the current context would be ignored > (current data returned), although this could be changed if desired. > 2. Values are not activated from loaders and visited listeners would > not be notified of access. The latter could be sensibly changed if > desired. > > - Will > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Radim Vansa <rvansa(a)redhat.com&gt; JBoss DataGrid QA _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

I do realize you need such a feature, still as I pointed out when we first drafted it I'm skeptical because of the complexities you mention. It highly depends on what use cases we want to address, but as a general purpose solution I think this "initial state" received by the clients needs to be accurate, which implies that providing similar iteration guarantees of ConcurrentMap is not fit for the purpose.

In the context of the Haddop integration discussions, the need for a fully consistent iteration was also mentioned; in that specific case we're confident that the feature would still be quite useful even if we assume that the user needs to somehow assume the state "shall not be changed" while the map/reduce jobs make progress. Probably not something that you should be working on in the context of this specific feature, but I'm getting convinced that we should also work on "read consistent iteration" which works on a globally consistent snaphost, could be implemented on TOA I guess. Another use case which comes to mind, is the recent discussion about the need for a consistent and accurate "count" implementation. Note the Snapshot capability would need to take into account CacheStores.

-- Sanne On 17 March 2014 15:02, William Burns <mudokonman(a)gmail.com&gt; wrote: > On Mon, Mar 17, 2014 at 10:45 AM, Radim Vansa <rvansa(a)redhat.com&gt; wrote: >> Why listeners are not invoked? JCache iterator() notifies the listeners. > > Like I mentioned this can be changed. However, I have not seen a > cache entry visitor listener in JCache. The only listeners I am aware > of are for created, removed, expired and updated which wouldn't be > affected by this. > >> >> Can the iterator remove entries? > > Sorry I forgot to mention this, but no it would not be able to remove > with what I was planning. This could be added easily, however this > would be problematic for when using repeatable read. Would the remove > take part of the transaction and if so it causes an inconsistency > since we aren't storing all the values that were read. I figured it > would be easier to just not support it and if the user wants they can > simply call cache.remove(key) which this would do anyways. > >> >> I assume there's no ordering guarantee, but behaviour under concurrent >> changes would be rather tricky. I don't like the idea of iterating over >> changing structure. > > There are no ordering of elements. This would have guarantees more > similar to the ConcurrentMap entrySet iteration in that you are > guaranteed to see a value that was valid for the given key that may or > may not be the most up to date. This would work similarly to how our > Cache works now currently though. > >> >> If you need to iterate through all entries, I'd rather introduce the >> snapshot ability and then iterate over the snapshot. Then, you wouldn't >> have to mess with tx and introduce non-tx operation on tx cache. > > That would definitely be helpful, but we don't have support for > snapshot atm. The way this is implemented currently though it would > be pretty simple to add an option for this though, as all you would > need to do is pass along the version of the snapshot in the request > command. > > The reason I was saying not to support this for tx right now, is > because of repeatable read, there is no way we can hold all the values > of the cache in the current context. > >> >> My 2c >> >> Radim >> >> On 03/17/2014 02:30 PM, William Burns wrote: >>> While working on ISPN-4068 to add the current state to listeners that >>> were added I found that what I essentially needed was a way to iterate >>> over the entries of the cache. I am thinking of adding this to the >>> public API available on the AdvancedCache interface. >>> >>> I wanted to get your guy's opinions if you don't think we should add >>> it or any changes you might suggest. >>> >>> My thought was to add 2 overloaded methods: >>> >>> <C> Iterator<Map.Entry<K, C>> entryIterator(KeyValueFilter<? super K, >>> ? super V> filter, Converter<? super K, ? super V, C> converter); >>> >>> and >>> >>> Iterator<Map.Entry<K, V>> entryIterator(KeyValueFilter<? super K, ? >>> super V> filter); >>> >>> The method would return almost immediately after invocation and the >>> iterator would queue entries and block as entries are required to be >>> returned. The filter and converter are applied on each of the remote >>> nodes and are required to be serializable or have an externalizer >>> registered. >>> >>> Internally the iterator would use chunking to help prevent memory >>> saturation. The max memory usage would be (chunkSize * N) + local >>> entries where N is the number of nodes. >>> >>> These methods would be different than other methods on the >>> Cache/AdvancedCache in the following things: >>> >>> 1. This operation is treated as nontx and thus won't store them into >>> the context and thus repeatable read semantics would not be >>> guaranteed. This doesn't preclude manually adding values to the >>> context. Also prior writes in the current context would be ignored >>> (current data returned), although this could be changed if desired. >>> 2. Values are not activated from loaders and visited listeners would >>> not be notified of access. The latter could be sensibly changed if >>> desired. >>> >>> - Will >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> -- >> Radim Vansa <rvansa(a)redhat.com&gt; >> JBoss DataGrid QA >> >> _______________________________________________ >> 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

Was wondering if anyone had any opinions on the API for this. These are a few options that Dan and I mulled over: Note the CloseableIterable inteface mentioned is just an interface that extends both Closeable and Iterable. 1. The API that is very similar to previously proposed in this list but slightly changed: Methods on AdvancedCache CloseableIterable<CacheEntry<K, V>> entryIterable(KeyValueFilter<? super K, ? super V> filter); <C> CloseableIterable<CacheEntry<K, C>> entryIterable(KeyValueFilter<? super K, ? super V> filter, Converter<? super K, ? super V, C> converter); Note the difference here is that it would return an Iterable instead of Iterator, which would allow for it being used in a for loop. Example usage would be (types omitted) for (CacheEntry entry : cache.entryIterable(someFilter, someConverter)) { // Do something }

2. An API that returns a new type EntryIterable for example that can chain methods to provide a filter and converter. on AdvancedCache EntryIterable<K, V> entryIterable(); where EntryIterable is defined as: public interface EntryIterable<K, V> extends CloseableIterable<CacheEntry<K, V>> { public EntryIterable<K, V> filter(KeyValueFilter<? super K, ? super V> filter); public EntryIterable<K, V> converter(Converter<? super K, ? super V, ? extends V> converter); public <C> CloseableIterable<CacheEntry<K, C>> projection(Converter<? super K, ? super V, C> converter); } Note that there are 2 methods that take a Converter, this is to preserve the typing, since the method would return a different EntryIterable instance. However I can also see removing one of the converter method and just rename projection to converter instead. This API would allow for providing optional fields more cleanly or not if all if desired. Example usage would be (types omitted) for (CacheEntry entry : cache.entryIterable().filter(someFilter).converter(someConverter)) { // Do something }

3. An API that requires the filter up front in the AdvancedCache method. This also brings up the point should we require a filter to always be provided? Unfortuantely this doesn't prevent a user from querying every entry as they can just use a filter that accepts all key/value pairs.

on AdvancedCache EntryIterable<K, V> entryIterable(Filter<? super K, ? super V> filter) where EntryIterable is defined as: public interface EntryIterable<K, V> extends CloseableIterable<CacheEntry<K, V>> { public <C> CloseableIterable<CacheEntry<K, C>> converter(Converter<? super K, ? super V, C> converter); } The usage would be identical to #2 except the filter is always provided.

Let me know what you guys think or if you have any other suggestions. Thanks, - Will _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On Thu, May 1, 2014 at 8:59 AM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: > On 30 April 2014 15:06, William Burns <mudokonman(a)gmail.com&gt; wrote: >> Was wondering if anyone had any opinions on the API for this. >> >> These are a few options that Dan and I mulled over: >> >> Note the CloseableIterable inteface mentioned is just an interface >> that extends both Closeable and Iterable. >> >> 1. The API that is very similar to previously proposed in this list >> but slightly changed: >> >> Methods on AdvancedCache >> >> CloseableIterable<CacheEntry<K, V>> entryIterable(KeyValueFilter<? >> super K, ? super V> filter); >> >> <C> CloseableIterable<CacheEntry<K, C>> >> entryIterable(KeyValueFilter<? >> super K, ? super V> filter, Converter<? super K, ? super V, C> >> converter); >> >> Note the difference here is that it would return an Iterable >> instead >> of Iterator, which would allow for it being used in a for loop. >> >> Example usage would be (types omitted) >> >> for (CacheEntry entry : cache.entryIterable(someFilter, >> someConverter)) { >> // Do something >> } > > > If it's important to close the Iterable, this example highlights a > problem of the API. > Ideally I think you might want to drop the need for the #close() > method, but I'm guessing that's not an option so I'd avoid the > Iterable API in that case. Good point, I totally forgot to cover the Closeable aspect in the first email. Unfortunately changing it to be Iterable does pose a slight issue. I was thinking we do something along the lines that Dan was thinking of by preventing the Iterable from producing more than 1 Iterable (maybe throw IllegalStateException). This way when we close the Iterable it would also close the underlying Iterator. try (EntryIterable<K, C> entries = advancedCache.entryIterable(someFilter, someConverter)) { for (Entry<K, C> e : entries) { ... } } > You could think of an intermediary place-holder to still allow for > natural iteration: > > try ( CacheEntryIteratorContext ctx = > cache.entryIterable(someFilter, > someConverter) ) { > for (CacheEntry entry : ctx.asIterable()) { > // Do something > } > } > > But I'm not liking the names I used above, as I would expect to be > able to reuse the same iterator for multiple invocations of > iterable(), and have each to restart the iteration from the > beginning. Obviously from above this wouldn't be possible if we made those changes. Do you think this is reason to prevent those changes? Or do you think we should allow multiple iterators but closing the Iterable would also close down each of the Iterators? I am worried this might be a bit cumbersome/surprising, but documenting it might be sufficient.

> Can this be solved with better name choices?

> > > >> 2. An API that returns a new type EntryIterable for example that >> can >> chain methods to provide a filter and converter. >> >> on AdvancedCache >> >> EntryIterable<K, V> entryIterable(); >> >> where EntryIterable is defined as: >> >> public interface EntryIterable<K, V> extends >> CloseableIterable<CacheEntry<K, V>> { >> >> public EntryIterable<K, V> filter(KeyValueFilter<? super K, ? >> super >> V> filter); >> >> public EntryIterable<K, V> converter(Converter<? super K, ? >> super >> V, ? extends V> converter); >> >> public <C> CloseableIterable<CacheEntry<K, C>> >> projection(Converter<? super K, ? super V, C> converter); >> } >> >> Note that there are 2 methods that take a Converter, this is to >> preserve the typing, since the method would return a different >> EntryIterable instance. However I can also see removing one of the >> converter method and just rename projection to converter instead. >> >> This API would allow for providing optional fields more cleanly or >> not >> if all if desired. >> >> Example usage would be (types omitted) >> >> for (CacheEntry entry : >> cache.entryIterable().filter(someFilter).converter(someConverter)) >> { >> // Do something >> } > > This looks very nice, assuming you fix the missing close(). So in that case you like #2 I assume? That is what I was leaning towards as well. > Am I missing a catch? Yes it would be very similar to above with the outer try block and then passing in the CloseableIterable into the inner for loop. > > Also it's quite trivial for the user to do his own filtering and > conversion in the "do something block", so I'm wondering if there > is a > reason beyond API shugar to expose this. The filters and converters are sent remotely to reduce resulting network traffic. The filter is applied on each node to determine what data it sends back and the converter is applied before sending back the value as well. > It would be lovely if for example certain filters could affect > loading > from CacheStores - narrowing down a relational database select for > example - and I guess the same concept could apply to the converted > if > you'd allow to select a subset of fields. There are plans to have a JPA string filter/converter that will live in a new forthcoming module. We could look into enhancing this in the future to have better integration with the JPACacheStore. A possible issue I can think of is if the projection doesn't contain the key value, because currently rehash can cause duplicate values that is detected by the key. > > I don't think these optimisations need to be coded right now, but it > would be nice to keep the option open for future enhancement. I agree that would be nice > > >> 3. An API that requires the filter up front in the AdvancedCache >> method. This also brings up the point should we require a filter >> to >> always be provided? Unfortuantely this doesn't prevent a user from >> querying every entry as they can just use a filter that accepts all >> key/value pairs. > > Why is that unfortunate? I was saying along the lines if we wanted to make sure a user doesn't query all data. But if we do want them to do this, then it is great. I know some people are on the fence about it.

> >> >> on AdvancedCache >> >> EntryIterable<K, V> entryIterable(Filter<? super K, ? super V> >> filter) >> >> where EntryIterable is defined as: >> >> public interface EntryIterable<K, V> extends >> CloseableIterable<CacheEntry<K, V>> { >> >> public <C> CloseableIterable<CacheEntry<K, C>> >> converter(Converter<? super K, ? super V, C> converter); >> } >> >> The usage would be identical to #2 except the filter is always >> provided. > > I wouldn't mandate it, but in case you overload the method it > probably > is a good idea to internally apply an accept-all filter so you have > a > single implementation. Could be a singleton, which also implies an > efficient Externalizer. Although for performance I guess it would be better to not provide a Filter for the retrieve all case.

> >> >> >> Let me know what you guys think or if you have any other >> suggestions. >> >> Thanks, >> >> - Will >> _______________________________________________ >> 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

On Thu, May 1, 2014 at 4:50 PM, William Burns <mudokonman(a)gmail.com&gt; wrote: On Thu, May 1, 2014 at 8:59 AM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: On 30 April 2014 15:06, William Burns <mudokonman(a)gmail.com&gt; wrote: Was wondering if anyone had any opinions on the API for this. These are a few options that Dan and I mulled over: Note the CloseableIterable inteface mentioned is just an interface that extends both Closeable and Iterable. 1. The API that is very similar to previously proposed in this list but slightly changed: Methods on AdvancedCache CloseableIterable<CacheEntry<K, V>> entryIterable(KeyValueFilter<? super K, ? super V> filter); <C> CloseableIterable<CacheEntry<K, C>> entryIterable(KeyValueFilter<? super K, ? super V> filter, Converter<? super K, ? super V, C> converter); Note the difference here is that it would return an Iterable instead of Iterator, which would allow for it being used in a for loop. Example usage would be (types omitted) for (CacheEntry entry : cache.entryIterable(someFilter, someConverter)) { // Do something } If it's important to close the Iterable, this example highlights a problem of the API. Ideally I think you might want to drop the need for the #close() method, but I'm guessing that's not an option so I'd avoid the Iterable API in that case. Good point, I totally forgot to cover the Closeable aspect in the first email. Unfortunately changing it to be Iterable does pose a slight issue. I was thinking we do something along the lines that Dan was thinking of by preventing the Iterable from producing more than 1 Iterable (maybe throw IllegalStateException). This way when we close the Iterable it would also close the underlying Iterator. try (EntryIterable<K, C> entries = advancedCache.entryIterable(someFilter, someConverter)) { for (Entry<K, C> e : entries) { ... } } You could think of an intermediary place-holder to still allow for natural iteration: try ( CacheEntryIteratorContext ctx = cache.entryIterable(someFilter, someConverter) ) { for (CacheEntry entry : ctx.asIterable()) { // Do something } } But I'm not liking the names I used above, as I would expect to be able to reuse the same iterator for multiple invocations of iterable(), and have each to restart the iteration from the beginning. Obviously from above this wouldn't be possible if we made those changes. Do you think this is reason to prevent those changes? Or do you think we should allow multiple iterators but closing the Iterable would also close down each of the Iterators? I am worried this might be a bit cumbersome/surprising, but documenting it might be sufficient. I don't think it would be surprising at all to invalidate all iterators on close, just as modifying java.util collections in any way invalidates all iterators.

Not allowing the user to iterate twice over the same Iterable, as I suggested, might be surprising, but the user can easily change his code to work around that.

I'm not sure the intermediate asIterable() call helps in any way, though, because it's just as easy for the user to "forget" to call close():

for (CacheEntry entry : ctx.entryIterable(someFilter, someConverter).asIterable()) { // Do something } It would be nice if Java would have followed C# in automatically calling close() at the end of a foreach loop, but I don't see a way to force the user to call close(). Can this be solved with better name choices? I don't think so...

2. An API that returns a new type EntryIterable for example that can chain methods to provide a filter and converter. on AdvancedCache EntryIterable<K, V> entryIterable(); where EntryIterable is defined as: public interface EntryIterable<K, V> extends CloseableIterable<CacheEntry<K, V>> { public EntryIterable<K, V> filter(KeyValueFilter<? super K, ? super V> filter); public EntryIterable<K, V> converter(Converter<? super K, ? super V, ? extends V> converter); public <C> CloseableIterable<CacheEntry<K, C>> projection(Converter<? super K, ? super V, C> converter); } Note that there are 2 methods that take a Converter, this is to preserve the typing, since the method would return a different EntryIterable instance. However I can also see removing one of the converter method and just rename projection to converter instead. This API would allow for providing optional fields more cleanly or not if all if desired. Example usage would be (types omitted) for (CacheEntry entry : cache.entryIterable().filter(someFilter).converter(someConverter)) { // Do something } This looks very nice, assuming you fix the missing close(). So in that case you like #2 I assume? That is what I was leaning towards as well. Am I missing a catch? Yes it would be very similar to above with the outer try block and then passing in the CloseableIterable into the inner for loop. Also it's quite trivial for the user to do his own filtering and conversion in the "do something block", so I'm wondering if there is a reason beyond API shugar to expose this. The filters and converters are sent remotely to reduce resulting network traffic. The filter is applied on each node to determine what data it sends back and the converter is applied before sending back the value as well. It would be lovely if for example certain filters could affect loading from CacheStores - narrowing down a relational database select for example - and I guess the same concept could apply to the converted if you'd allow to select a subset of fields. There are plans to have a JPA string filter/converter that will live in a new forthcoming module. We could look into enhancing this in the future to have better integration with the JPACacheStore. A possible issue I can think of is if the projection doesn't contain the key value, because currently rehash can cause duplicate values that is detected by the key. I don't think these optimisations need to be coded right now, but it would be nice to keep the option open for future enhancement. I agree that would be nice 3. An API that requires the filter up front in the AdvancedCache method. This also brings up the point should we require a filter to always be provided? Unfortuantely this doesn't prevent a user from querying every entry as they can just use a filter that accepts all key/value pairs. Why is that unfortunate? I was saying along the lines if we wanted to make sure a user doesn't query all data. But if we do want them to do this, then it is great. I know some people are on the fence about it. This was my original proposal. My main concern was finding a good name for the entryIterable() method, so I figured we could avoid it completely: try (CloseableIterable entries = cache.filter(filter).convert(converter)) { for (CacheEntry entry : ctx.asIterable()) { // Do something } }

From an API such as "cache.filter(filter)" I would expect a return of

I figured it would be good to nudge users toward using a filter instead of filtering in the for block, because of the savings in network. OTOH, even though it doesn't prevent the user from iterating over all the entries in the cache, it does make it look a bit awkward. on AdvancedCache EntryIterable<K, V> entryIterable(Filter<? super K, ? super V> filter) where EntryIterable is defined as: public interface EntryIterable<K, V> extends CloseableIterable<CacheEntry<K, V>> { public <C> CloseableIterable<CacheEntry<K, C>> converter(Converter<? super K, ? super V, C> converter); } The usage would be identical to #2 except the filter is always provided. I wouldn't mandate it, but in case you overload the method it probably is a good idea to internally apply an accept-all filter so you have a single implementation. Could be a singleton, which also implies an efficient Externalizer. Although for performance I guess it would be better to not provide a Filter for the retrieve all case. I doubt the size of a custom filter would matter much compared to all the entries in the cache, the only issue is the ease of use. Let me know what you guys think or if you have any other suggestions. Thanks, - Will _______________________________________________ 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 _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev