Re: [infinispan-dev] CR3: async stores, again

2010/8/31 Mircea Markus <mircea.markus(a)jboss.com&gt;: > > On 31 Aug 2010, at 11:57, Sanne Grinovero wrote: > >> 2010/8/31 Mircea Markus <mircea.markus(a)jboss.com&gt;: >>> >>> On 27 Aug 2010, at 19:37, Sanne Grinovero wrote: >>> >>>> Some more news about this: >>>> Mircea attached a patch he was working on to JIRA, but he hadn't time >>>> to fix all tests, so I'm continuing. >>>> >>>> There are two more issues: >>>> >>>> 1)keeping an ordered list of transactions works fine for transactions >>>> only, but modifications done in a transaction are wrongly reordered >>>> with operations not being done in a transaction. >>> ah I see. The operations within a transaction do not get reordered though, right? >> >> That's right, if I only use transactions it works as a charm, ignoring >> the corner-case of the clear() operation as explained in next >> paragraph. >> The problem with Lucene (to show a use case) is that it sometimes >> starts a batch for performance reasons; other one-shot operations are >> missing the batch. >> >>> >>>> >>>> 2) Even in the committed current code, I see that a clear() operation >>>> is sent async together with all other operations. Then, when the state >>>> changes are performed, the clear() causes the previously found >>>> operations to be canceled, but "previous" applies here on the iterator >>>> on the state map, not on the order of application of those state >>>> Modifications, so the clear() is removing random Modifications. >>> yes >>>> >>>> 3) Also my attempt to reuse some code from AbstractCacheStore though >>>> inheritance is not going to make it as this implementation is doing >>>> too much for the needs of AsyncStore: it cares for async Purge, and >>>> all methods I wanted to commit are slightly different because of that. >>>> >>>> Proposal: >>>> 1- Out-of-transaction modifications are pushed into a BlockingDeque >>>> instead of a Map. >>> do we need a deque for this? Also do we want it blocking? Perhaps we can make this configurable and operate on an interface directly? >> >> any kind of order-mantaining will do, I did it with an unbounded >> blocking deque as that's nice to wait on for new work to appear, to >> avoid starvation and provide a clean road for shutdown. >> >>>> >>>> 2- A prepare(tx, list) sets the list aside in the "transactions" Map. >>>> >>>> 3- A commit(tx) takes the list of operations from the "transactions" >>>> and pushes the list on the same BlockingDeque as all other operations, >>>> guaranteeing same order of application. >>> looks good >>>> >>>> 4- a clear operation clears the deque and pushes a Clear() operation on it >>>> >>>> 5- a purge operation is sent though the same deque: only risk I see is >>>> being removed by a clear, but I bet there won't be much to purge after >>>> that :) Also we don't care if the purge is going to be handled async >>>> in the other store, that's a store implementation detail from what I >>>> see in AbstractCacheStore. >> Not sure we need to process purge in an async way: it is already >> called asynchronously by the eviction thread, so we can just delegate >> the call to the underlaying call. >> Right, I got inspired by previous version. I have it done now by the >> AsyncStoreCoordinator, not spawning additional AsyncProcessors, as >> it's assumed to be async and return quickly; this way this kind of >> operation gets a sort of "priority" towards other modifications, but >> sill async from the point of view of the caller. > The caller of purge is the eviction thread which is again an async thread (not a user thread). So there would be two levels of async calls. > The only way in which a user can call purge is through the AdvancedCache.getEvictionManager().processEviction() - but I think if one does so it would want to have purging executed sync. +1, changing that. Now I'm in need of feedback about adding the "shutdown timeout" option to AsyncStoreConfig; and what to set as default?