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

Zhijie Shen commented on YARN-1702:
-----------------------------------

[~vvasudev], thanks for the big patch! I've looked through it, and bellow are 
some high level comments:

1. There're lot of formatting changes in TestRMWebServicesApps, which seem no 
to be necessary, and affect review.

2. "getAppState" seem not to be necessary, as we have "getApp", which returns a 
full report including the state.

3. I'm not sure it is good idea to have a "updateAppState" API, but only allow 
to change the state to KILLED. Why not having "killApp" directly, and accepting 
an appId?
{code}
+  @PUT
+  @Path("/apps/{appid}/state")
+  @Produces({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML })
+  @Consumes({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML })
+  public Response updateAppState(AppState targetState,
+      @Context HttpServletRequest hsr, @PathParam("appid") String appId)
+      throws AuthorizationException, YarnException, InterruptedException,
+      IOException {
{code}

4. We should make killApp work in insecure mode as well, as we can do it via 
PRC.

5. In YarnClientImpl, we have implemented the logic to keep sending the kill 
request until we get confirmed that the app is killed. IMHO, as the user of 
REST API should be a thin client, we may want to implement this logic at the 
server side, blocking the response until we confirm that the app is killed. In 
RPC we have limited the number of concurrent threads. However, at the web side, 
we don't have this limitation, right?

6. As to the authentication filter, I think it's not just the problem of 
killApp, the whole RM web is unprotected, but we can handle this issue 
separately. Some lessons from implementing security for the timeline server:

a) It's better to have separate configs for RM only, and load the 
authentication filter for RM daemon only instead of all.
b) RM may also want Kerberos + DT authentication style.

> Expose kill app functionality as part of RM web services
> --------------------------------------------------------
>
>                 Key: YARN-1702
>                 URL: https://issues.apache.org/jira/browse/YARN-1702
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Varun Vasudev
>            Assignee: Varun Vasudev
>         Attachments: apache-yarn-1702.10.patch, apache-yarn-1702.2.patch, 
> apache-yarn-1702.3.patch, apache-yarn-1702.4.patch, apache-yarn-1702.5.patch, 
> apache-yarn-1702.7.patch, apache-yarn-1702.8.patch, apache-yarn-1702.9.patch
>
>
> Expose functionality to kill an app via the ResourceManager web services API.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to