yzhang559 commented on code in PR #6774:
URL: https://github.com/apache/hadoop/pull/6774#discussion_r1614787242


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java:
##########
@@ -269,12 +269,23 @@ public ValueQueue(final int numValues, final float 
lowWaterMark, long expiry,
    * Initializes the Value Queues for the provided keys by calling the
    * fill Method with "numInitValues" values
    * @param keyNames Array of key Names
-   * @throws ExecutionException executionException.
+   * @throws IOException if no successful initialization for any key
    */
-  public void initializeQueuesForKeys(String... keyNames)
-      throws ExecutionException {
+  public void initializeQueuesForKeys(String... keyNames) throws IOException {
+    int successfulInitializations = 0;
+    ExecutionException lastException = null;
+
     for (String keyName : keyNames) {
-      keyQueues.get(keyName);
+      try {
+        keyQueues.get(keyName);
+        successfulInitializations++;
+      } catch (ExecutionException e) {
+        lastException = e;
+      }
+    }
+
+    if (keyNames.length > 0 && successfulInitializations == 0) {
+      throw new IOException("Failed to initialize any queue for the provided 
keys.", lastException);

Review Comment:
   So this is used to indicate all key initialization failed and should be 
retried. 
   Since the return value is void of this func and it's caller funcs, keeping 
the IOException. Otherwise, it's hard to update the `success` flag in 
[.../hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java](https://github.com/apache/hadoop/pull/6774/files/10d763a5ff514541ef1eea11d70bf5173374a5d1#diff-092663652ffe33b10e51bfa062724d70e6334a9a14e64db35c9854805e09da14)



##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java:
##########
@@ -601,18 +604,22 @@ public void run() {
           NameNode.LOG.error("Cannot warm up EDEKs.", e);
           throw e;
         }
-        try {
-          Thread.sleep(retryInterval);
-        } catch (InterruptedException ie) {
-          NameNode.LOG.info("EDEKCacheLoader interrupted during retry.");
-          break;
+
+        if (!success) {
+          try {
+            Thread.sleep(retryInterval);
+          } catch (InterruptedException ie) {
+            NameNode.LOG.info("EDEKCacheLoader interrupted during retry.");
+            break;
+          }
+          retryCount++;
         }
-        sinceLastLog += retryInterval;

Review Comment:
   removed. 



-- 
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