sodonnel commented on a change in pull request #678: HDDS-3179 Pipeline placement based on Topology does not have fallback URL: https://github.com/apache/hadoop-ozone/pull/678#discussion_r394356101
########## File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java ########## @@ -21,6 +21,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdds.conf.OzoneConfiguration; Review comment: There are a lot of changes in this test file, but do we have a test that reproduces the problem reported? The problem is that if the anchor node is on a rack with only a single node, then only 2 nodes would be allocated rather than 3? It should be possible to reproduce this easily with a 3 node and 3 rack cluster, one rack per node. I think we should add a test that reproduces the problem so we know the changes correct it and we have no regressions in the future. We should also test this via the public API of PipelinePlacementPolicy if possible. Right now, most of the tests in TestPipelinePolicy seem to call "@VisibleForTesting" methods rather than chooseDatanodes or getResultSet, which are the public methods. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
