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

Wangda Tan commented on YARN-9075:
----------------------------------

Thanks [~billie.rinaldi], 

The overall code flow looks good to me.

Some comments for implementations: 

1) 
org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices#loadManifest,

First I suggest to make all methods which will change internal data structure 
to synchronized method. Unless you feel there will be some performance issues, 
I would prefer to make it simpler. 

And inside the method, I found service could be started before it added to 
serviceMap. Is it possible that service could "leak" when some issues happen 
for other services (like throw RuntimeException AuxServices#initAuxService). 

To make it simpler, I suggest to change the code flow to: 
{code:java}
foreach (service : configured-service):
  if (serviceMap.contains(newService)):
    stop old service
  Service newService = load-and-start-service 
  serviceMap.add(newService){code}
This can avoid leaking services. 

And it seems service will be stopped twice in existing logic, inside 
initAuxService and after all service initializations: 
{code:java}
if (!loadedAuxServices.contains(entry.getKey())) {
  foundChanges = true;
  stopAuxService(entry.getValue());
  it.remove();
}{code}
2) Why change ShuffleHandler? 

3) Could u provide an example file for shuffle service manifest json file? We 
need to file a separate Jira for documentation changes.

> Dynamically add or remove auxiliary services
> --------------------------------------------
>
>                 Key: YARN-9075
>                 URL: https://issues.apache.org/jira/browse/YARN-9075
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: nodemanager
>            Reporter: Billie Rinaldi
>            Assignee: Billie Rinaldi
>            Priority: Major
>         Attachments: YARN-9075.001.patch, YARN-9075.002.patch, 
> YARN-9075.003.patch, YARN-9075_Dynamic_Aux_Services_V1.pdf
>
>
> It would be useful to support adding, removing, or updating auxiliary 
> services without requiring a restart of NMs.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to