[jbosscache-dev] Re: new utility classes for removing Thread.sleeps
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(a)jboss.org