Issue Details (XML | Word | Printable)

Key: CONCURRENCY_EE_SPEC-22
Type: Task Task
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: anthony.lai
Reporter: anthony.lai
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
concurrency-ee-spec

Simplify MES, MSES APIs regarding ManagedTaskListener

Created: 10/Dec/12 11:38 PM   Updated: 27/Dec/12 07:17 PM   Resolved: 27/Dec/12 07:17 PM
Component/s: None
Affects Version/s: None
Fix Version/s: Public Review

Time Tracking:
Not Specified

Issue Links:
Related
 

Tags:
Participants: anthony.lai


 Description  « Hide

Reduce the number of new APIs in ManagedExecutorService and ManagedScheduledExeuctorService. Currently we have 7 new APIs in the ManagedExecutorService
and 4 in ManagedScheduledExecutorService whose only difference from the ones in the super class in java.util.concurrent package was the addition of an extra parameter for ManagedTaskListener,



anthony.lai added a comment - 11/Dec/12 12:27 AM

From Nathan:
The proposal about ManagedTaskListener is also interesting because I remember in one of the posts (which we never really addressed) where someone asked about specifying context properties on tasks submitted to a managed executor service. That's a very similar scenario in that it would also require additional method signatures.

I gave some more thought to whether there are any other alternatives, and I think a better alternative than what I mentioned previously could be a mix-in interface that provides a getManagedTaskListener.
We already have Identifiable being used here as a mix-in interface. We could convert Identifiable into something more general, say ManagedTask, and then add a getManagedTaskListener (and possibly also a getContextProperties) to it.

So the idea here would be to replace
javax.enterprise.concurrent.Identifiable
.getIdentityName()
.getIdentityDescription(Locale)
with
javax.enterprise.concurrent.ManagedTask
.getIdentityName()
.getIdentityDescription(Locale)
.getManagedTaskListener()
.getContextProperties() ?

and then we could meet the request of removing all of the method signatures on ManagedExecutorServce/ManagedScheduledExecutorService which copy from ExecutorService/ScheduledExecutorService without introducing the unpredictable behavior that would be introduced by ManagedExecutorService.add/removeManagedTaskListener.


anthony.lai added a comment - 11/Dec/12 12:27 AM

From Anthony:
I like this idea. We could also use this mix-in interface to provide hints for distributed execution, as tracked by jira issue 14.

But this seems to tightly couple the task implementation with the ManagedTaskListener implementation. Suppose an application developer schedules a task which is provided by some library but needs to monitor the task using a ManagedTaskListener that he/she writes. Is this a valid use case and do we need to worry about such scenarios?


anthony.lai added a comment - 11/Dec/12 12:28 AM

From Nathan:
Yes, that's a valid use case. It would still be possible to register a ManagedTaskListener for a task that's provided by a library or third party – by wrapping it with a ManagedTask.
For example,

Runnable r = new MyManagedTaskProxy(runnableFrom3rdPartyLibrary, managedTaskListener); // implements Runnable and ManagedTask
managedExecutorService.submit(r);

I'm not sure how common this will be. It does make this scenario more difficult.

If we want to go this direction, a further step that could be taken to help here is to provide static convenience methods,
ManagedExecutors.managedTask(runnable/callable, ManagedTaskListener)
along the lines of what the Java SE concurrent package does for
Executors.callable(Runnable) or Executors.callable(PrivilegedAction)
so the that application doesn't need to implement its own proxy,

Runnable r = ManagedExecutors.managedTask(runnable, listener);
managedExecutorService.submit(r);

As you pointed out, the mix-in interface approach could also solve JIRA issue 14.
ManagedTask could have a method,
.getExecutionProperties()
instead of
.getContextProperties()
since context properties and distributed execution properties are both particular types of execution properties (We should also update ContextService here, so that it refers to execution properties, and is not limited to context properties).
I would recommend that we try to standardize any known execution properties as much as possible so that we don't end up encouraging non-portable apps.
In addition to whatever particular information for distributed execution was requested under issue 14, one piece of information that I think will be useful for task execution is a IS_LONG_RUNNING=true/false hint, since an implementation might want to make use of that sort of information when deciding how to allocate work to a thread pool.


anthony.lai added a comment - 12/Dec/12 09:54 PM

Proposed changes:

ManagedExecutorService:

  • Removal of all 7 APIs:
  • <T> List <Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit, ManagedTaskListener taskListener)
  • <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, ManagedTaskListener taskListener)
  • <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit, ManagedTaskListener taskListener)
  • <T> T invokeAny(Collection<? extends Callable<T>> tasks, ManagedTaskListener taskListener)
  • <T> Future<T> submit(Callable<T> task, ManagedTaskListener taskListener)
  • Future<?> submit(Runnable task, ManagedTaskListener taskListener)
  • <T> Future<T> submit(Runnable task, T result, ManagedTaskListener taskListener)
  • it is now possible to receive ManagedTaskListener events using the void execute(Runnable command) method from the base class Executor.

ManagedScheduledExecutorService

  • remove the following 4 APIs:
  • <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUnit unit, ManagedTaskListener taskListener)
  • ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit, ManagedTaskListener taskListener)
  • ScheduledFuture<?> scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit, ManagedTaskListener taskListener)
  • ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit, ManagedTaskListener taskListener)
  • APIs that use Trigger would have the taskListener argument removed, and become:
  • <V> ScheduledFuture<V> schedule(Callable<V> callable, Trigger trigger)
  • ScheduledFuture<?> schedule(Runnable command, Trigger trigger)

ManagedTask

  • optionally implemented by tasks (Runnable or Callable) submitted to MES and MSES
  • replaces Identifiable
  • contains these methods:
  • String getIdentityDescription(Locale locale)
  • String getIdentityName()
  • ManagedTaskListener getManagedTaskListener()
  • Properties getExecutionProperties()
  • Predefined constants for execution property names and values include
  • DISTRIBUTABLE: LOCAL (default), DISTRIBUTABLE, DISRTIBUTABLE_WITH_AFFINITY
  • LONGRUNNING_HINT: TRUE, FALSE (default)

One other possible property we may add here is for contextual callback, eg.

  • CONTEXTUAL_CALLBACK: TRUE, FALSE (default)

ManagedExecutors

  • new utility methods to add ManagedTaskListener to Runnable and Callable
  • static Runnable managedTask(Runnable task, ManagedTaskListener taskListener)
  • static <T> Callable<T> managedTask(Callable<T> callable, ManagedTaskListener taskListener)

anthony.lai added a comment - 14/Dec/12 11:17 PM

2 more methods for ManagedExecutors

  • static Runnable managedTask(Runnable task, Properties executionProperties, ManagedTaskListener taskListener)
  • static <T> Callable<T> managedTask(Callable<T> callable, Properties executionProperties, ManagedTaskListener taskListener)

anthony.lai added a comment - 27/Dec/12 07:17 PM

Proposed changes made.

Minor changes to execution property names:
DISTRIBUTABLE_HINT - true or false
CONTEXTUAL_CALLBACK_HINT - true or false

ContextService.createContextObject renamed to createContextualProxy