[ https://issues.apache.org/jira/browse/HDFS-8647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14736163#comment-14736163 ]
Ming Ma commented on HDFS-8647: ------------------------------- Thanks [~brahmareddy]! The patch covers the basic idea of the rack policy refactoring. * Backward compatibility for {{BlockPlacementPolicy}}. There is a hypothetic scenario where people have developed some custom {{BlockPlacementPolicy}} class packaged in a separate jar from the hdfs jar. Upgrading from 2.7 to 2.8 (assume we put this change in 2.8) will break that unless they recompile their custom class. That is because {{BlockManager}} will start calling {{chooseReplicasToDelete}} method which isn't implemented in their class. Perhaps that hasn't been the supported scenario. For example, HDFS-6584 modifies {{chooseReplicaToDelete}} directly. If that is the case, you might as well remove the old method {{chooseReplicaToDelete}} from {{BlockPlacementPolicy}}. * {{BlockManager}} still has its own {{blockHasEnoughRacks}} function that implies rack based policy. Given we plan to add new block placement policy that isn't rack related, {{isNeededReplication}} is essentially checking if the existing replicas violate the policy. Perhaps we can rename that function to be something like {{verifyReplicasPlacement}}. Same for the new {{BlockPlacementPolicy}}'s {{blockHasEnoughRacks}}. Note that this seems similar to {{BlockPlacementPolicy}}'s {{verifyBlockPlacement}}. So if they can be combined, that is nice. * With the code refactored into {{BlockPlacementPolicy}}, it also allows us to write unit tests that might be tricky before. So it will be good if you can add unit tests for new methods added to {{BlockPlacementPolicy}}. > Abstract BlockManager's rack policy into BlockPlacementPolicy > ------------------------------------------------------------- > > Key: HDFS-8647 > URL: https://issues.apache.org/jira/browse/HDFS-8647 > Project: Hadoop HDFS > Issue Type: Improvement > Reporter: Ming Ma > Assignee: Brahma Reddy Battula > Attachments: HDFS-8647-001.patch, HDFS-8647-002.patch > > > Sometimes we want to have namenode use alternative block placement policy > such as upgrade domains in HDFS-7541. > BlockManager has built-in assumption about rack policy in functions such as > useDelHint, blockHasEnoughRacks. That means when we have new block placement > policy, we need to modify BlockManager to account for the new policy. Ideally > BlockManager should ask BlockPlacementPolicy object instead. That will allow > us to provide new BlockPlacementPolicy without changing BlockManager. -- This message was sent by Atlassian JIRA (v6.3.4#6332)