[
https://issues.apache.org/jira/browse/HDFS-13097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16347565#comment-16347565
]
Surendra Singh Lilhore commented on HDFS-13097:
-----------------------------------------------
Fixing below comments
*Comment-1)*
{quote}BlockManager
Shouldn’t spsMode be volatile? Although I question why it’s here.
{quote}
[Rakesh's reply] Agreed, will do the changes.
*Comment-2)*
{quote}Adding SPS methods to this class implies an unexpected coupling of the
SPS service to the block manager. Please move them out to prove it’s not
tightly coupled.
{quote}
[Rakesh's reply] Agreed. I'm planning to create {{StoragePolicySatisfyManager}}
and keep all the related apis over there.
*Comment-5)*
{quote}DatanodeDescriptor
Why use a synchronized linked list to offer/poll instead of BlockingQueue?
{quote}
[Rakesh's reply] Agreed, will do the changes.
*Comment-8)*
{quote}DFSUtil
DFSUtil.removeOverlapBetweenStorageTypes and {{DFSUtil.getSPSWorkMultiplier
}}. These aren’t generally useful methods so why are they in DFSUtil? Why
aren’t they in the only calling class StoragePolicySatisfier?
{quote}
[Rakesh's reply] Agreed, Will do the changes.
*Comment-11)*
{quote}HdfsServerConstants
The xattr is called user.hdfs.sps.xattr. Why does the xattr name actually
contain the word “xattr”?
{quote}
[Rakesh's reply] Sure, will remove “xattr” word.
*Comment-12)*
{quote}NameNode
Super trivial but using the plural pronoun “we” in this exception message is
odd. Changing the value isn’t a joint activity.
For enabling or disabling storage policy satisfier, we must pass either
none/internal/external string value only
{quote}
[Rakesh's reply] oops, sorry for the mistake. Will change it.
*Comment-16)*
{quote}FSDirStatAndListOp
Not sure why javadoc was changed to add needLocation. It's already present and
now doubled up.{quote}
[Rakesh'r reply] Agreed, will correct it.
*Comment-18)*
{quote}DFS_MOVER_MOVERTHREADS_DEFAULT is 1000 per DN? If the DN is concurrently
doing 1000 moves, it's not in a good state, disk io is probably saturated, and
this will only make it much worse. 10 is probably more than sufficient.{quote}
[Rakesh'r reply] Agreed, will make it to smaller value 10.
> [SPS]: Fix the branch review comments(Part1)
> --------------------------------------------
>
> Key: HDFS-13097
> URL: https://issues.apache.org/jira/browse/HDFS-13097
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: HDFS-10285
> Reporter: Surendra Singh Lilhore
> Assignee: Surendra Singh Lilhore
> Priority: Major
>
> Fix the branch review comment.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
