duongkame commented on code in PR #4547:
URL: https://github.com/apache/ozone/pull/4547#discussion_r1171706645


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/symmetric/DefaultSecretKeyVerifierClient.java:
##########
@@ -44,37 +45,45 @@ public class DefaultSecretKeyVerifierClient implements 
SecretKeyVerifierClient {
   private static final Logger LOG =
       LoggerFactory.getLogger(DefaultSecretKeyVerifierClient.class);
 
-  private final LoadingCache<UUID, ManagedSecretKey> cache;
+  private final LoadingCache<UUID, Optional<ManagedSecretKey>> cache;
 
   DefaultSecretKeyVerifierClient(SCMSecurityProtocol scmSecurityProtocol,
                                  ConfigurationSource conf) {
     Duration expiryDuration = parseExpiryDuration(conf);
     Duration rotateDuration = parseRotateDuration(conf);
-    long cacheSize = expiryDuration.toMillis() / rotateDuration.toMillis() + 1;
 
-    CacheLoader<UUID, ManagedSecretKey> loader =
-        new CacheLoader<UUID, ManagedSecretKey>() {
+    // if rotation is 1d, and each keys is valid for 7d before expiring,
+    // the expected number valid keys at any time is 7.
+    long expectedValidKeys =
+        expiryDuration.toMillis() / rotateDuration.toMillis() + 1;
+    // However, we want to cache some expired keys as well, to avoid requesting
+    // SCM for recently expire secret keys.
+    long cacheSize = expectedValidKeys * 2;

Review Comment:
   > Hi @duongkame thank you for your work on this one, I added two minor 
comments, but otherwise the patch looks good.
   > 
   > I leave it on you if you address the two things those are mostly just 
making the code nicer to me, so I am not in particular against committing the 
patch as it is.
   
   Thanks for the review @fapifta. Somehow I only saw one comment and make an 
update accordingly. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to