[
https://issues.apache.org/jira/browse/HDFS-12214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16109006#comment-16109006
]
Brahma Reddy Battula edited comment on HDFS-12214 at 8/4/17 9:43 AM:
---------------------------------------------------------------------
[~rakeshr] thanks for updating the patch and considering some of my offline
comments.
*Few minor nits:*
1) Now you need add the following while starting the SPS..? just I disabled
{{storagePolicyEnabled}} and executed the {{TestStoragePolicyCommands}},it's
pass.
{noformat}
if (!(storagePolicyEnabled && spsEnabled)) {
LOG.warn(
"Failed to start StoragePolicySatisfier"
+ " since {} set to {} and {} set to {}.",
DFSConfigKeys.DFS_STORAGE_POLICY_ENABLED_KEY, storagePolicyEnabled,
DFSConfigKeys.DFS_STORAGE_POLICY_SATISFIER_ENABLED_KEY, spsEnabled);
}
{noformat}
2) how about having one testcase,when {{DFS_STORAGE_POLICY_ENABLED_KEY}} is
disabled..? if already there,please ignore
3) can you update {{writeUnlock(operationName);}} in
{{org.apache.hadoop.hdfs.server.namenode.FSNamesystem#satisfyStoragePolicy}}
{noformat}
2173 writeUnlock();
{noformat}
4) can we add null check in
{{org.apache.hadoop.hdfs.server.namenode.FSDirSatisfyStoragePolicyOp#unprotectedSatisfyStoragePolicy}}..?
{noformat}
if (inode == null) {
throw new FileNotFoundException("File/Directory does not exist: "
+ iip.getPath());
}
{noformat}
5) can we update statistics ..?
org.apache.hadoop.hdfs.DistributedFileSystem#satisfyStoragePolicy, seperate
jira also ok, as it's not target for this.
{noformat}
statistics.incrementWriteOps(1);
storageStatistics.incrementOpCounter(OpType.SET_STORAGE_POLICY);
{noformat}
May be you can change jira Title..?
was (Author: brahmareddy):
[~rakeshr] thanks for updating the patch and considering some of my offline
comments.
*Few minor nits:*
1) Now you need add the following while starting the SPS..? just I disabled
{{storagePolicyEnabled}] and executed the {{TestStoragePolicyCommands}},it's
pass.
{noformat}
if (!(storagePolicyEnabled && spsEnabled)) {
LOG.warn(
"Failed to start StoragePolicySatisfier"
+ " since {} set to {} and {} set to {}.",
DFSConfigKeys.DFS_STORAGE_POLICY_ENABLED_KEY, storagePolicyEnabled,
DFSConfigKeys.DFS_STORAGE_POLICY_SATISFIER_ENABLED_KEY, spsEnabled);
}
{noformat}
2) how about having one testcase,when {{DFS_STORAGE_POLICY_ENABLED_KEY}} is
disabled..? if already there,please ignore
3) can you update {{writeUnlock(operationName);}} in
{{org.apache.hadoop.hdfs.server.namenode.FSNamesystem#satisfyStoragePolicy}}
{noformat}
2173 writeUnlock();
{noformat}
4) can we add null check in
{{org.apache.hadoop.hdfs.server.namenode.FSDirSatisfyStoragePolicyOp#unprotectedSatisfyStoragePolicy}}..?
{noformat}
if (inode == null) {
throw new FileNotFoundException("File/Directory does not exist: "
+ iip.getPath());
}
{noformat}
5) can we update statistics ..?
org.apache.hadoop.hdfs.DistributedFileSystem#satisfyStoragePolicy, seperate
jira also ok, as it's not target for this.
{noformat}
statistics.incrementWriteOps(1);
storageStatistics.incrementOpCounter(OpType.SET_STORAGE_POLICY);
{noformat}
May be you can change jira Title..?
> [SPS]: Fix review comments of 'dfs.storage.policy.satisfier.activate'
> configuration
> -----------------------------------------------------------------------------------
>
> Key: HDFS-12214
> URL: https://issues.apache.org/jira/browse/HDFS-12214
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Reporter: Rakesh R
> Assignee: Rakesh R
> Attachments: HDFS-12214-HDFS-10285-00.patch,
> HDFS-12214-HDFS-10285-01.patch, HDFS-12214-HDFS-10285-02.patch,
> HDFS-12214-HDFS-10285-03.patch, HDFS-12214-HDFS-10285-04.patch
>
>
> This sub-task is to address [~andrew.wang]'s review comments. Please refer
> the [review
> comment|https://issues.apache.org/jira/browse/HDFS-10285?focusedCommentId=16103734&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16103734]
> in HDFS-10285 umbrella jira.
> # Rename configuration property 'dfs.storage.policy.satisfier.activate' to
> 'dfs.storage.policy.satisfier.enabled'
> # Disable SPS feature by default.
> # Rather than using the acronym (which a user might not know), maybe rename
> "-isSpsRunning" to "-isSatisfierRunning"
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
