[
https://issues.apache.org/jira/browse/HDFS-12310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16210106#comment-16210106
]
Lei (Eddy) Xu commented on HDFS-12310:
--------------------------------------
Hi, [~surendrasingh]
Thanks for working on it. Please see the following reviews:
* Please fix the comments:
{code}
/**
* @return ture if all the files satisfy policy in given path.
* @throws IOException
*/
public StoragePolicySatisfyPathStatus checkStoragePolicySatisfyPathStatus(
{code}
It should put {{spsStatusInfo}} into the map?
{code}
StoragePolicySatisfyPathStatusInfo spsStatusInfo =
spsStatus.get(startInode.getId());
if (spsStatusInfo == null) {
spsStatusInfo = new StoragePolicySatisfyPathStatusInfo();
}
spsStatusInfo.setSuccessStatus();
{code}
The above code is in muitple places, i.e., {{removeItemTrackInfo}}, looks
suspicious.
* Would it make sense to put {{cleanSpsStatus();}} into {{ if
(!namesystem.isInSafeMode()) }}, and avoid run it for every processed inode.
* {{StoragePolicySatisfyPathStatusInfo#setSuccessStatus()/setInProgress() /
canRemoveStatus()}} can be project-private. Also it'd be nice to be consistent
with the names, like {{setSuccess() / canRemove()}}, {{setInProgress()}} is
a good example.
* Could you add comment to {{DFSClient# checkStoragePolicySatisfyPathStatus()}}
as well?
Thanks!
> [SPS]: Provide an option to track the status of in progress requests
> --------------------------------------------------------------------
>
> Key: HDFS-12310
> URL: https://issues.apache.org/jira/browse/HDFS-12310
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Reporter: Uma Maheswara Rao G
> Assignee: Surendra Singh Lilhore
> Attachments: HDFS-12310-HDFS-10285-01.patch,
> HDFS-12310-HDFS-10285-02.patch, HDFS-12310-HDFS-10285-03.patch
>
>
> As per the [~andrew.wang] 's review comments in HDFS-10285, This is the JIRA
> for tracking about the options how we track the progress of SPS requests.
>
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
