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