[
https://issues.apache.org/jira/browse/HDFS-9405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15198113#comment-15198113
]
Andrew Wang commented on HDFS-9405:
-----------------------------------
A couple more review comments, otherwise looks good:
Could we use log templating for these logs?
{noformat}
NameNode.LOG.info("Warming up " + edeks.length + " EDEKs...");
...
.info("Successfully warmed up " + keyNames.length + " EDEKs.");
{noformat}
Don't need the if guard here for slf4j:
{noformat}
if (NameNode.LOG.isDebugEnabled()) {
NameNode.LOG.debug("Failed to warm up EDEKs.", ioe);
}
{noformat}
Do we need this check? The sleep right before it will throw
InterruptedException and clear the interrupted status.
{noformat}
if(Thread.currentThread().isInterrupted()) {
NameNode.LOG.info("EDEKCacheLoader thread interrupted.");
stop();
}
{noformat}
* Now that we're using the ExecutorService, I don't think we need the
{{stop()}} method and {{shouldRun}}, can just {{break}} instead?
* Can we name the thread EDEKCacheLoader so it's the same as the class?
* It looks like we're trying to ride out KMS downtime, so is there a downside
to retrying indefinitely? Another alternative is to make this configurable. If
we do indefinite retries, we'd also still want a periodic IOE print for
debugging.
> When starting a file, NameNode should generate EDEK in a separate thread
> ------------------------------------------------------------------------
>
> Key: HDFS-9405
> URL: https://issues.apache.org/jira/browse/HDFS-9405
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: encryption, namenode
> Affects Versions: 2.7.1
> Reporter: Zhe Zhang
> Assignee: Xiao Chen
> Attachments: HDFS-9405.01.patch, HDFS-9405.02.patch,
> HDFS-9405.03.patch, HDFS-9405.04.patch, HDFS-9405.05.patch,
> HDFS-9405.06.patch, HDFS-9405.07.patch, HDFS-9405.08.patch
>
>
> {{generateEncryptedDataEncryptionKey}} involves a non-trivial I/O operation
> to the key provider, which could be slow or cause timeout. It should be done
> as a separate thread so as to return a proper error message to the RPC caller.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
