Damans227 commented on code in PR #12847:
URL: https://github.com/apache/cloudstack/pull/12847#discussion_r4252434799
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -167,26 +210,188 @@ public boolean takeBackup(final VirtualMachine vm) {
command.setVolumePaths(volumePaths);
}
- BackupAnswer answer = null;
+ // Concurrent backups are bounded per host across all management
servers: the BackingUp row of this
+ // backup is the reservation of one of the host's slots, created only
while the host has a free slot.
+ // The wait happens here, on the management server, never on the
agent's worker threads.
+ final BackupVO backupVO = parallel
+ ? reserveHostBackupSlot(vm, host.getId(), backupPath,
NASBackupParallelMaxPerHost.valueIn(zoneId),
NASBackupParallelQueueTimeout.valueIn(zoneId))
+ : createBackupObject(vm, backupPath);
+ boolean succeeded = false;
try {
- answer = (BackupAnswer) agentManager.send(host.getId(), command);
- } catch (AgentUnavailableException e) {
- throw new CloudRuntimeException("Unable to contact backend control
plane to initiate backup");
- } catch (OperationTimedoutException e) {
- throw new CloudRuntimeException("Operation to initiate backup
timed out, please try again");
- }
-
- if (answer != null && answer.getResult()) {
+ final BackupAnswer answer;
+ try {
+ answer = (BackupAnswer) agentManager.send(host.getId(),
command);
+ } catch (AgentUnavailableException e) {
+ throw new CloudRuntimeException("Unable to contact backend
control plane to initiate backup");
+ } catch (OperationTimedoutException e) {
+ throw new CloudRuntimeException("Operation to initiate backup
timed out, please try again");
+ }
+ if (answer == null || !answer.getResult()) {
+ return false;
+ }
+ succeeded = true;
backupVO.setDate(new Date());
backupVO.setSize(answer.getSize());
backupVO.setStatus(Backup.Status.BackedUp);
backupVO.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
return backupDao.update(backupVO.getId(), backupVO);
- } else {
- backupVO.setStatus(Backup.Status.Failed);
- backupDao.remove(backupVO.getId());
+ } finally {
+ if (!succeeded) {
+ // Releases the slot: a failed or interrupted backup must
never stay BackingUp.
Review Comment:
what happens when a backup runs past the 30 min wait? the host keeps going
but the record is removed here, so the slot frees up early and the backup goes
missing from the list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
