[
https://issues.apache.org/jira/browse/HDFS-10899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105525#comment-16105525
]
Wei-Chiu Chuang commented on HDFS-10899:
----------------------------------------
Hi [~xiaochen] nice to see real perf number!
One high level question: did you notice any significant NameNode pause during
re-encryption other than GC pauses?
Here's my code review note. I'm still half way through my review.
* The class ZoneStatus in ReencryptionStatus should be refactored into its own
class file, because it is used everywhere and itself is around half of
ReencryptionStatus. Considering future expansion, this class would be
refactored in the future anyway.
* ReencryptionHandler#cancelZone() should throw exception if ZoneStatus is not
found. Otherwise the caller can't tell if zone is actually being cancelled.
* I feel that the behavior of "cancel" should be documented somewhere, that is,
it does not interrupt existing re-encrypt operation and does not wait for the
ongoing re-encrypt operation to stop.
* EncryptionZoneManager#listReencryptionStatus and
ReencryptionStatus#listReencryptionStatus instantiates an empty arraylist. We
can use a flyweight pattern (pre-allocate an empty BatchedListEntries/array
list) to reduce new object allocation. This is rather minor issue though.
* ReencryptHandler#startThreads/stopThreads are a little confusing to me
initially, because ReencryptHandler itself is a Runnable, but
startThreads/stopThreads actually are about ReencryptionUpdater threads.
* Suggest that ReencryptHandler#run() and ReencryptUpdater#run() add extra logs
at the end like
{code}
LOG.info("ReencryptHandler thread exiting.");
{code}
* Also in ReencryptHandler#run() :
{code}
catch (Exception e) {
LOG.error("Exception caught when re-encrypting zone {}.", zoneId, e);
throw e;
}
{code}
The {{throw e}} does not make much sense, since the code runs in a thread and
{{run()}} does not declare Exception exception. (java compiler does not
complain about it, though)
Suggest the following instead:
{code}
catch (Throwable t) {
LOG.error("Exiting when re-encrypting zone {}", zoneId, t);
return;
}
{code}
> 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.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
