Skip to content

Commit 864751d

Browse files
GaOrtigaGabrielwinterhazel
authored
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 <[email protected]> Co-authored-by: Fabricio Duarte <[email protected]>
1 parent 617fee8 commit 864751d

File tree

2 files changed

+74
-38
lines changed

2 files changed

+74
-38
lines changed

‎engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java

Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import java.util.ArrayList;
2222
import java.util.Collections;
23-
import java.util.Comparator;
2423
import java.util.HashMap;
2524
import java.util.List;
2625
import java.util.Map;
@@ -180,52 +179,26 @@ public DataStore getRandomImageStore(List<DataStore> imageStores) {
180179

181180
@Override
182181
public DataStore getImageStoreWithFreeCapacity(List<DataStore> imageStores) {
183-
if (imageStores.size() > 1) {
184-
imageStores.sort(new Comparator<DataStore>() { // Sort data stores based on free capacity
185-
@Override
186-
public int compare(DataStore store1, DataStore store2) {
187-
return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1),
188-
_statsCollector.imageStoreCurrentFreeCapacity(store2));
189-
}
190-
});
191-
for (DataStore imageStore : imageStores) {
192-
// Return image store if used percentage is less then threshold value i.e. 90%.
193-
if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
194-
return imageStore;
195-
}
196-
}
197-
} else if (imageStores.size() == 1) {
198-
if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) {
199-
return imageStores.get(0);
182+
imageStores.sort((store1, store2) -> Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store2),
183+
_statsCollector.imageStoreCurrentFreeCapacity(store1)));
184+
for (DataStore imageStore : imageStores) {
185+
if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
186+
return imageStore;
200187
}
201188
}
202-
203-
// No store with space found
204-
logger.error(String.format("Can't find an image storage in zone with less than %d usage",
189+
logger.error(String.format("Could not find an image storage in zone with less than %d usage",
205190
Math.round(_statsCollector.getImageStoreCapacityThreshold() * 100)));
206191
return null;
207192
}
208193

209194
@Override
210195
public List<DataStore> orderImageStoresOnFreeCapacity(List<DataStore> imageStores) {
211196
List<DataStore> stores = new ArrayList<>();
212-
if (imageStores.size() > 1) {
213-
imageStores.sort(new Comparator<DataStore>() { // Sort data stores based on free capacity
214-
@Override
215-
public int compare(DataStore store1, DataStore store2) {
216-
return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1),
217-
_statsCollector.imageStoreCurrentFreeCapacity(store2));
218-
}
219-
});
220-
for (DataStore imageStore : imageStores) {
221-
// Return image store if used percentage is less then threshold value i.e. 90%.
222-
if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
223-
stores.add(imageStore);
224-
}
225-
}
226-
} else if (imageStores.size() == 1) {
227-
if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) {
228-
stores.add(imageStores.get(0));
197+
imageStores.sort((store1, store2) -> Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store2),
198+
_statsCollector.imageStoreCurrentFreeCapacity(store1)));
199+
for (DataStore imageStore : imageStores) {
200+
if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) {
201+
stores.add(imageStore);
229202
}
230203
}
231204
return stores;

‎engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImplTest.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
// under the License.
1717
package org.apache.cloudstack.storage.image.manager;
1818

19+
import com.cloud.server.StatsCollector;
20+
import com.cloud.utils.Pair;
21+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
1922
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
2023
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
2124
import org.junit.Assert;
@@ -26,14 +29,22 @@
2629
import org.mockito.Mockito;
2730
import org.mockito.junit.MockitoJUnitRunner;
2831

32+
import java.util.ArrayList;
33+
import java.util.Arrays;
34+
import java.util.List;
35+
2936
@RunWith(MockitoJUnitRunner.class)
3037
public class ImageStoreProviderManagerImplTest {
3138

3239
@Mock
3340
ImageStoreDao imageStoreDao;
3441

42+
@Mock
43+
StatsCollector statsCollectorMock;
44+
3545
@InjectMocks
3646
ImageStoreProviderManagerImpl imageStoreProviderManager = new ImageStoreProviderManagerImpl();
47+
3748
@Test
3849
public void testGetImageStoreZoneId() {
3950
final long storeId = 1L;
@@ -44,4 +55,56 @@ public void testGetImageStoreZoneId() {
4455
long value = imageStoreProviderManager.getImageStoreZoneId(storeId);
4556
Assert.assertEquals(zoneId, value);
4657
}
58+
59+
private Pair<List<DataStore>, List<DataStore>> prepareUnorderedAndOrderedImageStoresForCapacityTests(boolean hasStoragesWithEnoughCapacity) {
60+
DataStore store1 = Mockito.mock(DataStore.class);
61+
Mockito.doReturn(100L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store1);
62+
Mockito.doReturn(false).when(statsCollectorMock).imageStoreHasEnoughCapacity(store1);
63+
DataStore store2 = Mockito.mock(DataStore.class);
64+
Mockito.doReturn(200L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store2);
65+
Mockito.doReturn(hasStoragesWithEnoughCapacity).when(statsCollectorMock).imageStoreHasEnoughCapacity(store2);
66+
DataStore store3 = Mockito.mock(DataStore.class);
67+
Mockito.doReturn(300L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store3);
68+
Mockito.doReturn(hasStoragesWithEnoughCapacity).when(statsCollectorMock).imageStoreHasEnoughCapacity(store3);
69+
DataStore store4 = Mockito.mock(DataStore.class);
70+
Mockito.doReturn(400L).when(statsCollectorMock).imageStoreCurrentFreeCapacity(store4);
71+
Mockito.doReturn(false).when(statsCollectorMock).imageStoreHasEnoughCapacity(store4);
72+
73+
List<DataStore> unordered = Arrays.asList(store1, store2, store3, store4);
74+
List<DataStore> orderedAndEnoughCapacity = new ArrayList<>();
75+
if (hasStoragesWithEnoughCapacity) {
76+
orderedAndEnoughCapacity.add(store3);
77+
orderedAndEnoughCapacity.add(store2);
78+
}
79+
80+
return new Pair<>(unordered, orderedAndEnoughCapacity);
81+
}
82+
83+
@Test
84+
public void getImageStoreWithFreeCapacityTestImageStoresWithEnoughCapacityExistReturnsImageStoreWithMostFreeCapacity() {
85+
Pair<List<DataStore>, List<DataStore>> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(true);
86+
87+
DataStore result = imageStoreProviderManager.getImageStoreWithFreeCapacity(unorderedAndOrdered.first());
88+
89+
Assert.assertEquals(unorderedAndOrdered.second().get(0), result);
90+
}
91+
92+
@Test
93+
public void getImageStoreWithFreeCapacityTestImageStoresWithEnoughCapacityDoNotExistReturnsNull() {
94+
Pair<List<DataStore>, List<DataStore>> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(false);
95+
96+
DataStore result = imageStoreProviderManager.getImageStoreWithFreeCapacity(unorderedAndOrdered.first());
97+
98+
Assert.assertNull(result);
99+
}
100+
101+
@Test
102+
public void orderImageStoresOnFreeCapacityTestReturnsImageStoresOrderedFromMostToLeast() {
103+
Pair<List<DataStore>, List<DataStore>> unorderedAndOrdered = prepareUnorderedAndOrderedImageStoresForCapacityTests(true);
104+
105+
List<DataStore> result = imageStoreProviderManager.orderImageStoresOnFreeCapacity(unorderedAndOrdered.first());
106+
107+
Assert.assertEquals(unorderedAndOrdered.second(), result);
108+
}
109+
47110
}

0 commit comments

Comments
 (0)