Damans227 commented on code in PR #12847:
URL: https://github.com/apache/cloudstack/pull/12847#discussion_r4337592197
##########
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:
that covers it, the longer wait and keeping the backup on a timeout in
8c441c5 fix both parts.
--
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]
