Skip to content

Introduce volume allocation algorithm global configuration #10696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 12 commits into from
May 16, 2025
Merged

Introduce volume allocation algorithm global configuration #10696

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b28db5d
Introduce volume allocation algorithm global configuration.
sudo87 Apr 11, 2025
f6e7fa6
Address review comments
sudo87 Apr 11, 2025
9c3d381
format import statements
sudo87 Apr 11, 2025
33f9e6f
make config dynamic in nature to avoid need of mgmt server restart
sudo87 Apr 14, 2025
c3006bf
Revert method signature change done for unit test
sudo87 Apr 14, 2025
5c10036
Add upgrade patch for volume.allocation.algorithm
sudo87 Apr 16, 2025
2934295
Use configDepot.isNewConfig() to populate volume.allocation.algorithm
sudo87 Apr 16, 2025
cf65404
replace 'random' with VolumeAllocationAlgorithm.getDefaultValue()
sudo87 Apr 17, 2025
e12fd49
resolve merge conflict
sudo87 Apr 22, 2025
979386d
log capacity alongwith host/storagaepool ids
sudo87 Apr 25, 2025
9721484
add unit tests to increase code coverage
sudo87 Apr 30, 2025
f396fa9
resolve merge conflict with main
sudo87 May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/main/java/com/cloud/deploy/DeploymentClusterPlanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public interface DeploymentClusterPlanner extends DeploymentPlanner {
"vm.allocation.algorithm",
"Advanced",
"random",
"Order in which hosts within a cluster will be considered for VM/volume allocation. The value can be 'random', 'firstfit', 'userdispersing', 'userconcentratedpod_random', 'userconcentratedpod_firstfit', or 'firstfitleastconsumed'.",
"Order in which hosts within a cluster will be considered for VM allocation. The value can be 'random', 'firstfit', 'userdispersing', 'userconcentratedpod_random', 'userconcentratedpod_firstfit', or 'firstfitleastconsumed'.",
true,
Comment thread
weizhouapache marked this conversation as resolved.
ConfigKey.Scope.Global, null, null, null, null, null,
ConfigKey.Kind.Select,
Expand Down
11 changes: 11 additions & 0 deletions ...n/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,17 @@ public interface VolumeOrchestrationService {
"The maximum size for a volume (in GiB).",
true);

ConfigKey<String> VolumeAllocationAlgorithm = new ConfigKey<>(
String.class,
"volume.allocation.algorithm",
"Advanced",
"random",
"Order in which storage pool within a cluster will be considered for volume allocation. The value can be 'random', 'firstfit', 'userdispersing', 'userconcentratedpod_random', 'userconcentratedpod_firstfit', or 'firstfitleastconsumed'.",
false,
ConfigKey.Scope.Global, null, null, null, null, null,
ConfigKey.Kind.Select,
"random,firstfit,userdispersing,userconcentratedpod_random,userconcentratedpod_firstfit,firstfitleastconsumed");

VolumeInfo moveVolume(VolumeInfo volume, long destPoolDcId, Long destPoolPodId, Long destPoolClusterId, HypervisorType dataDiskHyperType)
throws ConcurrentOperationException, StorageUnavailableException;

Expand Down
4 changes: 3 additions & 1 deletion ...stration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2018,7 +2018,9 @@

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {RecreatableSystemVmEnabled, MaxVolumeSize, StorageHAMigrationEnabled, StorageMigrationEnabled, CustomDiskOfferingMaxSize, CustomDiskOfferingMinSize, VolumeUrlCheck};
return new ConfigKey<?>[] {

Check warning on line 2021 in engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

View check run for this annotation

Codecov / codecov/patch

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java#L2021 

Added line #L2021 was not covered by tests
RecreatableSystemVmEnabled, MaxVolumeSize, StorageHAMigrationEnabled, StorageMigrationEnabled,
CustomDiskOfferingMaxSize, CustomDiskOfferingMinSize, VolumeUrlCheck, VolumeAllocationAlgorithm};
}

@Override
Expand Down
77 changes: 38 additions & 39 deletions ...e/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,58 +16,58 @@
// under the License.
package org.apache.cloudstack.storage.allocator;

import java.math.BigDecimal;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.api.query.dao.StoragePoolJoinDao;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.storage.ScopeType;
import com.cloud.storage.StoragePoolStatus;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.commons.lang3.StringUtils;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.collections.CollectionUtils;

import com.cloud.utils.Pair;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;

import com.cloud.capacity.Capacity;
import com.cloud.capacity.dao.CapacityDao;
import com.cloud.dc.ClusterVO;
import com.cloud.dc.dao.ClusterDao;
import com.cloud.deploy.DeploymentPlan;
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.storage.ScopeType;
import com.cloud.storage.Storage;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolStatus;
import com.cloud.storage.StorageUtil;
import com.cloud.storage.Volume;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.user.Account;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.StringUtils;
import com.cloud.utils.component.AdapterBase;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VirtualMachineProfile;

import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;

import org.apache.commons.collections.CollectionUtils;
Comment thread
DaanHoogland marked this conversation as resolved.

import javax.inject.Inject;
import javax.naming.ConfigurationException;
import java.math.BigDecimal;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public abstract class AbstractStoragePoolAllocator extends AdapterBase implements StoragePoolAllocator {

protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1);
protected String allocationAlgorithm = "random";
protected String volumeAllocationAlgorithm = "random";
protected long extraBytesPerVolume = 0;
@Inject protected DataStoreManager dataStoreMgr;
@Inject protected PrimaryDataStoreDao storagePoolDao;
Expand Down Expand Up @@ -95,9 +95,9 @@
String globalStorageOverprovisioningFactor = configs.get("storage.overprovisioning.factor");
storageOverprovisioningFactor = new BigDecimal(NumbersUtil.parseFloat(globalStorageOverprovisioningFactor, 2.0f));
extraBytesPerVolume = 0;
String allocationAlgorithm = configs.get("vm.allocation.algorithm");
if (allocationAlgorithm != null) {
this.allocationAlgorithm = allocationAlgorithm;
String volAllocationAlgorithm = VolumeOrchestrationService.VolumeAllocationAlgorithm.value();

Check warning on line 98 in engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java

View check run for this annotation

Codecov / codecov/patch

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java#L98 

Added line #L98 was not covered by tests
if (volAllocationAlgorithm != null) {
this.volumeAllocationAlgorithm = volAllocationAlgorithm;

Check warning on line 100 in engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java

View check run for this annotation

Codecov / codecov/patch

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java#L100 

Added line #L100 was not covered by tests
}
return true;
}
Expand Down Expand Up @@ -227,16 +227,15 @@
}

List<StoragePool> reorderStoragePoolsBasedOnAlgorithm(List<StoragePool> pools, DeploymentPlan plan, Account account) {
logger.debug(String.format("Using allocation algorithm [%s] to reorder pools.", allocationAlgorithm));

if (allocationAlgorithm.equals("random") || allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
logger.debug("Using volume allocation algorithm {} to reorder pools.", volumeAllocationAlgorithm);
if (volumeAllocationAlgorithm.equals("random") || volumeAllocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
reorderRandomPools(pools);
} else if (StringUtils.equalsAny(allocationAlgorithm, "userdispersing", "firstfitleastconsumed")) {
} else if (StringUtils.equalsAny(volumeAllocationAlgorithm, "userdispersing", "firstfitleastconsumed")) {
if (logger.isTraceEnabled()) {
logger.trace(String.format("Using reordering algorithm [%s]", allocationAlgorithm));
logger.trace("Using reordering algorithm {}", volumeAllocationAlgorithm);

Check warning on line 235 in engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java

View check run for this annotation

Codecov / codecov/patch

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java#L235 

Added line #L235 was not covered by tests
}

if (allocationAlgorithm.equals("userdispersing")) {
if (volumeAllocationAlgorithm.equals("userdispersing")) {
pools = reorderPoolsByNumberOfVolumes(plan, pools, account);
} else {
pools = reorderPoolsByCapacity(plan, pools);
Expand All @@ -248,7 +247,7 @@
void reorderRandomPools(List<StoragePool> pools) {
StorageUtil.traceLogStoragePools(pools, logger, "pools to choose from: ");
if (logger.isTraceEnabled()) {
logger.trace(String.format("Shuffle this so that we don't check the pools in the same order. Algorithm == '%s' (or no account?)", allocationAlgorithm));
logger.trace("Shuffle this so that we don't check the pools in the same order. Algorithm == {} (or no account?)", volumeAllocationAlgorithm);

Check warning on line 250 in engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java

View check run for this annotation

Codecov / codecov/patch

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java#L250 

Added line #L250 was not covered by tests
}
StorageUtil.traceLogStoragePools(pools, logger, "pools to shuffle: ");
Collections.shuffle(pools, secureRandom);
Expand Down
32 changes: 12 additions & 20 deletions ...c/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,27 @@
// under the License.
package org.apache.cloudstack.storage.allocator;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.storage.VolumeApiServiceImpl;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.springframework.stereotype.Component;

import com.cloud.deploy.DeploymentPlan;
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.offering.ServiceOffering;
import com.cloud.storage.ScopeType;
import com.cloud.storage.StoragePool;
import com.cloud.storage.VolumeApiServiceImpl;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VirtualMachineProfile;

import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
Comment thread
DaanHoogland marked this conversation as resolved.

import org.springframework.stereotype.Component;

import javax.inject.Inject;
import javax.naming.ConfigurationException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

@Component
public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocator {

Expand Down Expand Up @@ -116,14 +116,6 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
super.configure(name, params);

if (configDao != null) {
Map<String, String> configs = configDao.getConfiguration(params);
String allocationAlgorithm = configs.get("vm.allocation.algorithm");
if (allocationAlgorithm != null) {
this.allocationAlgorithm = allocationAlgorithm;
}
}
return true;
}
}
6 changes: 3 additions & 3 deletions ...c/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_random() {

@Test
public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {
allocator.allocationAlgorithm = "userdispersing";
allocator.volumeAllocationAlgorithm = "userdispersing";
Mockito.doReturn(pools).when(allocator).reorderPoolsByNumberOfVolumes(plan, pools, account);
allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools);
Expand All @@ -92,7 +92,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {

@Test
public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
allocator.allocationAlgorithm = "userdispersing";
allocator.volumeAllocationAlgorithm = "userdispersing";
allocator.volumeDao = volumeDao;

when(plan.getDataCenterId()).thenReturn(1l);
Expand All @@ -115,7 +115,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {

@Test
public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() {
allocator.allocationAlgorithm = "firstfitleastconsumed";
allocator.volumeAllocationAlgorithm = "firstfitleastconsumed";
Mockito.doReturn(pools).when(allocator).reorderPoolsByCapacity(plan, pools);
allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByCapacity(plan, pools);
Expand Down
Loading