Skip to content

server: Search zone-wide storage pool when allocation algothrim is firstfitleastconsumed #4002

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
andrijapanicsb merged 1 commit into from
Apr 6, 2020
Merged

server: Search zone-wide storage pool when allocation algothrim is firstfitleastconsumed #4002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,5 @@ List<SummedCapacity> findCapacityBy(Integer capacityType, Long zoneId,

float findClusterConsumption(Long clusterId, short capacityType, long computeRequested);

List<Long> orderHostsByFreeCapacity(Long clusterId, short capacityType);
List<Long> orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityType);
}
22 changes: 15 additions & 7 deletions engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,20 +903,28 @@ public Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long
}

@Override
public List<Long> orderHostsByFreeCapacity(Long clusterId, short capacityTypeForOrdering){
public List<Long> orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityTypeForOrdering){
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache wouldn't a cluster imply a zone? we wouldn't have to pass a zone if we already know the cluster.

Copy link
Copy Markdown
Member

@weizhouapache weizhouapache Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland if we search storage pools by clusterid, we would not get the list of zone-wide storage pools, because cluster_id is NULL.

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, but if we have a cluster we can find the zone it is in and use that to find zone wide pools for it?

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear, I think this is aesthetic, not a potential bug.

Copy link
Copy Markdown
Member

@weizhouapache weizhouapache Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland
please look at the code, there is only 'AND cluster_id = ' in the sql.
you can also execute the query in mysql

select * from op_host_capacity where capacity_type=3 and cluster_id=??

it lists only the cluster-wide storage pools.

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i read the code. Either zone_id or cluster_id should be null. What is the use case of both being not null?
My point is; pass the cluster and find all storage pools for that cluster either cluster wide or for the zone the cluster is in.

Copy link
Copy Markdown
Member

@weizhouapache weizhouapache Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland what if the storage pool is used for another zone ?

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we pass the cluster and deduce the zone from that, or if we deliberately pass another zone than the cluster we are looking to deploy in? I don't see how can it ever be another zone,except for a bad bug. Or do you mean a storage pool that is used for multiple zones? I have not seen that before.

Copy link
Copy Markdown
Member

@weizhouapache weizhouapache Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland as I said, if we pass the cluster id, it lists only cluster-wide storage pools.
To list zone-wide storage pools, we should pass the zoneId/data_center_id (and cluster_id should be NULL).

TransactionLegacy txn = TransactionLegacy.currentTxn();
PreparedStatement pstmt = null;
List<Long> result = new ArrayList<Long>();
StringBuilder sql = new StringBuilder(ORDER_HOSTS_BY_FREE_CAPACITY_PART1);
if(clusterId != null) {
sql.append("AND cluster_id = ?");
}
sql.append(ORDER_HOSTS_BY_FREE_CAPACITY_PART2);
if (zoneId != null) {
sql.append(" AND data_center_id = ?");
}
if (clusterId != null) {
sql.append(" AND cluster_id = ?");
}
sql.append(ORDER_HOSTS_BY_FREE_CAPACITY_PART2);
try {
pstmt = txn.prepareAutoCloseStatement(sql.toString());
pstmt.setShort(1, capacityTypeForOrdering);
if(clusterId != null) {
pstmt.setLong(2, clusterId);
int index = 2;
if (zoneId != null) {
pstmt.setLong(index, zoneId);
index ++;
}
if (clusterId != null) {
pstmt.setLong(index, clusterId);
}

ResultSet rs = pstmt.executeQuery();
Expand Down
3 changes: 2 additions & 1 deletion ...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 @@ -94,6 +94,7 @@ public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile

protected List<StoragePool> reorderPoolsByCapacity(DeploymentPlan plan,
List<StoragePool> pools) {
Long zoneId = plan.getDataCenterId();
Long clusterId = plan.getClusterId();
short capacityType;
if(pools != null && pools.size() != 0){
Expand All @@ -102,7 +103,7 @@ protected List<StoragePool> reorderPoolsByCapacity(DeploymentPlan plan,
return null;
}

List<Long> poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(clusterId, capacityType);
List<Long> poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType);
if (s_logger.isDebugEnabled()) {
s_logger.debug("List of pools in descending order of free capacity: "+ poolIdsByCapacity);
}
Expand Down
38 changes: 38 additions & 0 deletions ...e/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;

import com.cloud.capacity.Capacity;
import com.cloud.capacity.dao.CapacityDao;
import com.cloud.deploy.DeploymentPlan;
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
Expand All @@ -43,6 +45,8 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator {
private static final Logger LOGGER = Logger.getLogger(ZoneWideStoragePoolAllocator.class);
@Inject
private DataStoreManager dataStoreMgr;
@Inject
private CapacityDao capacityDao;


@Override
Expand Down Expand Up @@ -110,6 +114,40 @@ private boolean canAddStoragePoolToAvoidSet(StoragePoolVO storagePoolVO) {
return !ScopeType.ZONE.equals(storagePoolVO.getScope()) || !storagePoolVO.isManaged();
}


@Override
protected List<StoragePool> reorderPoolsByCapacity(DeploymentPlan plan,
List<StoragePool> pools) {
Long zoneId = plan.getDataCenterId();
short capacityType;
if(pools != null && pools.size() != 0){
capacityType = pools.get(0).getPoolType().isShared() ? Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED : Capacity.CAPACITY_TYPE_LOCAL_STORAGE;
} else{
return null;
}

List<Long> poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, null, capacityType);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("List of zone-wide storage pools in descending order of free capacity: "+ poolIdsByCapacity);
}

//now filter the given list of Pools by this ordered list
Map<Long, StoragePool> poolMap = new HashMap<>();
for (StoragePool pool : pools) {
poolMap.put(pool.getId(), pool);
}
List<Long> matchingPoolIds = new ArrayList<>(poolMap.keySet());

poolIdsByCapacity.retainAll(matchingPoolIds);

List<StoragePool> reorderedPools = new ArrayList<>();
for(Long id: poolIdsByCapacity){
reorderedPools.add(poolMap.get(id));
}

return reorderedPools;
}

@Override
protected List<StoragePool> reorderPoolsByNumberOfVolumes(DeploymentPlan plan, List<StoragePool> pools, Account account) {
if (account == null) {
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,14 +345,15 @@ protected List<Host> allocateTo(DeploymentPlan plan, ServiceOffering offering, V

// Reorder hosts in the decreasing order of free capacity.
private List<? extends Host> reorderHostsByCapacity(DeploymentPlan plan, List<? extends Host> hosts) {
Long zoneId = plan.getDataCenterId();
Long clusterId = plan.getClusterId();
//Get capacity by which we should reorder
String capacityTypeToOrder = _configDao.getValue(Config.HostCapacityTypeToOrderClusters.key());
short capacityType = CapacityVO.CAPACITY_TYPE_CPU;
if("RAM".equalsIgnoreCase(capacityTypeToOrder)){
capacityType = CapacityVO.CAPACITY_TYPE_MEMORY;
}
List<Long> hostIdsByFreeCapacity = _capacityDao.orderHostsByFreeCapacity(clusterId, capacityType);
List<Long> hostIdsByFreeCapacity = _capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType);
if (s_logger.isDebugEnabled()) {
s_logger.debug("List of hosts in descending order of free capacity in the cluster: "+ hostIdsByFreeCapacity);
}
Expand Down