[PATCH guest-common,manager,docs 0/5] backup jobs: add a start window

[PATCH guest-common,manager,docs 0/5] backup jobs: add a start window

[Thomas Lamprecht]

A backup job can run into business hours, for example when it has to
wait for other jobs first or backing up its guests takes longer than
planned, and nothing stops it from starting further guest backups then.

The new 'start-window' job option sets how many minutes after the
scheduled start guest backups may still begin. Running guest backups
finish, and guests not started in time are reported as skipped. The
window also bounds waiting for the global lock and for backup slots.
It counts from the scheduled time, so a missed run repeated later only
gets what is left of it.

This applies on top of the series "parallel guest backups and backup
concurrency limits" [0].

[0]: https://lore.proxmox.com/pve-devel/20261101225849.884587-1-t.lamprecht@proxmox.com/

Repo (build-)dependencies:
 pve-manager -> pve-guest-common

pve-guest-common must be released first, see the note on patch 3.

pve-guest-common:

Thomas Lamprecht (1):
      vzdump: add option to stop starting guest backups after a time window

 src/PVE/VZDump/Common.pm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

pve-manager:

Thomas Lamprecht (3):
      jobs: pass scheduled run time to job plugins
      vzdump: honor start window for guest backups
      ui: backup job: add start window option

 PVE/API2/Backup.pm                          |  2 +-
 PVE/API2/VZDump.pm                          | 18 ++++++--
 PVE/Jobs.pm                                 |  2 +-
 PVE/Jobs/VZDump.pm                          |  3 +-
 PVE/VZDump.pm                               | 51 +++++++++++++++++----
 www/manager6/panel/BackupAdvancedOptions.js | 21 +++++++++
 6 files changed, 81 insertions(+), 16 deletions(-)

pve-docs:

Thomas Lamprecht (1):
      vzdump: document the start window

 vzdump.adoc | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

-- 
2.47.3

[PATCH guest-common 1/5] vzdump: add option to stop starting guest backups after a time window

[Thomas Lamprecht]

A backup job can easily run into business hours, when the node has to
wait for other jobs first, or when backing up the guests takes longer
than planned. Nothing prevents starting further guests at that point.

The new 'start-window' option defines how many minutes after the
scheduled start of a job, or the start of a manual backup, guest
backups may still begin. Guest backups already running finish
normally, and guests not started in time are reported as skipped, so
a shortened run stays visible.

The job runner passes the scheduled start as 'job-scheduled-time'.
Like the job ID, it is left out of the command line shown in the task
log.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/VZDump/Common.pm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index f98c9ae..538405b 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -348,6 +348,15 @@ my $confdesc = {
         optional => 1,
         default => 0,
     },
+    'start-window' => {
+        type => 'integer',
+        description => "Do not start backing up further guests once this many minutes have"
+            . " passed since the scheduled start of a backup job, or since the start of a manual"
+            . " backup. Running guest backups finish, and the remaining guests are reported as"
+            . " skipped.",
+        optional => 1,
+        minimum => 1,
+    },
     stopwait => {
         type => 'integer',
         description => "Maximal time to wait until a guest system is stopped (minutes).",
@@ -554,7 +563,8 @@ sub command_line {
             || $p eq 'dow'
             || $p eq 'stdout'
             || $p eq 'enabled'
-            || $p eq 'job-id';
+            || $p eq 'job-id'
+            || $p eq 'job-scheduled-time';
         my $v = $param->{$p};
         my $pd = $confdesc->{$p} || die "no such vzdump option '$p'\n";
         if ($p eq 'exclude-path') {
-- 
2.47.3

[PATCH manager 2/5] jobs: pass scheduled run time to job plugins

[Thomas Lamprecht]

Job plugins only got the schedule, so they could not tell how late a
run actually starts, for example for a missed run repeated after the
node was down. Pass the time the run was scheduled for as well.

Existing plugins ignore the additional argument.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 PVE/Jobs.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm
index 22e947f9c..b33b52b57 100644
--- a/PVE/Jobs.pm
+++ b/PVE/Jobs.pm
@@ -326,7 +326,7 @@ sub run_jobs {
         if (starting_job($id, $type)) {
             PVE::Cluster::cfs_update();
 
-            my $upid = eval { $plugin->run($cfg, $id, $schedule) };
+            my $upid = eval { $plugin->run($cfg, $id, $schedule, $next_sync) };
             if (my $err = $@) {
                 warn $@ if $@;
                 started_job($id, $type, undef, $err);
-- 
2.47.3

[PATCH manager 3/5] vzdump: honor start window for guest backups

[Thomas Lamprecht]

A backup job can now limit how long after its scheduled start guest
backups may still begin, with the new 'start-window' option. For
manual backups, the window starts with the task.

The window also bounds waiting for the global lock and for backup
slots, and a job whose window ended before it got the lock does not
start at all. Guests not backed up because of this are listed as
skipped in the task log and in the notification, which then reports a
failure.

The job runner passes the scheduled time along, so a run that starts
late, for example a missed one repeated after the node was down, only
gets what is left of its window.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

Notes:
    NOTE: needs pve-guest-common with patch 1. Older versions die on the
    'job-scheduled-time' parameter that scheduled jobs now pass, so every
    scheduled backup would fail.

 PVE/API2/Backup.pm |  2 +-
 PVE/API2/VZDump.pm | 18 ++++++++++++----
 PVE/Jobs/VZDump.pm |  3 ++-
 PVE/VZDump.pm      | 51 ++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 3bd855939..f9ad25065 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -40,7 +40,7 @@ sub assert_param_permission_common {
     my ($rpcenv, $user, $param, $is_delete) = @_;
     return if $user eq 'root@pam'; # always OK
 
-    for my $key (qw(tmpdir dumpdir script job-id)) {
+    for my $key (qw(tmpdir dumpdir script job-id job-scheduled-time)) {
         raise_param_exc({ $key => "Only root may set this option." }) if exists $param->{$key};
     }
 
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 3e31505f3..ab45f04ab 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -42,10 +42,11 @@ __PACKAGE__->register_method({
     permissions => {
         description => "The user needs 'VM.Backup' permissions on any VM, and"
             . " 'Datastore.AllocateSpace' on the backup storage (and fleecing storage when fleecing"
-            . " is used). The 'tmpdir', 'dumpdir', 'script' and 'job-id' parameters are restricted"
-            . " to the 'root\@pam' user. The 'prune-backups' setting requires 'Datastore.Allocate'"
-            . " on the backup storage. The 'bwlimit', 'performance' and 'ionice' parameters require"
-            . " 'Sys.Modify' on '/'. The 'stop' parameter requires 'Sys.Modify' on '/nodes/{node}'",
+            . " is used). The 'tmpdir', 'dumpdir', 'script', 'job-id' and 'job-scheduled-time'"
+            . " parameters are restricted to the 'root\@pam' user. The 'prune-backups' setting"
+            . " requires 'Datastore.Allocate' on the backup storage. The 'bwlimit', 'performance'"
+            . " and 'ionice' parameters require 'Sys.Modify' on '/'. The 'stop' parameter requires"
+            . " 'Sys.Modify' on '/nodes/{node}'",
         user => 'all',
     },
     protected => 1,
@@ -63,6 +64,15 @@ __PACKAGE__->register_method({
                     optional => 1,
                 },
             ),
+            'job-scheduled-time' => {
+                type => 'integer',
+                description =>
+                    "The time the backup job was scheduled for, as UNIX epoch. Used as start"
+                    . " of the 'start-window' instead of the current time. Only root\@pam can set"
+                    . " this parameter.",
+                optional => 1,
+                minimum => 0,
+            },
             stdout => {
                 type => 'boolean',
                 description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index d7a2e08dd..fe3e0a0ec 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
 use base qw(PVE::VZDump::JobBase);
 
 sub run {
-    my ($class, $conf, $job_id) = @_;
+    my ($class, $conf, $job_id, $schedule, $scheduled_time) = @_;
 
     my $props = $class->properties();
     # remove all non vzdump related options
@@ -21,6 +21,7 @@ sub run {
     }
 
     $conf->{'job-id'} = $job_id;
+    $conf->{'job-scheduled-time'} = $scheduled_time if defined($scheduled_time);
 
     # Required as string parameters # FIXME why?! we could just check ref()
     for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 59809d85c..417cf9e53 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -692,9 +692,20 @@ sub new {
     };
     $self->{exclusive} = !$opts->{overlap} || $uses_provider;
 
+    if (my $window = $opts->{'start-window'}) {
+        $self->{'start-deadline'} = ($opts->{'job-scheduled-time'} // time()) + $window * 60;
+    }
+
     return $self;
 }
 
+sub start_window_ended {
+    my ($self) = @_;
+
+    my $deadline = $self->{'start-deadline'};
+    return defined($deadline) && time() >= $deadline;
+}
+
 sub init_job {
     my ($self) = @_;
 
@@ -788,7 +799,19 @@ sub getlock {
 
     my $fh;
 
-    my $maxwait = $self->{opts}->{lockwait} || $self->{lockwait};
+    my $maxwait = ($self->{opts}->{lockwait} || $self->{lockwait} || 0) * 60;
+    my $window_err = "start window ended before the job could start\n";
+    my $timeout_err = "got timeout\n";
+
+    if (defined(my $deadline = $self->{'start-deadline'})) {
+        my $left = $deadline - time();
+        if ($left <= 0) {
+            debugmsg('err', $window_err, undef, 1);
+            die $window_err;
+        } elsif ($left < $maxwait) {
+            ($maxwait, $timeout_err) = ($left, $window_err);
+        }
+    }
 
     die "missing UPID" if !$upid; # should not happen
     $self->{upid} = $upid;
@@ -824,9 +847,9 @@ sub getlock {
 
         debugmsg('info', "trying to get global lock - waiting...", undef, 1);
         eval {
-            alarm($maxwait * 60);
+            alarm($maxwait);
 
-            local $SIG{ALRM} = sub { alarm(0); die "got timeout\n"; };
+            local $SIG{ALRM} = sub { alarm(0); die $timeout_err; };
 
             if (!$acquire->(0)) {
                 my $err = $!;
@@ -841,8 +864,9 @@ sub getlock {
         my $err = $@;
 
         if ($err) {
-            debugmsg('err', "can't acquire lock '$lockfile' - $err", undef, 1);
-            die "can't acquire lock '$lockfile' - $err";
+            $err = "can't acquire lock '$lockfile' - $err" if $err ne $window_err;
+            debugmsg('err', $err, undef, 1);
+            die $err;
         }
 
         debugmsg('info', "got global lock", undef, 1);
@@ -1791,6 +1815,11 @@ my sub guest_moved_to {
 my sub claim_next_guest {
     my ($self, $queue, $deferred) = @_;
 
+    if ($self->start_window_ended()) {
+        skip_task($_, 'start window ended') for splice($queue->@*);
+        return;
+    }
+
     my $maxwait = ($self->{opts}->{lockwait} || 0) * 60;
 
     for (my $i = 0; $i < scalar($queue->@*); $i++) {
@@ -1935,7 +1964,7 @@ my sub next_guest_to_start {
         $pending->{'request-error'} = $err;
     } elsif (!defined($granted)) {
         $pending->{key} = undef; # vanished, for example if released by an admin
-    } elsif ($granted) {
+    } elsif ($granted && !$self->start_window_ended()) {
         delete $schedule->{pending};
         $schedule->{'cluster-wait'} += time() - $pending->{since};
         if (my $node = guest_moved_to($task->{vmid})) {
@@ -1948,14 +1977,18 @@ my sub next_guest_to_start {
         delete $pending->{'request-error'};
     }
 
+    my $window_ended = $self->start_window_ended();
     my $waited = $schedule->{'cluster-wait'} + time() - $pending->{since};
     my $maxwait = ($self->{opts}->{lockwait} || 0) * 60;
-    if ($waited >= $maxwait) {
+    if ($window_ended || $waited >= $maxwait) {
         release_cluster_slots($pending->{key});
         delete $schedule->{pending};
         $schedule->{'cluster-wait'} = $waited;
-        my $reason = 'timeout waiting for cluster-wide backup slot';
-        $reason .= " - $pending->{'request-error'}" if defined($pending->{'request-error'});
+        my $reason = 'start window ended';
+        if (!$window_ended) {
+            $reason = 'timeout waiting for cluster-wide backup slot';
+            $reason .= " - $pending->{'request-error'}" if defined($pending->{'request-error'});
+        }
         skip_task($task, $reason);
     } elsif (defined($pending->{key}) && !$pending->{logged}++) {
         my $reached = eval { PVE::VZDump::ClusterSlots::reached_limits($pending->{key}) };
-- 
2.47.3

[PATCH manager 4/5] ui: backup job: add start window option

[Thomas Lamprecht]

Add the start window to the advanced tab of backup jobs. Like the
other options there, an empty field uses the fallback from the node's
vzdump.conf.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 www/manager6/panel/BackupAdvancedOptions.js | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/www/manager6/panel/BackupAdvancedOptions.js b/www/manager6/panel/BackupAdvancedOptions.js
index 8af0fa77e..ed6ac05cc 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -311,6 +311,27 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
                 ),
             },
         },
+        {
+            xtype: 'pveTwoColumnContainer',
+            startColumn: {
+                xtype: 'proxmoxintegerfield',
+                name: 'start-window',
+                minValue: 1,
+                fieldLabel: gettext('Start Window'),
+                fieldStyle: 'text-align: right',
+                emptyText: gettext('Fallback'),
+                cbind: {
+                    deleteEmpty: '{!isCreate}',
+                },
+            },
+            endFlex: 2,
+            endColumn: {
+                xtype: 'displayfield',
+                value: gettext(
+                    'Minutes after the scheduled start in which guest backups may still begin. Guests not started by then are reported as skipped.',
+                ),
+            },
+        },
         {
             xtype: 'pveTwoColumnContainer',
             startColumn: {
-- 
2.47.3

[PATCH docs 5/5] vzdump: document the start window

[Thomas Lamprecht]

Describe the new 'start-window' job option, and that it also ends
waiting for backup slots.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 vzdump.adoc | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index 6e65c29..d3ead93 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -351,7 +351,8 @@ time as other jobs that have it enabled, for example, a short hourly job
 alongside a long nightly one. Together, they stay within the backup concurrency
 of the node. A job without the option still runs alone, and while it waits for
 running jobs to finish, overlapping jobs that are due later do not start before
-it. Waiting for a free backup slot of the node is not limited by `lockwait`.
+it. Waiting for a free backup slot of the node is only limited by the start
+window, not by `lockwait`.
 
 If two running jobs include the same guest, the second one backs it up once the
 first one is done with it, waiting at most `lockwait` minutes.
@@ -361,6 +362,17 @@ example, a hook script that mounts a disk in the `job-start` phase and unmounts
 it in the `job-end` phase must not be used for overlapping jobs. With a job
 concurrency above one, the phases of different guests of a job overlap as well.
 
+Start Window
+~~~~~~~~~~~~
+
+The `start-window` job option ('Start Window' in the 'Advanced' tab) sets how
+many minutes after the scheduled start of a job guest backups may still begin.
+For manual backups, the window starts with the backup task. Guest backups
+running when the window ends finish normally, and guests that did not start in
+time are reported as skipped in the task log and in the notification. This
+keeps further guest backups from starting during business hours, also if a
+missed job is repeated later.
+
 Cluster-wide and Storage Limits
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -394,9 +406,9 @@ count separately, so use the cluster-wide limit to bound their sum.
 Guest backups get cluster-wide slots in the order they started waiting. One that
 waits for a storage at its limit lets others with room go first, but only a few
 times. A job has at most one guest waiting for cluster-wide slots at a time, and
-waits for them at most `lockwait` minutes in total. Like for node slots, waiting
-for free slots of a local storage is not limited by `lockwait`. The task log
-tells which limit a guest waits for.
+waits for them at most `lockwait` minutes in total, or until its start window
+ended. Like for node slots, waiting for free slots of a local storage is only
+limited by the start window. The task log tells which limit a guest waits for.
 To see which guest backups hold or wait for cluster-wide slots, use:
 
 ----
-- 
2.47.3