[jira] [Commented] (YARN-1123) [YARN-321] Adding ContainerReport and Protobuf implementation

["Zhijie Shen (JIRA)" <ji...@apache.org>]

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

Zhijie Shen commented on YARN-1123:
-----------------------------------

The patch is almost good with some minor issues to address:
* The javadoc of ContainerReport needs to be updated
* ContainerReport class needs to be annotated as well
* "this != null" is a bug. In addition, it's better to move "ContainerReportProtoOrBuilder p = viaProto ? proto : builder;" after "if{}" block, as it may not need to be executed. Same for the following getters.
{code}
+  @Override
+  public Resource getAllocatedResource() {
+    ContainerReportProtoOrBuilder p = viaProto ? proto : builder;
+    if (this != null) {
+      return this.resource;
+    }
+    if (!p.hasResource()) {
+      return null;
+    }
+    this.resource = convertFromProtoFormat(p.getResource());
+    return this.resource;
+  }
{code}

BTW, this patch should be clean to be directly applied to trunk. it seems to be unrelated that Jenkins failed to compile. We'd better kick another round of Jenkins test.

> [YARN-321] Adding ContainerReport and Protobuf implementation
> -------------------------------------------------------------
>
>                 Key: YARN-1123
>                 URL: https://issues.apache.org/jira/browse/YARN-1123
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Zhijie Shen
>            Assignee: Mayank Bansal
>         Attachments: YARN-1123-1.patch, YARN-1123-2.patch, YARN-1123-3.patch, YARN-1123-4.patch
>
>
> Like YARN-978, we need some client-oriented class to expose the container history info. Neither Container nor RMContainer is the right one.



--
This message was sent by Atlassian JIRA
(v6.1#6144)