On Tue, Mar 21, 2017 at 1:42 PM William Burns <mudokon...@gmail.com> wrote:
> On Tue, Mar 21, 2017 at 12:53 PM Radim Vansa <rva...@redhat.com> wrote: > > On 03/21/2017 04:37 PM, William Burns wrote: > > Some users have expressed the need to have some sort of forEach > > operation that is performed where the Consumer is called while holding > > the lock for the given key and subsequently released after the > > Consumer operation completes. > > Seconding Dan's question - is that intended to be able to modify the > entry? In my opinion, sending a function that will work on the > ReadWriteEntryView directly to the node is the only reasonable request. > I wouldn't like to see blocking operations in there. > > > Hrmm the user can use the FunctionalMap interface for this then it seems? > I wonder if this should just be the going in API. I will need to discuss > with Galder the semantics of the evalAll/evalMany methods. > Actually looking at evalAll it seems it doesn't scale as it keeps all entries in memory at once, so this is only for caches with a limited amount of entries. > > > > > > > Due to the nature of how streams work with retries and performing the > > operation on the primary owner, this works out quite well with forEach > > to be done in an efficient way. > > > > The problem is that this only really works well with non tx and > > pessimistic tx. This obviously leaves out optimistic tx, which at > > first I was a little worried about. But after thinking about it more, > > this prelocking and optimistic tx don't really fit that well together > > anyways. So I am thinking whenever this operation is performed it > > would throw an exception not letting the user use this feature in > > optimistic transactions. > > How exactly reading streams interacts with transactions? Does it wrap > read entries into context? This would be a scalability issue. > > > It doesn't wrap read entries into the context for that exact reason. It > does however use existing entries in the context to override ones in > memory/store. > > > > I agree that "locking" should not be exposed with optimistic transactions. > > > Yeah I can't find a good way to do this really and it seems to be opposite > of what optimistic transactions are. > > > > With pessimistic transactions, how do you expect to handle locking > order? For regular operations, user is responsible for setting up some > locking order in order to not get a deadlock. With pessimistic > transaction, it's the cache itself who will order the calls. Also, if > you lock anything that is read, you just end up locking everything (or, > getting a deadlock). If you don't it's the same as issuing the lock and > reading again (to check the locked value) - but you'd do that internally > anyway. Therefore, I don't feel well about pessimistic transactions > neither. > > > The lock is done per key only for each invocation. There is no ordering as > only one is obtained at a time before it goes to the next. If the user then > acquires a lock for another key while in the Consumer this could cause a > deadlock if the inverse occurs on a different thread/node, but this is on > the user. It is the same as it is today really, except we do the read lock > for them before invoking their Consumer. > > > > > > > Another question is what does the API for this look like. I was > > debating between 3 options myself: > > > > 1. AdvancedCache.forEachWithLock(BiConsumer<Cache, CacheEntry<K, V>> > > consumer) > > > > This require the least amount of changes, however the user can't > > customize certain parameters that CacheStream currently provides > > (listed below - big one being filterKeys). > > > > 2. CacheStream.forEachWithLock(BiConsumer<Cache, CacheEntry<K, V>> > > consumer) > > > > This method would only be allowed to be invoked on the Stream if no > > other intermediate operations were invoked, otherwise an exception > > would be thrown. This still gives us access to all of the CacheStream > > methods that aren't on the Stream interface (ie. > > sequentialDistribution, parallelDistribution, parallel, sequential, > > filterKeys, filterKeySegments, distributedBatchSize, > > disableRehashAware, timeout). > > For both options, I don't like Cache being passed around. You should > modify the CacheEntry (or some kind of view) directly. > > > I don't know for sure if that is sufficient for the user. Sometimes they > may modify another Cache given the value in this one for example, which > they could access from the CacheManager of that Cache. Maybe Tristan knows > more about some use cases. > > > > Radim > > > > > 3. LockedStream<CacheEntry<K, V>> AdvancedCache.lockedStream() > > > > This requires the most changes, however the API would be the most > > explicit. In this case the LockedStream would only have the methods on > > it that are able to be invoked as noted above and forEach. > > > > I personally feel that #3 might be the cleanest, but obviously > > requires adding more classes. Let me know what you guys think and if > > you think the optimistic exclusion is acceptable. > > > > Thanks, > > > > - Will > > > > > > _______________________________________________ > > infinispan-dev mailing list > > infinispan-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > -- > Radim Vansa <rva...@redhat.com> > JBoss Performance Team > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > >
_______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
