Koushik, The deployment planning process is divided between DeploymentPlanners and Allocators. The planners are supposed to feed a list of clusters the allocators should look into to find a potential destination. While the allocators should provide a avoid set back to the planners consisting of any resource not considered.
Reasoning how the planners work: Planners can set/reset clusters into avoid set depending on the heuristics they implement and the order in which they want the clusters to be traversed. When all options of the planner are exhausted they should return an empty cluster list halting the search. What should the allocators do? Iterate over the given cluster list and in each cluster find out suitable resources - all other resources not considered AND not suitable MUST be added to the avoid set so that the planners get the correct avoid input. This is necessary for the logic to not enter an infinite loop. As you see, only planners can halt the search process by referring to the avoid set provided by the allocators. If you see any case not following this, that needs to be fixed. Also, I do think in general it will be safe to add a configurable retry limit on this loop to have control in case any new logic in allocators don't follow this reasoning. I will add that limit. Thanks, Prachi -----Original Message----- From: Koushik Das [mailto:koushik....@citrix.com] Sent: Tuesday, October 15, 2013 9:19 AM To: <dev@cloudstack.apache.org> Subject: Possible bug in DeploymentPlanner? I was making some changes in the storage pool allocators related to some bug fix and came across this code snippet in planDeplyment() method of DeploymentPlanningManagerImpl.java. In this if the checkClustersforDestination() returns null and the 'avoids' parameter is not correctly updated (one such place can be the storage allocators) then the while loop will never terminate. Is there any assumption about how the 'avoids' parameter needs to be updated? From the code it is not very intuitive. I saw some places in the storage pool allocators where this will not get updated. Wanted to understand the reason for doing it this way? Can the while (true) condition be replaced with something more intuitive? while (true) { if (planner instanceof DeploymentClusterPlanner) { ExcludeList plannerAvoidInput = new ExcludeList(avoids.getDataCentersToAvoid(), avoids.getPodsToAvoid(), avoids.getClustersToAvoid(), avoids.getHostsToAvoid(), avoids.getPoolsToAvoid()); clusterList = ((DeploymentClusterPlanner) planner).orderClusters(vmProfile, plan, avoids); if (clusterList != null && !clusterList.isEmpty()) { // planner refactoring. call allocators to list hosts ExcludeList plannerAvoidOutput = new ExcludeList(avoids.getDataCentersToAvoid(), avoids.getPodsToAvoid(), avoids.getClustersToAvoid(), avoids.getHostsToAvoid(), avoids.getPoolsToAvoid()); resetAvoidSet(plannerAvoidOutput, plannerAvoidInput); dest = checkClustersforDestination(clusterList, vmProfile, plan, avoids, dc, getPlannerUsage(planner, vmProfile, plan, avoids), plannerAvoidOutput); if (dest != null) { return dest; } // reset the avoid input to the planners resetAvoidSet(avoids, plannerAvoidOutput); } else { return null; } } else { ............ ............ } }
