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

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

 FSNamesystem.java:
- logAuditEvent(false, "getXAttr", src); --> logAuditEvent(false, "getXAttrs", 
src);

- 
{code}
} else {
+        throw new IOException("No matching attributes found");
{code}
One suggestion to tell "No matching attributes found to remove"
And this condition makes me to think on retryCache. I hope it is done here let 
me see there.
Example first call may succeed internally but restarted/disconnected, in that 
case idempotent API will be retried from client. So, next call may fail as it 
was already removed. Do you think, we need to mark this as ATMostOnce?
Let me know, if you agree for it, I would be happy to help in fixing that.


TestDFSShell.java:

- From the below code, we don't need out.toString as we did not asserted 
anything.
{code}
 {
+        final int ret = ToolRunner.run(fshell, new String[] {
+            "-setfattr", "-n", "user.a1", "-v", "1234", "/foo"});
+        assertEquals("Returned should be 0", 0, ret);
+        final String str = out.toString();
+        out.reset();
+      }
{code}


- We need to shutdown the mini cluster as well.
{code}
} finally {
+      if (bakErr != null) {
+        System.setErr(bakErr);
+      }
+    }
{code}

 FSXAttrBaseTest.java:

- Please handle only specific exceptions. If it throws unexpected exception let 
it throwout, we need not assert and throw.
{code}
} catch (HadoopIllegalArgumentException e) {
+    } catch (Exception e) {
{code}


- if an an xattr --> if an xattr


- Please do handle specific exceptions.
{code}
 } catch (Exception e) {
+      GenericTestUtils.assertExceptionContains
+          ("An XAttr name must be prefixed with user/trusted/security/system, 
" +
+           "followed by a '.'",
+          e);
+    }
+
{code}

- XattrNameParam.java:

- 
{code}
private static Domain DOMAIN = new Domain(NAME,
+      Pattern.compile(".*"));
{code}
I understand that we try to eliminate the client validation as we will not have 
flexibility to add more namespaces in future. But that pattern can be same as 
<Namespace>. right.
So, how about validating pattern? Please check with Andrew as well what he 
says. But I have no strong feeling on that. It is a suggestion.




> getfattr in CLI doesn't throw exception or return non-0 return code when 
> xattr doesn't exist
> --------------------------------------------------------------------------------------------
>
>                 Key: HDFS-6422
>                 URL: https://issues.apache.org/jira/browse/HDFS-6422
>             Project: Hadoop HDFS
>          Issue Type: Bug
>    Affects Versions: 3.0.0, 2.5.0
>            Reporter: Charles Lamb
>            Assignee: Charles Lamb
>            Priority: Blocker
>         Attachments: HDFS-6422.005.patch, HDFS-6422.006.patch, 
> HDFS-6422.1.patch, HDFS-6422.2.patch, HDFS-6422.3.patch, HDFS-6474.4.patch
>
>
> If you do
> hdfs dfs -getfattr -n user.blah /foo
> and user.blah doesn't exist, the command prints
> # file: /foo
> and a 0 return code.
> It should print an exception and return a non-0 return code instead.



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

Reply via email to