[
https://issues.apache.org/jira/browse/HDFS-7811?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14391748#comment-14391748
]
Jing Zhao commented on HDFS-7811:
---------------------------------
Thanks for updating the patch, Xiaoyu! The patch looks good to me overall. Some
comments:
# In {{tryGetBlockStoragePolicyId(byte)}}, it is still possible that
{{getStoragePolicyID}} gets called repeatedly when the storage policy id is not
specified on any INode in the path.
# In {{FSImage#updateCountForQuotaRecursively}}, the following code may keep
using the same policy id (which is not ID_UNSPECIFIED) instead of refreshing it
based on child INode's local setting.
{code}
- updateCountForQuotaRecursively(bsps, child.asDirectory(), counts);
+ updateCountForQuotaRecursively(bsps,
+ (blockStoragePolicyId == BlockStoragePolicySuite.ID_UNSPECIFIED) ?
+ child.tryGetBlockStoragePolicyId() : blockStoragePolicyId,
+ child.asDirectory(), counts);
{code}
# For {{computeQuotaUsage}} of a reference INode we can simply use the given
storage policy id
# However, for {{INodeReference#computeContentSummary}}, we should call
{{getStoragePolicyID}}
# It looks to me that the parameter blockStoragePolicyId's semantic is not
consistent currently. In some cases it is the policy id of the parent, but
sometimes it's the policy id of the current INode (e.g.,
{{DirectoryWithSnapshotFeature#computeQuotaUsage4CurrentDirectory}})
> 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
> Attachments: HDFS-7811.00.patch, HDFS-7811.01.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)
