[ 
https://issues.apache.org/jira/browse/HIVE-17297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16149766#comment-16149766
 ] 

Zhiyuan Yang commented on HIVE-17297:
-------------------------------------

Sorry for the delay. Patch mostly looks good.
Comments: 
{code}
static final ThreadLocal<LlapTaskCommunicator> instance = new ThreadLocal<>();
{code}
1. TLS trick depends on assumption that scheduler and task communicator are 
initialized in the same thread. This is not guaranteed although unlikely to be 
broken. Need Tez support to remove this trick.

bq. The state could be inconsistent, making a deadlock possible in extreme 
cases if not handled. This will be detected by heartbeat.
2. How will heartbeat solve the problem? Periodically syncing #ducks?

3. Performance wise, there will be one rpc call for each update. Whether this 
can be a problem depends on how frequent the update will be. Could consider 
updating multiple tasks with single rpc call in future.

Nits
{code}
// See the comment in handleSinglePriorityLevelForUpdate
{code}
1. Wrong function name in comments

{code}
private int handleUpdateForSinglePriorityLevel(int updateCount, int count,
{code}
2. updateCount and count can be combined into one argument

> allow AM to use LLAP guaranteed tasks
> -------------------------------------
>
>                 Key: HIVE-17297
>                 URL: https://issues.apache.org/jira/browse/HIVE-17297
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Sergey Shelukhin
>            Assignee: Sergey Shelukhin
>         Attachments: HIVE-17297.01.nogen.patch, HIVE-17297.01.patch, 
> HIVE-17297.02.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to