[
https://issues.apache.org/jira/browse/TEZ-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13967305#comment-13967305
]
Siddharth Seth commented on TEZ-708:
------------------------------------
Apologies, took a bit of time to look at this.
{code} if (shouldWait()) {code} - this, I believe, needs to be inside the
synchronized block. Otherwise the following sequence is possible. shouldWait
returns true, deallocateTask notifies, run thread goes to wait.
It'd be good to log exception cases as well as normal cases to help with
debugging - if (container != null) { in deallocateTask, Interrupts, allocation
/ de-allocation
getAvailableResources - potential option is to make use of (local mode
parallelism - running tasks). Can be done later, after local mode is functional
- and we have a better idea of interactions between the allocator and the
launcher.
Even though most requests will come in on a single thread, I think it's useful
to make LocalTaskScheduler thread safe.
AsyncDelegateRequestHandler.run - handle exceptions - which would otherwise
cause the thread to silently fail (an NPE for example).
Minor:
- @override annotations in TaskScheduler
- init / start / stop - Don't think these should be part of the interface,
since they come from abstract service. TSEH could just cast to abstractservice
instead to call these.
- createAppCallbackDelegate, createAppCallbackExecutorService etc - can these
be private or have appropriate annotations (@visiblefortesting)
- nextId - atomic integer ? (primarily from a thread safety perspective)
- appCallbackExecutor, taskAllocations can be final
- null checks in serviceStop (can be invoked after construction before init /
start)
- In stop, appCallbackExecutor.shutdownNow or shutDown - do pending tasks need
to be executed ?
- TaskRequest can implement Comparable<TaskRequest> instead of Comparable -
gets rid of a compiler warning
- Priority priority = Priority.newInstance(1); - could be a single instance for
the entire class. Priority value of 0 instead of 1, or even negative
Otherwise, I think this is good to go. Getting the entire local mode
functioning would be really useful.
> Avoid asking for new containers in uber-local mode
> --------------------------------------------------
>
> Key: TEZ-708
> URL: https://issues.apache.org/jira/browse/TEZ-708
> Project: Apache Tez
> Issue Type: Sub-task
> Reporter: Chen He
> Assignee: Jonathan Eagles
> Attachments: TEZ-708.patch, TEZ-708.patch, TEZ-708.patch,
> TEZ-708.patch
>
>
> Once we finish this task, the TaskSchedulerEventHandler should not ask for
> new container in the Uber-mode.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
