[
https://issues.apache.org/jira/browse/HIVE-16926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16082995#comment-16082995
]
Siddharth Seth commented on HIVE-16926:
---------------------------------------
Functionally, looks good to me. Minor comments.
- umbilicalServer.umbilicalProtocol.pendingClients.putIfAbsent -> Would be a
little cleaner to add a method for this, similar to unregisterClient.
- {code} + for (String key : umbilicalImpl.pendingClients.keySet()) {
+ LlapTaskUmbilicalExternalClient client =
umbilicalImpl.pendingClients.get(key);{code}
Replace with an iterator over the entrySet to avoid the get() ?
Also, this pattern is repeated in hearbeat and nodeHeartbeat - could likely be
a method.
If I'm not mistaken, the shared umbilical server will not be shut down ever?
Maybe in a follow up - some of the static classes could be split out.
> LlapTaskUmbilicalExternalClient should not start new umbilical server for
> every fragment request
> ------------------------------------------------------------------------------------------------
>
> Key: HIVE-16926
> URL: https://issues.apache.org/jira/browse/HIVE-16926
> Project: Hive
> Issue Type: Sub-task
> Components: llap
> Reporter: Jason Dere
> Assignee: Jason Dere
> Attachments: HIVE-16926.1.patch, HIVE-16926.2.patch,
> HIVE-16926.3.patch, HIVE-16926.4.patch
>
>
> Followup task from [~sseth] and [~sershe] after HIVE-16777.
> LlapTaskUmbilicalExternalClient currently creates a new umbilical server for
> every fragment request, but this is not necessary and the umbilical can be
> shared.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)