[Geotools-devel] Process extends Callable

[Jody G. <jga...@re...>]

Hi Martin this is not proving an effective use of either of our time; I 
am not sure you understand the design of Executor/Callable/Future. Many 
of your questions seem focused on the technicalities of stopping a 
process; rather than the responsibilities and relationships of the 
objects involved.

Basically if you are waiting for me to come up with a design that says 
"Process extends Callable" I have already provided it; It does not look 
exactly like what you are expecting to see so I am worried that it is 
not being considered for what it is.

I need to make sure; am I being understood about the requirements for a 
Process interface? If not we should get out of the details and return to 
the goals.

I have commented a bit more inline, but we need another way to discuss this.
Jody

Martin Desruisseaux wrote:
> But we don't need a point of contact!
>
> We would need a point of contact if a call to Thread.interrupt() was 
> the only way to stop a process, i.e. if the process has no build-in 
> support for cancelation. But if you require a process to implement the 
> Process interface and if this interface requires to implement a 
> cancel() method, then implementors are required to support cancelation 
> in the way recommanded by the link above (or something similar). This 
> is what Image I/O does and it work well (I use it).
>
> The Future.cancel() method could be seen as more aggresive way to 
> cancel a progress for the waiting thread only. This is not the only 
> way and not necessarly the preferred one since the approach in the 
> above link would leave the system in a cleaner state.
I need a better word than "point of contact". This comes down to design 
and responsibility ... if I am defining a Process API it is a contract 
between the "Executor" (or Process Engine) and the bit of code being 
run. You are asking me to open up this relationship to others (a list of 
listeners), and I find that a poor choice that is in fact limiting (I 
cannot spread the tasks across servers), dangerous (any listener could 
have a mistake) and unnecessary.
>> These facilities do not provide any kind of information or reporting 
>> on the running Callable. The moment we provide more we have stepped 
>> outside of what Future / Callable can do out of the box.
> Yes but again why the following is not acceptable?
>
>     interface Process extends Callable {
>         void addProgressListener(...)
>         void removeProgressListener(...)
>         void cancel();
>     }
>
Line by line:
- Progress.cancel() - what you describe here *has* to exist. It can be 
covered by ProgressListener.isCanceled(). But *yes* this functionality 
is needed for an Executor to stop the progress. Providing this 
functionality as a parameter object (ProgressListener currently) takes 
the responsibility away from the person implementing the Process (and it 
is a *huge* responsibility that I personally don't trust them to get 
right - you cannot dictate that they syncronize their cancel() method).
- Progress.addProgressListener(...) violates information hiding and the 
reason for an Executor. Any listeners should be handled by a callback 
object provided by the Executor; don't make it harder for anyone to 
implement a Process than it needs to be.
> It goes against the usage of Future.cancel(), but I don't see that as 
> a problem.
> * Listeners can stop a process using Process.cancel(), which should be 
> implemented in the way described in the above link as in Image I/O.
Are you expecting the Process implementor to check their entire list of 
listeners? Please note that this technique is what is used by the 
Executor (by way of the ProgressListener) in my design; I find it way 
more safe/sane then anything todo with Thread (and as a bonus you can 
recycle your Thread for the next Process).
> * The thread that holds the Future object can invokes Process.cancel() 
> or Future.cancel() at its choice - the later can been see as a more 
> aggressive way to kill a process if we don't want to let it finish 
> cleanly.
Yes that is the point where we have different designs. I don't want the 
Thread that holds the Future object to have any way of influencing 
things other than the Executor; or the Future callback object the 
Executor provided.

So let me define something for you that is *very* similar to what you 
describe above; just different in intent.

interface Process extends Callable<Map> {
    void setInput( Map input ); // part of the Process contract; we are 
going to process the data provided as input
    void setProgressListener( ... ); // Contract with executor, no need 
for cancel() since progressListener.isCancelled() exists
}
Progress extends Future {
    addListener( ... );
    removeListener( ... );
    int getProgress();
    String getTask();
}