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

Xiao Chen commented on HADOOP-14705:
------------------------------------

Thanks a lot for the prompt review [~shahrs87], and apologies for the 
difficulty in applying the patch.

All comments addressed in patch 7 with exception and explanations below.

bq. Do we need to increment AdminCallsMeter ?
I think it's the other way. These meters are exposed to the jmx, where you 
would see things like:
{noformat}
    "name" : "metrics:name=hadoop.kms.unauthorized.calls.meter",
    "name" : "metrics:name=hadoop.kms.invalid.calls.meter",
    "name" : "metrics:name=hadoop.kms.decrypt_eek.calls.meter",
    "name" : "metrics:name=hadoop.kms.admin.calls.meter",
    "name" : "metrics:name=hadoop.kms.generate_eek.calls.meter",
    "name" : "metrics:name=hadoop.kms.key.calls.meter",
    "name" : "metrics:name=hadoop.kms.unauthenticated.calls.meter",
{noformat}
>From my understanding, the goal for these is purely for maintenance and 
>statistics. Since key level operations are rare, they're aggregated to the 
>same meters - either admin.calls or key.calls.
For eek calls (generate/decrypt), they both have their own meters. reencrypt 
fits into this category.

bq. We can remove the try catch block around user.doAs context and let the 
outer try catch block handle the exception propagating from the doAs call.
Problem with that is, the precondition checks also become IOEs. 
I think the current way is more consistent with other methods in KMS and 
creates least surprise.
The outer wrapper is only to make it possible to log a debug message in the KMS 
if things go wrong, the inner exception seems to consider provider-thrown 
exceptions more serious and log an error.... I don't fully get the history for 
this, so didn't change.

bq. ... in the test ... I think we are comparing apples to oranges.
Agreed. Looking at {{testGenerateEncryptedKey}} this case is also there, and I 
think it doesn't hurt to make sure they're different fruits. :) Also added your 
suggestion #5 which does the more important comparison of apples to apples.

> 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-14705.03.patch, HADOOP-14705.04.patch, HADOOP-14705.05.patch, 
> HADOOP-14705.06.patch, HADOOP-14705.07.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]

Reply via email to