[ 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)