[
https://issues.apache.org/jira/browse/HDFS-7811?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14366129#comment-14366129
]
Jing Zhao commented on HDFS-7811:
---------------------------------
Thanks for working on this, Xiaoyu. Some comments below:
# In {{tryGetBlockStoragePolicyId}}, looks like {{getStoragePolicyID}} will not
throw {{UnsupportedOperationException}} in our current code?
{code}
try {
storagePolicyId = getStoragePolicyID();
} catch (UnsupportedOperationException ex) {
if (LOG.isDebugEnabled()) {
LOG.debug("BlockStoragePolicy is not supported on " + this.getClass());
}
}
{code}
# We now have 4 different versions of {{computeQuotaUsage}} to support different
parameter combinations. We can remove some of them for simplification. E.g.,
{{computeQuotaUsage(BlockStoragePolicySuite, byte, QuotaCounts, boolean)}} is
only called in one place.
# In {{INodeFile#computeQuotaUsage}}, we should check if the INodeFile has local
storage policy and if so use it to overwrite the given policy.
{code}
byte storagePolicyId =
(blockStoragePolicyId != BlockStoragePolicySuite.ID_UNSPECIFIED) ?
blockStoragePolicyId : getStoragePolicyID();
{code}
# Similar logic should also be used in INodeDirectory. In general, we should use
the current directory's storage policy (if there is one) to overwrite the
given policy and pass it down to its children.
# For deleted files/directories in snapshots, we can do the same thing
as a normal child file/directory. Thus simply passing the parent directory's
storage policy down should be good enough. Handling a referrence INode can be
more complicated, but we can do it in a separate jira.
{code}
- deleted.computeQuotaUsage(bsps, counts, false,
Snapshot.CURRENT_STATE_ID);
+ deleted.computeQuotaUsage(bsps, deleted.tryGetBlockStoragePolicyId(),
+ counts, false, Snapshot.CURRENT_STATE_ID);
{code}
# Minor: there are couple of places where the line length exceeds 80
> Avoid recursive call getStoragePolicyID in INodeFile#computeQuotaUsage
> ----------------------------------------------------------------------
>
> Key: HDFS-7811
> URL: https://issues.apache.org/jira/browse/HDFS-7811
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Reporter: Xiaoyu Yao
> Assignee: Xiaoyu Yao
> Fix For: 2.7.0
>
> Attachments: HDFS-7811.00.patch
>
>
> This is a follow up based on comment from [~jingzhao] on HDFS-7723.
> I just noticed that INodeFile#computeQuotaUsage calls getStoragePolicyID to
> identify the storage policy id of the file. This may not be very efficient
> (especially when we're computing the quota usage of a directory) because
> getStoragePolicyID may recursively check the ancestral INode's storage
> policy. I think here an improvement can be passing the lowest parent
> directory's storage policy down while traversing the tree.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
