Junping Du commented on YARN-1402:

Thanks [~xgong] for the patch! I have a few comments after going through the 
public abstract String getLogAggregationStatus();
I think we should reuse enum of LogAggregationStatus in YARN-1376 with moving 
it (only enum, not others) from server-api to api. In such similar cases, we 
always prefer an enum over String for limited statuses.

For calculating application LA status over each node's LA status: 
+      if (logNotStartCount == reports.size()) {
+        return LogAggregationStatus.NOT_START.toString();
+      } else if (logCompletedCount == reports.size()) {
+        if (logFailedCount > 0) {
+          return LogAggregationStatus.FAILED.toString();
+        } else if (logTimeOutCount > 0) {
+          return LogAggregationStatus.TIME_OUT.toString();
+        }
+        return LogAggregationStatus.FINISHED.toString();
+      } else {
+        return LogAggregationStatus.RUNNING.toString();
+      }
+    }
A few comments here: 
1. if only one container (other containers haven't report for some reasons, 
e.g. pending on scheduled by RM) has a report to RM which completed, we 
shouldn't mark log aggregation as completed for this app. We should check app's 
status also?
2. Shall we return app's LA status still as running even some node LA (log 
aggregation) report already get failed. I don't think app's LA status can 
finish successfully later in this case. So may be return failed earlier could 
be a good choice?
3. I think a better way to generate app's log aggregation status is not when do 
get() but when each node's LA report do update (in method of 
aggregateLogReport()). What do you think?

Other looks fine to me.

> Related Web UI, CLI changes on exposing client API to check log aggregation 
> status
> ----------------------------------------------------------------------------------
>                 Key: YARN-1402
>                 URL: https://issues.apache.org/jira/browse/YARN-1402
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: YARN-1402.1.patch, YARN-1402.2.patch

This message was sent by Atlassian JIRA

Reply via email to