GutoVeronezi commented on code in PR #6307:
URL: https://github.com/apache/cloudstack/pull/6307#discussion_r862784314


##########
engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java:
##########
@@ -169,25 +175,57 @@ protected List<StoragePool> 
reorderPoolsByNumberOfVolumes(DeploymentPlan plan, L
 
     @Override
     public List<StoragePool> reorderPools(List<StoragePool> pools, 
VirtualMachineProfile vmProfile, DeploymentPlan plan, DiskProfile dskCh) {
+        if (s_logger.isTraceEnabled()) {
+            s_logger.trace("reordering pools");
+        }
         if (pools == null) {
             return null;
         }
+        if (s_logger.isTraceEnabled()) {
+            s_logger.trace(String.format("reordering %d pools", pools.size()));
+        }
         Account account = null;
         if (vmProfile.getVirtualMachine() != null) {
             account = vmProfile.getOwner();
         }
 
         if (allocationAlgorithm.equals("random") || 
allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
-            // Shuffle this so that we don't check the pools in the same order.
-            Collections.shuffle(pools);
+            if (s_logger.isTraceEnabled()) {
+                s_logger.trace(String.format("Shuffle this so that we don't 
check the pools in the same order. Algorithm == '%s' (or no 
account?)",allocationAlgorithm));
+                StringBuilder pooltable = new StringBuilder();
+                pooltable.append("pools to choose from: ");
+                int i = 1;
+                for (StoragePool pool : pools) {
+                    pooltable.append("\nno ").append(i).append(": 
").append(pool.getName()).append("/").append(pool.getUuid());
+                }
+                s_logger.trace(pooltable.toString());
+            }
+            Collections.shuffle(pools, secureRandom);
+            if (s_logger.isTraceEnabled()) {
+                StringBuilder pooltable = new StringBuilder();
+                pooltable.append("shuffled list of pools to choose from: ");
+                int i = 1;
+                for (StoragePool pool : pools) {
+                    pooltable.append("\nno ").append(i).append(": 
").append(pool.getName()).append("/").append(pool.getUuid());
+                }
+                s_logger.trace(pooltable.toString());
+            }
         } else if (allocationAlgorithm.equals("userdispersing")) {
+            if (s_logger.isTraceEnabled()) {
+                s_logger.trace(String.format("reordering: Algorithm == 
'%s'",allocationAlgorithm));
+            }
             pools = reorderPoolsByNumberOfVolumes(plan, pools, account);
         } else if(allocationAlgorithm.equals("firstfitleastconsumed")){
+            if (s_logger.isTraceEnabled()) {
+                s_logger.trace(String.format("reordering: Algorithm == 
'%s'",allocationAlgorithm));
+            }
             pools = reorderPoolsByCapacity(plan, pools);

Review Comment:
   @DaanHoogland, I mean something like this, to avoid multiple blocks 
repeating the same log:
   
   ```java
           } else if (StringUtils.equalsAny(allocationAlgorithm, 
"userdispersing", "firstfitleastconsumed")) {
               if (s_logger.isTraceEnabled()) {
                   s_logger.trace(String.format("Using reordering algorithm 
[%s]", allocationAlgorithm));
               }
               
               if (allocationAlgorithm.equals("userdispersing")) {
                   pools = reorderPoolsByNumberOfVolumes(plan, pools, account);
               } else {
                   pools = reorderPoolsByCapacity(plan, pools);
               }
   ``` 



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to