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

Xiao Chen commented on HDFS-11210:
----------------------------------

Thanks a lot for the review Andrew! Exactly the 'quality review' I mentioned in 
the last comment. :) 

bq. locks
Used a fixed-sized preallocated array for the lock.
Researched a bit but think we should be able to live with the fixed-sized locks 
in this case. Resizing an array is easy, but seems a little messy to do for the 
array of stripped locks without a synchronized block on the {{ValueQueue}} 
object. (IIUC we need to writelock all stripes before switching to a new-sized 
array, to prevent the same key that's locked at index 1 unprotected due to its 
new index 2).
If this looks good to you, I can make the size configurable (and potentially 
modify {{ValueQueue}} to use a builder instead of adding the 7th param to ctor)

bq. Do you know why UniqueKeyBlockingQueue#put is synchronized, but the other 
methods aren't?
Good q, I'm not sure. Looking at the code + comments, the only explanation IMHO 
is we want to make sure no more than 1 runnable per key is created. And the 
only thing the async thread do is {{put}}, so kinda make sense... We could ping 
the authors for the answer.

bq. it looks like we'll invalidate multiple times since there's an 
invalidateCache call in both KMSCP and LBKMSCP. Do you think we can remove the 
invalidates in LBKMSCP?
I don't think it'd be safe to remove it from LBKMSCP, because {{ValueQueue}} is 
per KMSCP. So 1 {{KMSCP#rollNewVersion}} call should really invalidates all 
KMSCPs' caches for a LBKMSCP group.
We may be able to only invalidate the rest of the KMSCPs in the LBKMSCP group, 
so each KMSCP only invalidates once. But I don't think this double invalidation 
will practically create problems, so would like to go for the current KISS way 
for code cleanness. Thoughts?

Other comments are all addressed.

> Enhance key rolling to guarantee new KeyVersion is returned from 
> generateEncryptedKeys after a key is rolled
> ------------------------------------------------------------------------------------------------------------
>
>                 Key: HDFS-11210
>                 URL: https://issues.apache.org/jira/browse/HDFS-11210
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: encryption, kms
>    Affects Versions: 2.6.5
>            Reporter: Xiao Chen
>            Assignee: Xiao Chen
>         Attachments: HDFS-11210.01.patch, HDFS-11210.02.patch, 
> HDFS-11210.03.patch
>
>
> To support re-encrypting EDEK, we need to make sure after a key is rolled, no 
> old version EDEKs are used anymore. This includes various caches when 
> generating EDEK.
> This is not true currently, simply because no such requirements / necessities 
> before.
> This includes
> - Client Provider(s), and corresponding cache(s).
> When LoadBalancingKMSCP is used, we need to clear all KMSCPs.
> - KMS server instance(s), and corresponding cache(s)
> When KMS HA is configured with multiple KMS instances, only 1 will receive 
> the {{rollNewVersion}} request, we need to make sure other instances are 
> rolled too.
> - The Client instance inside NN(s), and corresponding cache(s)
> When {{hadoop key roll}} is succeeded, the client provider inside NN should 
> be drained too.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to