hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Charles Lamb (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-7529) Consolidate encryption zone related implementation into a single class
Date Wed, 17 Dec 2014 20:43:13 GMT

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

Charles Lamb commented on HDFS-7529:
------------------------------------

Hi [~wheat9],

Thanks for looking into this. I have a few comments and then a bunch of formatting nits that
are introduced as part of this patch.

FSDirEncryptionZoneOp.java:

In #ensureKeysAreInitialized, why do you return if provider, keyName, or metadata or null?
The existing code would throw an exception, which the new code eventually does, but not before
it has waited around to take the writeLock(). Wouldn't it be better to fail fast in this case?
Did you copy the wrong code to #ensureKeysAreInitialized?

Likewise, I think that the checks for nullness of provider, keyName, and metadata can be removed
from #createEncryptionZoneInt, right?

These two lines:

{code}
+    final byte[][] pathComponents =
+      FSDirectory.getPathComponentsForReservedPath(src);
+    FSPermissionChecker pc = fsn.getPermissionChecker();
{code}

are now inside the FSN#writeLock(). I suppose that's not the end of the world, but every little
bit of extra code inside the writeLock() hurts. Same issue with the call to #logAuditEvent
(for the failure case only) being inside the writeLock() now. IWBNI that call could be moved
out of the scope of the lock.

The same general comment for #getEZForPath. auditStat can be made final in that method.

Formatting issues:

You introduced a newline at the end of #createEncryptionZone.

#getFileEncryptionInfo. The formatting for the call to getEZForPath is weird. Bump the 'iip);'
up a line. In that same method, the call to unprotectedGetXAttrByName busts the 80 char limit.
I realize that this was already in the codebase before this patch, but it was introduced in
the previous Jira (the one which introduced FSDirXAttrOp) so we might as well fix it now for
cleanliness purposes.

In #createEncryptionZoneInt, the block comment did not get re-indented -2 when you moved it
so it's out of alignment now.

FSNamesystem.java:

Call to FSDirEncryptionZoneOp.getFileEncryptionInfo could use some formatting. It exceeds
the 80 char limit. Ditto the call to #generateEncryptedDataEncryptionKey.

FSDirStatAndListingOp.java:

Lines 204 and 423 exceed the 80 char limit.


> Consolidate encryption zone related implementation into a single class
> ----------------------------------------------------------------------
>
>                 Key: HDFS-7529
>                 URL: https://issues.apache.org/jira/browse/HDFS-7529
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Haohui Mai
>            Assignee: Haohui Mai
>         Attachments: HDFS-7529.000.patch
>
>
> This jira proposes to consolidate encryption zone related implementation to a single
class.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message