[
https://issues.apache.org/jira/browse/HADOOP-14705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16117245#comment-16117245
]
Rushabh S Shah commented on HADOOP-14705:
-----------------------------------------
Thanks [~xiaochen] for working on this.
Seems like a good change.
I have some review comments from the client side.
Will review server side changes later.
1. If I understand the patch correctly, this server request will be either All
or None.
I haven't seen HDFS-10899, so I might miss some context.
Either all the EDEKs within the list will be re-encrypted or if anyone of ekv
encounters Exception, then the whole call will fail.
It would be better if we don't fail if one EDEK fails to process.
We can return Map<originalEKV, re-encryptedEKV>. In case of failed operations,
we can just set null and traverse the map on the client side to see which ekv's
failed.
2. +KeyProviderCryptoExtension.java+
Instead of creating {{CryptoCodec}} and {{Encryptor}} for every ekv, we can
create it just once.
I don't think it stores some state related to each {{ekv}}.
Also we need to close {{CryptoCodec}} instance.
I would use try with resources block.
3. +KMSClientProvider.java+
In {{reencryptEncryptedKeys}} method, we need to add null check also along with
zero length array.
if (ekvs == null || ekvs.isEmpty())
4. +KMSUtil.java+
{{toJSON(EncryptedKeyVersion encryptedKeyVersion)}} and
{{toJSON(KeyProvider.KeyVersion keyVersion)}}
Why do we need to use LinkedHashMap ?
I don't think in the server side, while iterating over the map, we care about
the insertion order.
{{toJSON(EncryptedKeyVersion encryptedKeyVersion, String keyName)}}
Will the keyName will ever be null ?
We do a "not null" check in the calling method
{{KMSClientProvider#reencryptEncryptedKeys(List<EncryptedKeyVersion> ekvs)}}
{{checkNotNull}} and {{checkNotEmpty}}
Both of these methods are actually utility methods. I would rather see this in
KMSUtil instead of KMSClientProvider
> Add batched reencryptEncryptedKey interface to KMS
> --------------------------------------------------
>
> Key: HADOOP-14705
> URL: https://issues.apache.org/jira/browse/HADOOP-14705
> Project: Hadoop Common
> Issue Type: Improvement
> Components: kms
> Reporter: Xiao Chen
> Assignee: Xiao Chen
> Attachments: HADOOP-14705.01.patch, HADOOP-14705.02.patch
>
>
> HADOOP-13827 already enabled the KMS to re-encrypt a {{EncryptedKeyVersion}}.
> As the performance results of HDFS-10899 turns out, communication overhead
> with the KMS occupies the majority of the time. So this jira proposes to add
> a batched interface to re-encrypt multiple EDEKs in 1 call.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
