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