[
https://issues.apache.org/jira/browse/HDFS-5119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13785558#comment-13785558
]
Chris Nauroth commented on HDFS-5119:
-------------------------------------
Hi, Andrew. This looks good. I ran the patch in a test cluster and did a
successful layout version upgrade. I ran all caching-related operations
multiple times, forced a checkpoint, and restarted the namenode to confirm that
it maintained the correct state. Nice job! Here are a few minor comments:
# {{Text#readString(DataInput)}} is now equivalent to
{{Text#readString(DataInput, int)}} passing {{Integer#MAX_VALUE}} for the
second argument. Do you want to call the 2-arg method from the 1-arg method to
cut some duplication?
# Is it time to remove the following TODO?
{code}
CacheManager(FSNamesystem namesystem, FSDirectory dir, Configuration conf) {
// TODO: support loading and storing of the CacheManager state
{code}
# Please add JavaDocs for the following {{CacheManager}} methods:
{{unprotectedAddEntry}}, {{addDirective}}, {{unprotectedAddDirective}},
{{unprotectedRemoveDescriptor}}, {{unprotectedAddCachePool}}.
# I noticed that {{FSNamesystem#setCacheReplicationInt}} logs an audit event
for "setReplication" instead of "setCacheReplication". Do you want to change
that string right now, or do you prefer if I file a new bug report?
# Regarding the following code, do we need to intercept setReplication calls on
existing files and also update cache replication to keep them in sync? Also, I
notice this is skipping directories. Will this code change when we add support
for caching a directory in HDFS-5096?
{code}
INode node = dir.getINode(entry.getPath());
if (node != null && node.isFile()) {
INodeFile file = node.asFile();
// TODO: adjustable cache replication factor
namesystem.setCacheReplicationInt(entry.getPath(),
file.getBlockReplication());
} else {
LOG.warn("Path " + entry.getPath() + " is not a file");
}
{code}
> Persist CacheManager state in the edit log
> ------------------------------------------
>
> Key: HDFS-5119
> URL: https://issues.apache.org/jira/browse/HDFS-5119
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: HDFS-4949
> Reporter: Colin Patrick McCabe
> Assignee: Andrew Wang
> Attachments: hdfs-5119-1.patch, hdfs-5119-2.patch
>
>
> CacheManager state should be persisted in the edit log. At the moment, this
> state consists of information about cache pools and cache directives. It's
> not necessary to persist any information about what is cached on the
> DataNodes at any particular moment, since this changes all the time.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
