hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yi Liu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-6492) Support create-time xattrs and atomically setting multiple xattrs
Date Mon, 16 Jun 2014 09:33:02 GMT

    [ 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)

Mime
View raw message