Mailing-List: contact dev-help@river.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@river.apache.org
Received-SPF: pass (athena.apache.org: domain of gergg@cox.net designates
 68.230.241.147 as permitted sender)
Content-Type: text/plain; charset=windows-1252
Mime-Version: 1.0 (Mac OS X Mail 7.1 \(1827\))
Subject: Re: TaskManager as an ExecutorService
From: Gregg Wonderly <gergg@cox.net>
In-Reply-To: <AYEo1n00q03wcJL01YEqza>
Date: Wed, 8 Jan 2014 11:28:28 -0600
Content-Transfer-Encoding: quoted-printable
Message-Id: <B03A9018-C39F-463F-93A9-1B07633960B0@cox.net>
References: <52C7E834.2070201@zeus.net.au>
 <FE3394D7-6371-4185-AF71-B66C4851CA68@stratuscom.com>
 <1388879476.6007.11.camel@Nokia-N900> <A1mt1n00L03wcJL011mu7p>
 <A5E64842-BF2A-49D1-859A-C6C8608089D6@cox.net> <AYEo1n00q03wcJL01YEqza>
To: dev@river.apache.org

=46rom the very early days of Jini, I have not had good luck with =
ServiceDiscoveryManager.  I actually have perhaps 3 different version of =
this behavior coded into various applications in different ways, because =
SDM just did not seem to provide sane behavior for me, ever.  I am not =
sure that I have spare cycles at this point to engage in development on =
River, because I am off doing some other non-Java work at this point, =
and I won=92t be as productive doing that, as I=92d like to be.

I would like to get involved with River development, but I am not at a =
place that I can do that right now.  I don=92t want to just be a =
non-participant, so I am trying to read through issues here and offer =
whatever experiences I can recall as examples of why the River codebase, =
truly is in need of all the work that Peter has been slaving through.

In many cases, people don=92t seem to see problems.  In service only =
environments, perhaps things join quickly enough and you are satisfied =
with what is happening.  In my applications, things are heavily client =
based, with clients being started and stopped continuously by people =
trying to access serviceUIs to do a task.  In that environment, the =
behaviors of all this broken and inefficient locking/synchronization, it =
becomes very painful and obvious, quite quickly that things are busted, =
big time.

Gregg

On Jan 6, 2014, at 2:14 AM, Peter <jini@zeus.net.au> wrote:

> Gregg,
>=20
> Are you able to help out with ServiceDiscoveryManager?
>=20
> Fixing SDM would make a huge stability improvement.
>=20
> Guava has an interface called ListenableFuture, which sounds like a =
possible candidate, listeners are registered and notified in the event =
of failure or success.
>=20
> The Guava site claims it makes it possible to have complex chains of =
asynchronous operations.
>=20
> Regards,
>=20
> Peter.
>=20
> ----- Original message -----
>> I think the better choice is to not try and handle failure with retry =
at
>> all.     Instead, we should use API which allows the dependent task =
to
>> know whether it=92s dependent has completed or failed.   It can then =
report
>> failure if its dependent failed, or submit its own work to be =
executed
>> in the queue.  =20
>>=20
>> This linking of application behavior into utility APIs is just not =
good,
>> testable design.   It requires that behaviors on the outside of the =
pool,
>> flow through to other uses of the API in non-programatic design ways.
>>=20
>> Gregg Wonderly
>>       =20
>> On Jan 4, 2014, at 7:46 PM, Peter <jini@zeus.net.au> wrote:
>>=20
>>> For a moment, lets consider how we might fix TaskManager, while
>>> retaining the existing Task.runAfter method and how the fix would
>>> impact TaskManager's users.
>>>=20
>>> TaskManager, like most thread pools has a queue.
>>>=20
>>> Before a task is removed from the queue for execution, it is asked =
if
>>> it should "runAfter" any other task present in the queue, while this
>>> occurs, the queue is locked.
>>>=20
>>> Each task is removed from the queue before it executes.
>>>=20
>>> Now what happens if a task fails to execute, or it's thread is
>>> suspended by the OS scheduler and is not in the queue during =
execution
>>> and another task/s that depend on it need to "runAfter" are allowed =
to
>>> execute before it completes?
>>>=20
>>> Because a task doesn't know which tasks depend upon it running =
first,
>>> the only way to fix TaskManager is to only remove a task after it
>>> completes successfully.
>>>=20
>>> Now TaskManager has no scheduling capability, which means that the =
fix
>>> will require the task to both remain in the queue and consume a pool
>>> thread until it successfully completes because once the Task.run
>>> method completes it is assumed to have completed successfully.
>>>=20
>>> RetryTask and WakupManager would also need to be rewritten to
>>> accommodate this requirement.
>>>=20
>>> The result will be correctly ordered execution of tasks, with
>>> increased thread & memory consumption as well as significantly =
reduced
>>> throughput.
>>>=20
>>> The reduced throughput could also help mask other issues by reducing
>>> concurrency.
>>>=20
>>> Regards,
>>>=20
>>> Peter.
>>>=20
>>>=20
>>> ----- Original message -----
>>>> The problem is with TaskManager's public api method Task.runAfter.  =
=20
>>>> This is well documented in River-344.
>>>>=20
>>>> The fix requires changing every class that uses it
>>>>=20
>>>> As a thread pool TaskManager is correct provided that no ordering
>>>> dependencies exist between tasks.
>>>>=20
>>>> TaskManager doesn't compare to Doug Lee's ExecutorService
>>>> implementations, it should be consigned to history, lets clean our
>>>> code up.
>>>>=20
>>>> What configuration choice did we originally have anyway?     Any
>>>> Executor you like as long as it's TaskManager?
>>>>=20
>>>> TaskManager is not part of our public api, it's an implementation
>>>> detail in com.sun.*
>>>>=20
>>>> Regards,
>>>>=20
>>>> Peter.
>>>> ----- Original message -----
>>>>>=20
>>>>> I=92d like you to make a reasonable case for why TaskManager needs
>>>>> to be replaced, requiring changes to many other classes that
>>>>> depend on TaskManager, rather than stating what the problem is
>>>>> with TaskManager and seeking to fix it, which would only affect
>>>>> TaskManager and not require modifying and then debugging other
>>>>> code.
>>>>>=20
>>>>> Greg.
>>>>>=20
>>>>> On Jan 4, 2014, at 5:53 AM, Peter Firmstone <jini@zeus.net.au>
>>>>> wrote:
>>>>>=20
>>>>>> Would you like me to add this class, so that existing
>>>>>> configurations utilising a TaskManager can also be used?       =
This
>>>>>> might be useful for retaining backward compatibility with
>>>>>> existing configurations?
>>>>>>=20
>>>>>> Regards,
>>>>>>=20
>>>>>> Peter.
>>>>>>=20
>>>>>>=20
>>>>>> /*
>>>>>> * Licensed to the Apache Software Foundation (ASF) under one
>>>>>> * or more contributor license agreements.       See the NOTICE =
file
>>>>>> * distributed with this work for additional information
>>>>>> * regarding copyright ownership. The ASF licenses this file
>>>>>> * to you under the Apache License, Version 2.0 (the
>>>>>> * "License"); you may not use this file except in compliance
>>>>>> * with the License. You may obtain a copy of the License at
>>>>>> *
>>>>>> *                       =
http://www.apache.org/licenses/LICENSE-2.0
>>>>>> *
>>>>>> * Unless required by applicable law or agreed to in writing,
>>>>>> software * distributed under the License is distributed on an
>>>>>> "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
>>>>>> either express or implied. * See the License for the specific
>>>>>> language governing permissions and * limitations under the
>>>>>> License. */
>>>>>>=20
>>>>>> package com.sun.jini.thread;
>>>>>>=20
>>>>>> import com.sun.jini.thread.TaskManager.Task;
>>>>>> import java.util.List;
>>>>>> import java.util.concurrent.AbstractExecutorService;
>>>>>> import java.util.concurrent.ExecutorService;
>>>>>> import java.util.concurrent.RejectedExecutionException;
>>>>>> import java.util.concurrent.TimeUnit;
>>>>>>=20
>>>>>> /**
>>>>>> *
>>>>>> * @author peter
>>>>>> */
>>>>>> public class TaskManagerWrapper extends AbstractExecutorService
>>>>>> implements ExecutorService {
>>>>>>=20
>>>>>> private final TaskManager tm;
>>>>>> private final PosionPill pill;
>>>>>> private volatile boolean isShutdown;
>>>>>>=20
>>>>>> public TaskManagerWrapper(TaskManager manager){
>>>>>> tm =3D manager;
>>>>>> isShutdown =3D false;
>>>>>> pill =3D new PosionPill(manager);
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public void shutdown() {
>>>>>> isShutdown =3D true;
>>>>>> tm.add(pill);
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public List<Runnable> shutdownNow() {
>>>>>> isShutdown =3D true;
>>>>>> tm.terminate();
>>>>>> return tm.getPending();
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public boolean isShutdown() {
>>>>>> return isShutdown;
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public boolean isTerminated() {
>>>>>> return isShutdown;
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public boolean awaitTermination(long timeout, TimeUnit unit)
>>>>>> throws InterruptedException { long start =3D
>>>>>> System.currentTimeMillis(); long duration =3D
>>>>>> unit.toMillis(timeout); synchronized (pill){
>>>>>> while (!pill.terminated){
>>>>>> wait(duration);
>>>>>> if (pill.terminated) return true;
>>>>>> long elapsed =3D System.currentTimeMillis() - start;
>>>>>> if (elapsed >=3D duration) return false;
>>>>>> duration =3D duration - elapsed;
>>>>>> }
>>>>>> }
>>>>>> return true; // pill was terminated.
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public void execute(Runnable command) {
>>>>>> if (isShutdown) throw new RejectedExecutionException("TaskManager
>>>>>> terminated"); }
>>>>>>=20
>>>>>> private static class PosionPill implements Task {
>>>>>>=20
>>>>>> private final TaskManager tm;
>>>>>> boolean terminated;
>>>>>>=20
>>>>>> PosionPill(TaskManager tm){
>>>>>> this.tm =3D tm;
>>>>>> terminated =3D false;
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public boolean runAfter(List tasks, int size) {
>>>>>> if (!tasks.isEmpty()) return true; // Make sure we always run
>>>>>> last. return false;
>>>>>> }
>>>>>>=20
>>>>>> @Override
>>>>>> public void run() {
>>>>>> tm.terminate(); // Bye.
>>>>>> synchronized (this){
>>>>>> terminated =3D true;
>>>>>> notifyAll();
>>>>>> }
>>>>>> }
>>>>>>=20
>>>>>> }
>>>>>>=20
>>>>>> }
>>>>>=20
>>>>=20
>>>=20
>>=20
>=20