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

Uma Maheswara Rao G commented on HDFS-6302:
-------------------------------------------

Thanks a lot Yi, for the patch.

Below are my initial comments on the patch.

{code}
if (f1 != null) {
+      throw new IllegalStateException("Duplicated XAttrsFeature");
+    }
{code}
How about having the preconditions check like Preconditions.checkState?


{code}
+/**
+ * Feature for extends attributes.
+ */
{code}
Is this "Feature for extended attributes." ?

I agree that more higher level tests will come as part of other patches. But 
having some test with unit level is needed. How about adding some test for 
change like below?
  {code}
  @Test
  public void testXattrFeature() {
    replication = 3;
    preferredBlockSize = 128 * 1024 * 1024;
    INodeFile inf = createINodeFile(replication, preferredBlockSize);
    List<XAttr> list = new ArrayList<XAttr>();
    list.add(new XAttr.Builder().setName("testxattrname")
        .setValue(new byte[] { 1, 2, 3 }).setNameSpace(NameSpace.USER).build());
    ImmutableList<XAttr> ls= ImmutableList.copyOf(list);
    XAttrFeature f = new XAttrFeature(ls);
    inf.addXAttrsFeature(f);
    XAttrFeature xAttrsFeature = inf.getXAttrsFeature();
    assertEquals("testxattrname", xAttrsFeature.getXAttrs().get(0).getName());
  }
  }
  {code}

> Implement XAttr as a INode feature.
> -----------------------------------
>
>                 Key: HDFS-6302
>                 URL: https://issues.apache.org/jira/browse/HDFS-6302
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode
>    Affects Versions: HDFS XAttrs (HDFS-2006)
>            Reporter: Yi Liu
>            Assignee: Yi Liu
>             Fix For: HDFS XAttrs (HDFS-2006)
>
>         Attachments: HDFS-6302.patch
>
>
> XAttr is based on INode feature(HDFS-5284).
> Persisting XAttrs in fsimage and edit log is handled by HDFS-6301.



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

Reply via email to