xiaoyuyao commented on a change in pull request #2259:
URL: https://github.com/apache/hadoop/pull/2259#discussion_r489599636



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
##########
@@ -299,19 +299,18 @@ public E getNext(String keyName)
    * @param keyName the key to drain the Queue for
    */
   public void drain(String keyName) {
+    Runnable e;
+    while ((e = queue.deleteByName(keyName)) != null) {
+      executor.remove(e);
+    }
+    writeLock(keyName);
     try {
-      Runnable e;
-      while ((e = queue.deleteByName(keyName)) != null) {
-        executor.remove(e);
-      }
-      writeLock(keyName);
-      try {
-        keyQueues.get(keyName).clear();
-      } finally {
-        writeUnlock(keyName);
+      LinkedBlockingQueue kq = keyQueues.getIfPresent(keyName);
+      if (kq != null) {
+        kq.clear();
       }
-    } catch (ExecutionException ex) {

Review comment:
       We don't have compatibility issue here. LoadingCache#get() may throw 
ExecutionException. But this ticket has change to use 
LoadingCache#getIfPresent() which does not throw ExecutionException. This way, 
we don't need to worry about either NPE from LoadingCache.get().clear() or 
ExecutionException.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to