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

Zhijie Shen commented on YARN-1917:
-----------------------------------

[~wangda], the patch looks almost good to me. Just some minor comments:

1.  The vars in bracket can be linked to the exact enums.
{code}
+   * {terminateStates} OR stopped state {completed, failed, killed}
{code}
{code}
+   *          wait for stopped state - {completed, failed, killed}
{code}

2. Call it FINAL_APPLICATION_STATES?
{code}
+  private static final Set<YarnApplicationState> STOPPED_APPLICATION_STATES = 
EnumSet
{code}

3. An application is going to stay at RUNNING for a long time, during which 
there will be no log at all. Is it better to report the progress as well at 
regular interval?
{code}
+        } else if (lastState != state) {
+          LOG.info(String.format(
+              "Application %s's state transfered from [%s] to [%s]",
+              appId.toString(), lastState.name(), state.name()));
+          lastState = state;
+        }
{code}

4. succeed -> succeeded/successful
{code}
+          // if it's not succeed, print diagnostic message
{code}

5. Merge to one?
{code}
+              LOG.info("Diagnostic message:");
+              LOG.info(diagMsg);
{code}

6. Test Failed/Killed with verbose == true as well?

In addition, how about letting this ticket open for a few days to see how 
community think about the new method? In particular, do we really need to wait 
for any state? If a user make use of YarnClient to submit an application, the 
application is already at SUBMITTED when submission call is done. Then, except 
the three final states, users have two other states can wait for, i.e., 
ACCEPTED and RUNNING.

> Add "waitForApplicationState" interface to YarnClient
> -----------------------------------------------------
>
>                 Key: YARN-1917
>                 URL: https://issues.apache.org/jira/browse/YARN-1917
>             Project: Hadoop YARN
>          Issue Type: New Feature
>          Components: client
>    Affects Versions: 2.4.0
>            Reporter: Wangda Tan
>            Assignee: Wangda Tan
>         Attachments: YARN-1917.patch, YARN-1917.patch
>
>
> Currently, YARN dosen't have this method. Users needs to write 
> implementations like UnmanagedAMLauncher.monitorApplication or 
> mapreduce.Job.monitorAndPrintJob on their own. This feature should be helpful 
> to end users.



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

Reply via email to