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

Wei-Chiu Chuang commented on HDFS-10899:
----------------------------------------

Still half way through my review. Post my notes for reference:

ReencryptionStatus#listReencryptionStatus
{code}
final boolean hasMore = (numResp < tailMap.size());
{code}
This seems untrue. if the system has 1001 (tailMap.size()) encryption zones, 
and we get 1 (count) encryption zone starting from the 1000th and we expect to 
get 100 results (numResp), the hasMore should be false.
It looks like it would cause an infinite loop between client and NN because 
hasMore is _always_ true.

Javadoc for HdfsAdmin#listReencryptionStatus:
This method can only be called by HDFS superusers.

This is a little unnecessary and misleading. 
By definition, methods in HdfsAdmin are superuser only.
Also, the permission check is enforced at namenode side, not client side.

ZoneReencryptionStatus

@InterfaceAudience.Private annotation for ZoneReencryptionStatus


It seems EncryptionZoneManager#reencryptionHandler can be null if NN does not 
configure any key providers.
So every access to reencryptionHandler should be checked to avoid NPE.


EncryptionZoneManager#fixReencryptionZoneNames()
suggest a more appropriate method name — “fixReencryptionZoneNames” feels like 
a hack for a bug.


EncryptionZoneManager#cancelReencryptEncryptionZone: the Javadoc “If the given 
path If the given path is not the root of an encryption zone,” has duplicate 
words

Would ReencryptionStatus#getZoneStatus ever return null? 
The method is called by multiple callers so I can’t be sure if it is ever 
possible. But it could return null, some value checking would be necessary for 
the callers.


ReencryptionHandler#submitCurrentBatch
IntelliJ is complaining that this code
{code}
TreeMap<String, FileEdekPair> batch = (TreeMap<String, FileEdekPair>)
    
((TreeMap<String, FileEdekPair>) currentBatch).clone();
{code}
is an unchecked cast.

ReencryptionHandler#run()
Append “millisecond” at the end of log message "Starting up re-encrypt thread 
with interval={}”

A number of typos
“inode cannot be resolve to a full path” : “resolve” —> resolved


> Add functionality to re-encrypt EDEKs
> -------------------------------------
>
>                 Key: HDFS-10899
>                 URL: https://issues.apache.org/jira/browse/HDFS-10899
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>          Components: encryption, kms
>            Reporter: Xiao Chen
>            Assignee: Xiao Chen
>         Attachments: editsStored, HDFS-10899.01.patch, HDFS-10899.02.patch, 
> HDFS-10899.03.patch, HDFS-10899.04.patch, HDFS-10899.05.patch, 
> HDFS-10899.06.patch, HDFS-10899.07.patch, HDFS-10899.08.patch, 
> HDFS-10899.09.patch, HDFS-10899.10.patch, HDFS-10899.10.wip.patch, 
> HDFS-10899.11.patch, HDFS-10899.12.patch, HDFS-10899.wip.2.patch, 
> HDFS-10899.wip.patch, Re-encrypt edek design doc.pdf, Re-encrypt edek design 
> doc V2.pdf
>
>
> Currently when an encryption zone (EZ) key is rotated, it only takes effect 
> on new EDEKs. We should provide a way to re-encrypt EDEKs after the EZ key 
> rotation, for improved security.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to