[
https://issues.apache.org/jira/browse/HDFS-10899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15803850#comment-15803850
]
Xiao Chen commented on HDFS-10899:
----------------------------------
[~dilaver] had a pretty good offline review of patch 2. Attaching #3 to address
the following comments:
- {{ReencryptionZonesStatus.reencryptRequests}} declared type should probably
be {{List}} instead of {{Collection}} considering "it should preserve the
order".
- Should {{ReencryptionZonesStatus}} set lastFileProcessed to null upon
{{removeZone}} when the removed zone is the current zone? Instead of the
callers invoking both removeZone() and setLastFileProcessed() in tandem?
- Rename {{flipPauseForTesting}} to separate methods ({{pauseForTesting}} and
{{resumeFromTestPause}})?
- {{Make ReencryptionHandler#pauseForTesting()}} synchronized instead of
synchronized block in the method? That way the second log statement will be
guaranteed to be executed together with the preceding statements.
- Add a max retry and terminate re-encrypt thread if keyprovider is still null.
- misplaced log statement "{}({}) is a nested EZ, skipping for re-encrypt"
- {{INodeDirectory.nextChild()}} will return 0 if {{startAfter}} has length 0
so there doesn't seem to be a need for the {{if}}.
- While re-encryption is single threaded (at least for now), could it be more
appropriate to create a ThreadFactory for a given instance of
EncrytpionZoneManager instead of creating a new one for every invocation of
{{EncryptionZoneManager#startReencryptThread()}}, especially considering the
logged names of threads will overlap (if/when there are multiple threads)?
- EncryptionZoneManager#removeEncryptionZone() make the logging clear and
unconditional
- Missing documentation for {{EncryptionZoneManager#reencryptEncryptionZone,
#cancelReencryptEncryptionZone, #isEncryptionZoneRoot,
#getIdRootEncryptionZone}}.
- In {{EncryptionZoneManager#reencryptEncryptionZone()}}, unnecessary break in
String constant (in throw).
- {{EncryptionZoneManager#loadReencryptStatus()}}: why no null check for
{{zoneId}} in the {{else}}? Note that {{LinkedHashSet}} doc says it allows null
elements. Move the {{zoneId}} null check before the {{if}}?
- In FSNamesystem, use {{this.dir.ezManager}} instead of {{dir.ezManager}} to
match the surrounding style for {{setProvider}}?
More coming...
> 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: HDFS-10899.01.patch, HDFS-10899.02.patch,
> HDFS-10899.wip.2.patch, HDFS-10899.wip.patch, Re-encrypt edek design doc.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.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
