Skip to content

Commit 6e0eeb4

Browse files
committed
Address reviews
1 parent e756a3d commit 6e0eeb4

1 file changed

Lines changed: 43 additions & 35 deletions

File tree

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java‎

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,10 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
144144

145145
Integer numConcurrentCopyTasksPerSSVM = 2;
146146

147-
private final Map<Long, ThreadPoolExecutor> zoneExecutorMap = new HashMap<>();
148-
private final Map<Long, Integer> zonePendingWorkCountMap = new HashMap<>();
149-
private final Map<Long, ThreadPoolExecutor> zoneKvmIncrementalResourcesExecutorMap = new ConcurrentHashMap<>();
147+
private final Map<Long, ThreadPoolExecutor> zoneExecutorMapSecondaryStorageVm = new HashMap<>();
148+
private final Map<Long, Integer> zonePendingWorkCountMapSecondaryStorageVm = new HashMap<>();
149+
private final Map<Long, ThreadPoolExecutor> zoneExecutorMapKvmAgent = new ConcurrentHashMap<>();
150+
private final Map<Long, Integer> zonePendingWorkCountMapKvmAgent = new ConcurrentHashMap<>();
150151

151152
@Override
152153
public String getConfigComponentName() {
@@ -389,8 +390,8 @@ protected Map<Long, Pair<Long, Long>> migrateAway(
389390

390391
if (isKvmIncrementalSnapshot) {
391392
MigrateKvmIncrementalSnapshotTask task = new MigrateKvmIncrementalSnapshotTask(chosenFileForMigration, snapshotChains, srcDatastore, destDataStore, snapshotIdsToMigrate);
392-
futures.add(submitKvmIncrementalMigration(srcDatastore.getScope().getScopeId(), task));
393-
logger.debug("Incremental snapshot migration {} submitted to incremental pool.", chosenFileForMigration.getUuid());
393+
futures.add(submitMigrationOnKvmAgent(srcDatastore.getScope().getScopeId(), task));
394+
logger.debug("Incremental snapshot migration {} submitted to KVM Agent pool.", chosenFileForMigration.getUuid());
394395
} else {
395396
createMigrateDataTask(chosenFileForMigration, snapshotChains, templateChains, srcDatastore, destDataStore, futures);
396397
}
@@ -452,10 +453,17 @@ private void updateSnapshotsReference(DataStore destDataStore, MigrateBetweenSec
452453
}
453454
}
454455

455-
protected <T> Future<T> submitKvmIncrementalMigration(Long zoneId, Callable<T> task) {
456-
ThreadPoolExecutor threadPoolExecutor = zoneKvmIncrementalResourcesExecutorMap.computeIfAbsent(zoneId, id -> new ThreadPoolExecutor(1, 1, 0L,
457-
TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>()));
458-
return threadPoolExecutor.submit(task);
456+
protected <T> Future<T> submitMigrationOnKvmAgent(Long zoneId, Callable<T> task) {
457+
ThreadPoolExecutor executor;
458+
synchronized (zoneExecutorMapKvmAgent) {
459+
if (!zoneExecutorMapKvmAgent.containsKey(zoneId)) {
460+
zoneExecutorMapKvmAgent.put(zoneId, new ThreadPoolExecutor(1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>()));
461+
zonePendingWorkCountMapKvmAgent.put(zoneId, 0);
462+
}
463+
zonePendingWorkCountMapKvmAgent.merge(zoneId, 1, Integer::sum);
464+
executor = zoneExecutorMapKvmAgent.get(zoneId);
465+
}
466+
return executor.submit(task);
459467
}
460468

461469
private HostVO getAvailableHost(long zoneId) throws AgentUnavailableException, OperationTimedoutException {
@@ -482,14 +490,14 @@ private void createMigrateDataTask(DataObject chosenFileForMigration, Map<DataOb
482490
protected <T> Future<T> submit(Long zoneId, Callable<T> task) {
483491
ThreadPoolExecutor executor;
484492
synchronized (this) {
485-
if (!zoneExecutorMap.containsKey(zoneId)) {
486-
zoneExecutorMap.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM,
493+
if (!zoneExecutorMapSecondaryStorageVm.containsKey(zoneId)) {
494+
zoneExecutorMapSecondaryStorageVm.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM,
487495
30, TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)));
488-
zonePendingWorkCountMap.put(zoneId, 0);
496+
zonePendingWorkCountMapSecondaryStorageVm.put(zoneId, 0);
489497
}
490-
zonePendingWorkCountMap.merge(zoneId, 1, Integer::sum);
498+
zonePendingWorkCountMapSecondaryStorageVm.merge(zoneId, 1, Integer::sum);
491499
scaleExecutorIfNecessary(zoneId);
492-
executor = zoneExecutorMap.get(zoneId);
500+
executor = zoneExecutorMapSecondaryStorageVm.get(zoneId);
493501
}
494502
return executor.submit(task);
495503

@@ -498,7 +506,7 @@ protected <T> Future<T> submit(Long zoneId, Callable<T> task) {
498506
protected void scaleExecutorIfNecessary(Long zoneId) {
499507
long activeSsvms = migrationHelper.activeSSVMCount(zoneId);
500508
long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM;
501-
ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId);
509+
ThreadPoolExecutor executor = zoneExecutorMapSecondaryStorageVm.get(zoneId);
502510
if (totalJobs > executor.getCorePoolSize()) {
503511
logger.debug("Scaling up executor of zone [{}] from [{}] to [{}] threads.", zoneId, executor.getCorePoolSize(),
504512
totalJobs);
@@ -508,46 +516,46 @@ protected void scaleExecutorIfNecessary(Long zoneId) {
508516
}
509517

510518
protected synchronized void tryCleaningUpExecutor(Long zoneId) {
511-
if (!zoneExecutorMap.containsKey(zoneId)) {
519+
if (!zoneExecutorMapSecondaryStorageVm.containsKey(zoneId)) {
512520
logger.debug("No executor exists for zone [{}].", zoneId);
513521
return;
514522
}
515523

516-
zonePendingWorkCountMap.merge(zoneId, -1, Integer::sum);
517-
Integer pendingWorkCount = zonePendingWorkCountMap.get(zoneId);
524+
zonePendingWorkCountMapSecondaryStorageVm.merge(zoneId, -1, Integer::sum);
525+
Integer pendingWorkCount = zonePendingWorkCountMapSecondaryStorageVm.get(zoneId);
518526
if (pendingWorkCount > 0) {
519527
logger.debug("Not cleaning executor of zone [{}] yet, as there is [{}] pending work.", zoneId, pendingWorkCount);
520528
return;
521529
}
522530

523531
logger.debug("Cleaning executor of zone [{}].", zoneId);
524-
ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId);
525-
zoneExecutorMap.remove(zoneId);
532+
ThreadPoolExecutor executor = zoneExecutorMapSecondaryStorageVm.get(zoneId);
533+
zoneExecutorMapSecondaryStorageVm.remove(zoneId);
526534
executor.shutdown();
527535
}
528536

529-
protected void tryCleaningUpKvmIncrementalExecutor(Long zoneId) {
530-
if (!zoneKvmIncrementalResourcesExecutorMap.containsKey(zoneId)) {
531-
logger.debug("No executor for KVM incremental resources exists for zone [{}].", zoneId);
532-
return;
533-
}
534-
535-
synchronized (zoneKvmIncrementalResourcesExecutorMap) {
536-
ThreadPoolExecutor executor = zoneKvmIncrementalResourcesExecutorMap.get(zoneId);
537+
protected void tryCleaningUpKvmAgentExecutor(Long zoneId) {
538+
synchronized (zoneExecutorMapKvmAgent) {
539+
if (!zoneExecutorMapKvmAgent.containsKey(zoneId)) {
540+
logger.debug("No executor for KVM Agent exists for zone [{}].", zoneId);
541+
return;
542+
}
537543

538-
int activeTasks = executor.getActiveCount();
539-
if (activeTasks > 1) {
540-
logger.debug("Not cleaning executor for KVM incremental resources of zone [{}] yet, as there are [{}] active tasks.", zoneId, activeTasks);
544+
zonePendingWorkCountMapKvmAgent.merge(zoneId, -1, Integer::sum);
545+
Integer pendingWorkCount = zonePendingWorkCountMapKvmAgent.get(zoneId);
546+
if (pendingWorkCount > 0) {
547+
logger.debug("Not cleaning executor for KVM Agent of zone [{}] yet, as there is [{}] pending work.",
548+
zoneId, pendingWorkCount);
541549
return;
542550
}
543551

544-
logger.debug("Cleaning executor for KVM incremental resources of zone [{}].", zoneId);
545-
zoneKvmIncrementalResourcesExecutorMap.remove(zoneId);
552+
logger.debug("Cleaning executor for KVM Agent of zone [{}].", zoneId);
553+
ThreadPoolExecutor executor = zoneExecutorMapKvmAgent.get(zoneId);
554+
zoneExecutorMapKvmAgent.remove(zoneId);
546555
executor.shutdown();
547556
}
548557
}
549558

550-
551559
private MigrationResponse handleResponse(List<Future<DataObjectResult>> futures, MigrationPolicy migrationPolicy, String message, boolean success) {
552560
int successCount = 0;
553561
for (Future<DataObjectResult> future : futures) {
@@ -820,7 +828,7 @@ public DataObjectResult call() {
820828
result.setResult(e.toString());
821829
return result;
822830
} finally {
823-
tryCleaningUpKvmIncrementalExecutor(srcDataStore.getScope().getScopeId());
831+
tryCleaningUpKvmAgentExecutor(srcDataStore.getScope().getScopeId());
824832
}
825833
}
826834
}

0 commit comments

Comments
 (0)