[jira] [Commented] (HDFS-7811) Avoid recursive call getStoragePolicyID in INodeFile#computeQuotaUsage

["Jing Zhao (JIRA)" <ji...@apache.org>]

    [ 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)