[jira] [Commented] (HDFS-6492) Support create-time xattrs and atomically setting multiple xattrs

[Yi Liu (JIRA)]

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

Yi Liu commented on HDFS-6492:
------------------------------

Thanks [~andrew.wang], nice work. I just have two comments for small 
improvements.

*1.*
{code}
List<XAttr> setINodeXAttrs(List<XAttr> existingXAttrs, List<XAttr> toSet,
      EnumSet<XAttrSetFlag> flag) throws IOException {
    // Check for duplicate XAttrs in toSet
    // We need to use a custom comparator, so using a HashSet is not suitable
    for (int i=0; i<toSet.size(); i++) {
      for (int j=0; j<toSet.size(); j++) {
        if (i==j) {
          continue;
        }
        if (toSet.get(i).equalsIgnoreValue(toSet.get(j))) {
          throw new IOException("Cannot specify the same XAttr to be set " +
              "more than once");
        }
      }
    }
......
{code}

The two {{for}} can be improved as following. Since we don’t need to compare 
the elements < i two times.
{code}
  for(int i = 0; i < toSet.size(); i++) {
    for(int j = i+1; j < toSet.size(); j++)  {
      if (toSet.get(i).equalsIgnoreValue(toSet.get(j))) {
          throw new IOException("Cannot specify the same XAttr to be set " +
              "more than once");
      }
    }
  }
{code}


*2.*
In {{FSDirectory#setINodeXAttrs}},  if change like following could be a bit 
more efficient?  (Can save one iteration)
*1).*
{code}
    if (existingXAttrs != null) {
      for (XAttr a: existingXAttrs) {
        if (isUserVisible(a)) {
          userVisibleXAttrsNum++;
        }
      }
    }
{code}
Remove this snippet code.

*2).*
{code}
      XAttrSetFlag.validate(xAttr.getName(), exist, flag);
      // add the new XAttr since it passed validation
      xAttrs.add(xAttr);
      if (isUserVisible(xAttr) && !exist) {
        userVisibleXAttrsNum++;
      }
{code}

Change it to :
{code}
      XAttrSetFlag.validate(xAttr.getName(), exist, flag);
      // add the new XAttr since it passed validation
      xAttrs.add(xAttr);
      if (isUserVisible(xAttr)) {
        userVisibleXAttrsNum++;
      }
{code}

*3).*
{code}
       if (!alreadySet) {
          xAttrs.add(existing);
        }
{code}

Change it to:
{code}
       if (!alreadySet) {
          xAttrs.add(existing);
          
          if (isUserVisible(existing)) {
            userVisibleXAttrsNum++;
          }
        }
{code}


> Support create-time xattrs and atomically setting multiple xattrs
> -----------------------------------------------------------------
>
>                 Key: HDFS-6492
>                 URL: https://issues.apache.org/jira/browse/HDFS-6492
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: namenode
>    Affects Versions: 2.4.0
>            Reporter: Andrew Wang
>            Assignee: Andrew Wang
>         Attachments: HDFS-6492.001.patch
>
>
> Ongoing work in HDFS-6134 requires being able to set system namespace 
> extended attributes at create and mkdir time, as well as being able to 
> atomically set multiple xattrs at once. There's currently no need to expose 
> this functionality in the client API, so let's not unless we have to.



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