[
https://issues.apache.org/jira/browse/HDFS-5188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767221#comment-13767221
]
Junping Du commented on HDFS-5188:
----------------------------------
Thanks for the patch. Nicholas! I haven't finished my review, some initial
comments below, and more will come later.
1.
{code}
+ public static final String DFS_BLOCK_REPLICATOR_CLASSNAME_KEY =
"dfs.block.replicator.classname";
{code}
Shall we move it to DFSConfigKeys as other configurations?
2.
{code}
+ LOG.info("XXX numOfResults=" + numOfResults + ", writer=" + writer
+ + ", excludedNodes=" + excludedNodes);
// Keep a copy of original excludedNodes
- final HashMap<Node, Node> oldExcludedNodes = avoidStaleNodes ?
+ final Map<Node, Node> oldExcludedNodes = avoidStaleNodes ?
new HashMap<Node, Node>(excludedNodes) : null;
try {
if (numOfResults == 0) {
writer = chooseLocalNode(writer, excludedNodes, blocksize,
maxNodesPerRack, results, avoidStaleNodes);
+ LOG.info("XXX2 numOfResults=" + numOfResults + ", writer=" + writer);
if (--numOfReplicas == 0) {
return writer;
}
{code}
XXX should be replaced with something else?
3.
{code}
@Override
public DatanodeDescriptor[] chooseTarget(String srcPath,
int numOfReplicas,
DatanodeDescriptor writer,
List<DatanodeDescriptor> chosenNodes,
- long blocksize) {
- return chooseTarget(numOfReplicas, writer, chosenNodes, false,
- null, blocksize);
- }
-
- @Override
- public DatanodeDescriptor[] chooseTarget(String srcPath,
- int numOfReplicas,
- DatanodeDescriptor writer,
- List<DatanodeDescriptor> chosenNodes,
boolean returnChosenNodes,
- HashMap<Node, Node> excludedNodes,
+ Map<Node, Node> excludedNodes,
long blocksize) {
return chooseTarget(numOfReplicas, writer, chosenNodes, returnChosenNodes,
excludedNodes, blocksize);
}
{code}
We may consider to remove chooseTarget(String srcPatch, ...) completely as
srcPath is not used actually. Isn't it?
4. I am thinking to refactor chooseTarget(..., DatanodeDescriptor writer, ...)
to chooseTarget (..., node writer, ...) as the only important property of
writer is to identify other nodes' location relationship so more generic one
could be better. It may helps to cover the case that client node is not a
Datanode also. Does it make sense?
Btw, the JIRA's number sounds lucky! :)
> Clean up BlockPlacementPolicy and its implementations
> -----------------------------------------------------
>
> Key: HDFS-5188
> URL: https://issues.apache.org/jira/browse/HDFS-5188
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: namenode
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Tsz Wo (Nicholas), SZE
> Attachments: h5188_20130912b.patch, h5188_20130912.patch
>
>
> Below is a list of code cleanup tasks for BlockPlacementPolicy and its
> implementations:
> - Define constants for dfs.block.replicator.classname.
> - Combine adjustExcludedNodes and addToExcludedNodes: subclasses should
> always add all the related nodes in addToExcludedNodes(..).
> - Remove duplicated code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
