[ 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