river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Firmstone <peter.firmst...@zeus.net.au>
Subject Discussion of recent changes to ServiceDiscoveryManager and Reggie.
Date Mon, 03 Feb 2014 10:47:25 GMT
Replacement of TaskManager with ExecutorService.

ServiceDiscoveryManager:

1.Complex dependency relationships.

2.Dependency is the exception, not the norm, existing runAfter(List 
task, int position) implementations, changed to dependsOn(CacheTask t), 
iteration being performed by the caller rather than the task implementation.

3.Used a wrapper ExecutorService to customise FutureTask to implement 
Observer call backs to notify dependant waiting tasks.

4.In addition to call backs, implemented subscriber to allow tasks to 
fire off new tasks and notify their dependants to wait for these also.

5.Dependant tasks manage a small lists of tasks they’re waiting to be 
notified for completion, when the last precedent task is removed, the 
dependant task is submitted to the executor.

Fixed a number of race conditions that were revealed as a result of 
replacing TaskManager. TaskManager masks data races by making code that 
submit and polls tasks relatively single threaded. The multiple threads 
in TaskManager were useful to hand off network calls.

Reggie:

1.Many dependencies, with a simple relationship, to quote Patricia:

    EventTask - run after EventTask for same listener, "Keep events
    going to the same listener ordered";

2.Not suited to the call back implementation in SDM.

3.Better suited to a simple priority queue, for tasks that were 
dependant to be executed in order, but only dependant tasks, those tasks 
with the same listener.

4.Dependant tasks must be single threaded.

5.Rather than have many single threaded executors, I used a number of 
priority queues, one for each listener, that utilise call backs to 
remove a task after it completes, preventing execution of dependant tasks.

6.Although dependency relationships have been preserved, tests expect 
remote event listeners to be notified in the order they register with 
the ServiceRegistrar, this behaves as expected with TaskManager, but 
ExecutorService is asynchronous and listeners are notified out of order. 
Yes that's right, to be compliant with qa tests, not only must events 
for each listener be ordered, but the listeners themselves must be 
notified in the order or their registration with the lookup service.

7.In the new implementation, Events are still ordered correctly to each 
listener, it’s just the first listener that registered may not be 
notified first when an event occurs because of the asynchronous nature 
of ExecutorService implementations, unlike TaskManager.

8.We need to consider the impact of maintaining 100% backward compatible 
behaviour on performance and complexity. This can be managed by ensuring 
there is a bottleneck in the ExecutorService, by limiting performance, 
order of notification to listeners is preserved. A fixed length queue 
with an ExecutorService that only creates new threads once the queue is 
full should do the trick.

a.This may sound brittle, but it’s the same guarantee that TaskManager 
provided, so the default ExecutorService can be performance limited just 
like TaskManager.

b.For those who want to tune for all out performance and don’t care 
which listener is notified first, they can configure their own 
ExecutorService implementation.

c.The complex option of creating dependency links between listeners 
harms performance, so there’s no point introducing this type of 
dependency as it forces everyone’s performance to suffer on the altar of 
compatibility.

What are your thoughts?

Peter.

Mime
View raw message