jmsperu commented on code in PR #12847:
URL: https://github.com/apache/cloudstack/pull/12847#discussion_r4252037168
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -64,6 +65,37 @@
public class NASBackupProvider extends AdapterBase implements BackupProvider,
Configurable {
private static final Logger LOG =
LogManager.getLogger(NASBackupProvider.class);
+ public static final ConfigKey<Boolean> NASBackupParallelExecution = new
ConfigKey<>("Advanced", Boolean.class,
+ "backup.nas.parallel.execution.enabled",
+ "true",
+ "Let NAS take-backup commands run concurrently on a KVM host
instead of queueing behind every earlier command on that host "
+ + "and holding up every later one. Concurrency is bounded per host
by backup.nas.parallel.max.per.host. "
+ + "Disable to restore strictly sequential execution.",
+ true, ConfigKey.Scope.Zone);
+
+ public static final ConfigKey<Integer> NASBackupParallelMaxPerHost = new
ConfigKey<>("Advanced", Integer.class,
+ "backup.nas.parallel.max.per.host",
+ "2",
+ "Maximum number of NAS take-backup commands each management server
keeps in flight on one KVM host when parallel "
+ + "execution is enabled; further backups for that host wait on
that management server. The count is kept per management "
+ + "server, so with several management servers a host can see up to
this value times the number of management servers. "
+ + "Keep that product below the agent's worker thread count
(workers in agent.properties, default 5) so start, stop, "
+ + "reboot and migrate commands are never queued behind backups.",
+ true, ConfigKey.Scope.Zone);
+
+ public static final ConfigKey<Integer> NASBackupParallelQueueTimeout = new
ConfigKey<>("Advanced", Integer.class,
+ "backup.nas.parallel.queue.timeout",
+ "7200",
+ "Seconds a NAS take-backup may wait for a free per-host slot
before it fails.",
+ true, ConfigKey.Scope.Zone);
+
+ /** In-flight take-backup commands per host, so backups never occupy every
agent worker thread. */
+ private final ConcurrentHashMap<Long, HostBackupSlots> hostBackupSlots =
new ConcurrentHashMap<>();
Review Comment:
@abh1sar thank you, done in 1ad9593642. The per-host cap no longer lives in
memory: the backup's own `BackingUp` row is now the reservation of a host slot.
Under a DB backed `GlobalLock` per host (`nas-backup-host-<hostId>`), held only
for the count-and-insert step, the provider counts the NAS backups in flight on
that host from the `backups` table and creates the row only if the count is
below `backup.nas.parallel.max.per.host`. Every management server reads the
same rows under the same lock, so the limit now holds across all of them.
A few details:
- A backup is attributed to the host its VM is on (or was last on when
stopped), the same rule `getVMHypervisorHost` uses to pick the host.
- The slot is released when the row leaves `BackingUp`. A failed, timed out
or unreachable backup is now marked Failed and removed in a `finally` block
instead of staying `BackingUp`.
- `BackingUp` rows older than the TakeBackupCommand timeout plus 5 minutes
are not counted, so a management server that dies mid-backup cannot hold a slot
forever.
- A full host is retried with back-off until
`backup.nas.parallel.queue.timeout`; waiters on the same management server wake
as soon as one of its backups finishes.
No schema change (`BackupDao` gains `listByZoneAndStatus`), the settings and
defaults are unchanged, and with one management server it behaves as before.
`NASBackupProviderTest` now runs two providers against one shared mocked
database and checks the combined in-flight count never goes above the cap
(10/10 green, checkstyle clean). Ready for another look.
@Damans227 your lab test was on 3340496, before this change, so a quick
re-run on 1ad9593642 would be very welcome.
--
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]
