[infinispan-dev] CR3: async stores, again

[Manik Surtani]

On 31 Aug 2010, at 12:46, Sanne Grinovero wrote:

> 2010/8/31 Mircea Markus <mircea.markus at jboss.com>:
>> 
>> On 31 Aug 2010, at 11:57, Sanne Grinovero wrote:
>> 
>>> 2010/8/31 Mircea Markus <mircea.markus at jboss.com>:
>>>> 
>>>> 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?

Yeah I think a config option makes sense.  I presume this timeout is a "max wait time", not a "guaranteed wait time"?  If the former, a default of 1h or something large, to make sure writes are flushed would be a sensible default.

--
Manik Surtani
manik at jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org