[jbosscache-dev] Re: new utility classes for removing Thread.sleeps

[Mircea Markus]

Manik Surtani wrote:
> Looks good.  A couple of implementation notes:
>
> 1.  ReplicationListener constructor - getting components from the cache
>
> TestingUtil.extractField() is brittle (since it relies on field name) 
> and could break with even the slightest refactoring.  Better to use 
> TestingUtil.extractComponentRegistry() and 
> ComponentRegistry.getComponent() to get the component you want.  Much 
> safer.
>
Fair point. It's not that I've even though about this nice solution :) , 
but in this scenario it just doesn't apply because rpcDispatcher in 
RPCmanagerImpl is build using 'new', not through dependency injection 
and it is not even registered in the component registry.
> 2.  ReplicationListener constructor - updating components in the cache
>
> Again rather than using TestingUtil.replaceField() - equally brittle - 
> try using componentRegistry.registerComponent() to register a new 
> component, and componentRegistry.rewire() if the cache is already 
> running to ensure the new component is re-injected everywhere it is 
> needed.
>
same reason as bellow, the marshaller is not injected.
> 3.  How does this behave with sync replication?  The javadocs say it 
> doesn't make any sense, but it may be done in base test classes, such 
> that the test could have subclasses which use sync and async 
> replication.  I'm guessing waitForReplication() will always return 
> immediately with sync replication?
yes, this is a valid scenario so I'll amend the comment. Some other 
words about using it in the scenarios you mentioned: I've tried 
integrating it in PFER test suite, which has a base test for 
async/sync/optimistic/pessimistic/invalidation and it's just to 
complicated to write the expectations in all scenarios (e.g. if no 
explicit tx pessimistic replication expects the command,but optimistic 
replication expects a commit command). The code would had been to 
complicated (i.e. expectations were very different from config to config 
so I ended up keeping
the old sleep (for this test only) for the sake of readability. PFER are 
some stable tests, so I don't reckon there's a high risk of encountering 
a failure due to sleep  - if this will be the case I'll take the tests 
and write then for each individual configuration rather than having a 
base class.
> Cheers,
> Manik
>
> PS: cc'ing jbosscache-dev - discussions like this should be on the 
> list, just in case anyone else is interested.  :-)
>
>
>
>
> On 29 May 2008, at 08:41, Mircea Markus wrote:
>
>> I've just migrated o.j.c.replicated.AsyncReplTest  to the new 
>> listener mechanism.
>> Can you please take a look and let me know what do you think.
>>
>> Cheers,
>> Mircea
>>
>> Brian Stansberry wrote:
>>> Try o.j.c.replicated.AsyncReplTest
>>>
>>> Mircea Markus wrote:
>>>> Sorry, wrong button :)
>>>> Can you point me to such a class? I'll continue refactoring on Th 
>>>> and I'll start with one of these for convenience. The plan is to 
>>>> remove (almost) all 'sleeps' from code.
>>>>
>>>> Mircea Markus wrote:
>>>>> Can you point me
>>>>>
>>>>> Brian Stansberry wrote:
>>>>>> Thanks. Are you planning to add one for the ordinary REPL_ASYNC 
>>>>>> case; i.e. no replication queue?  That's the one where I end up 
>>>>>> adding Thread.sleep() the most -- waiting to ensure the async 
>>>>>> message has arrived on the remote node before testing failover.
>>>>>>
>>>>>> Mircea Markus wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Take a look at org.jboss.cache.util.internals.EvictionController 
>>>>>>> and org.jboss.cache.util.internals.ReplicationQueueNotifier.
>>>>>>> Right now they are only being used in 
>>>>>>> StateTransferConcurrencyTest, I will change more tests to use 
>>>>>>> them later on this week. Any feedback is welcomed!
>>>>>>> Cheers,
>>>>>>> Mircea
>>>>>>
>>>
>
> -- 
> Manik Surtani
> Lead, JBoss Cache
> manik at jboss.org
>
>
>
>
>
>