Fix ordering of secondary storages with the algorithm firstfitleastconsumed (#8557)

* Fix ordering of secondary storages with the algorithm `firstfitleastconsumed`

* return store without checking all

* Add unit tests

---------

Co-authored-by: Gabriel <gabriel.fernandes@scclouds.com.br>
Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
This commit is contained in:
GaOrtiga 2025-02-14 08:33:04 -03:00 committed by GitHub
parent 617fee8416
commit 864751d5f9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 74 additions and 38 deletions

View File

@ -20,7 +20,6 @@ package org.apache.cloudstack.storage.image.manager;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
@ -180,28 +179,14 @@ public class ImageStoreProviderManagerImpl implements ImageStoreProviderManager,
@Override @Override
public DataStore getImageStoreWithFreeCapacity(List<DataStore> imageStores) { public DataStore getImageStoreWithFreeCapacity(List<DataStore> imageStores) {
if (imageStores.size() > 1) { imageStores.sort((store1, store2) -> Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store2),
imageStores.sort(new Comparator<DataStore>() { // Sort data stores based on free capacity _statsCollector.imageStoreCurrentFreeCapacity(store1)));
@Override for (DataStore imageStore : imageStores) {
public int compare(DataStore store1, DataStore store2) { if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1), return imageStore;
_statsCollector.imageStoreCurrentFreeCapacity(store2));
}
});
for (DataStore imageStore : imageStores) {
// Return image store if used percentage is less then threshold value i.e. 90%.
if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
return imageStore;
}
}
} else if (imageStores.size() == 1) {
if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) {
return imageStores.get(0);
} }
} }
logger.error(String.format("Could not find an image storage in zone with less than %d usage",
// No store with space found
logger.error(String.format("Can't find an image storage in zone with less than %d usage",
Math.round(_statsCollector.getImageStoreCapacityThreshold() * 100))); Math.round(_statsCollector.getImageStoreCapacityThreshold() * 100)));
return null; return null;
} }
@ -209,23 +194,11 @@ public class ImageStoreProviderManagerImpl implements ImageStoreProviderManager,
@Override @Override
public List<DataStore> orderImageStoresOnFreeCapacity(List<DataStore> imageStores) { public List<DataStore> orderImageStoresOnFreeCapacity(List<DataStore> imageStores) {
List<DataStore> stores = new ArrayList<>(); List<DataStore> stores = new ArrayList<>();
if (imageStores.size() > 1) { imageStores.sort((store1, store2) -> Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store2),
imageStores.sort(new Comparator<DataStore>() { // Sort data stores based on free capacity _statsCollector.imageStoreCurrentFreeCapacity(store1)));
@Override for (DataStore imageStore : imageStores) {
public int compare(DataStore store1, DataStore store2) { if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1), stores.add(imageStore);
_statsCollector.imageStoreCurrentFreeCapacity(store2));
}
});
for (DataStore imageStore : imageStores) {
// Return image store if used percentage is less then threshold value i.e. 90%.
if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
stores.add(imageStore);
}
}
} else if (imageStores.size() == 1) {
if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) {
stores.add(imageStores.get(0));
} }
} }
return stores; return stores;

View File

@ -16,6 +16,9 @@
// under the License. // under the License.
package org.apache.cloudstack.storage.image.manager; package org.apache.cloudstack.storage.image.manager;
import com.cloud.server.StatsCollector;
import com.cloud.utils.Pair;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
import org.junit.Assert; import org.junit.Assert;
@ -26,14 +29,22 @@ import org.mockito.Mock;
import org.mockito.Mockito; import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner; import org.mockito.junit.MockitoJUnitRunner;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@RunWith(MockitoJUnitRunner.class) @RunWith(MockitoJUnitRunner.class)
public class ImageStoreProviderManagerImplTest { public class ImageStoreProviderManagerImplTest {
@Mock @Mock
ImageStoreDao imageStoreDao; ImageStoreDao imageStoreDao;
@Mock
StatsCollector statsCollectorMock;
@InjectMocks @InjectMocks
ImageStoreProviderManagerImpl imageStoreProviderManager = new ImageStoreProviderManagerImpl(); ImageStoreProviderManagerImpl imageStoreProviderManager = new ImageStoreProviderManagerImpl();
@Test @Test
public void testGetImageStoreZoneId() { public void testGetImageStoreZoneId() {
final long storeId = 1L; final long storeId = 1L;
@ -44,4 +55,56 @@ public class ImageStoreProviderManagerImplTest {
long value = imageStoreProviderManager.getImageStoreZoneId(storeId); long value = imageStoreProviderManager.getImageStoreZoneId(storeId);
Assert.assertEquals(zoneId, value); Assert.assertEquals(zoneId, value);
} }
private Pair<List<DataStore>, List<DataStore>> prepareUnorderedAndOrderedImageStoresForCapacityTests(boolean hasStoragesWithEnoughCapacity) {
DataStore store1 = Mockito.mock(DataStore.class);
Mockito.doReturn(100L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store1);
Mockito.doReturn(false).when(statsCollectorMock).imageStoreHasEnoughCapacity(store1);
DataStore store2 = Mockito.mock(DataStore.class);
Mockito.doReturn(200L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store2);
Mockito.doReturn(hasStoragesWithEnoughCapacity).when(statsCollectorMock).imageStoreHasEnoughCapacity(store2);
DataStore store3 = Mockito.mock(DataStore.class);
Mockito.doReturn(300L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store3);
Mockito.doReturn(hasStoragesWithEnoughCapacity).when(statsCollectorMock).imageStoreHasEnoughCapacity(store3);
DataStore store4 = Mockito.mock(DataStore.class);
Mockito.doReturn(400L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store4);
Mockito.doReturn(false).when(statsCollectorMock).imageStoreHasEnoughCapacity(store4);
List<DataStore> unordered = Arrays.asList(store1, store2, store3, store4);
List<DataStore> orderedAndEnoughCapacity = new ArrayList<>();
if (hasStoragesWithEnoughCapacity) {
orderedAndEnoughCapacity.add(store3);
orderedAndEnoughCapacity.add(store2);
}
return new Pair<>(unordered, orderedAndEnoughCapacity);
}
@Test
public void getImageStoreWithFreeCapacityTestImageStoresWithEnoughCapacityExistReturnsImageStoreWithMostFreeCapacity() {
Pair<List<DataStore>, List<DataStore>> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(true);
DataStore result = imageStoreProviderManager.getImageStoreWithFreeCapacity(unorderedAndOrdered.first());
Assert.assertEquals(unorderedAndOrdered.second().get(0), result);
}
@Test
public void getImageStoreWithFreeCapacityTestImageStoresWithEnoughCapacityDoNotExistReturnsNull() {
Pair<List<DataStore>, List<DataStore>> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(false);
DataStore result = imageStoreProviderManager.getImageStoreWithFreeCapacity(unorderedAndOrdered.first());
Assert.assertNull(result);
}
@Test
public void orderImageStoresOnFreeCapacityTestReturnsImageStoresOrderedFromMostToLeast() {
Pair<List<DataStore>, List<DataStore>> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(true);
List<DataStore> result = imageStoreProviderManager.orderImageStoresOnFreeCapacity(unorderedAndOrdered.first());
Assert.assertEquals(unorderedAndOrdered.second(), result);
}
} }