[pve-devel] [PATCH manager/docs] handle missed jobs better

[pve-devel] [PATCH manager/docs] handle missed jobs better

[Dominik Csapak]

by adding a config option 'skip-missed' for jobs that skips the jobs
on pvescheduler start and when changing from disabled -> enabled

i did not use the systemd name 'persistent' since the name does
not really convey what it means, so i tried to come up with
a better name for it (the logic is just reversed from the systemd
variant, and our default is 0)

pve-manager:

Dominik Csapak (3):
  fix #4026: add 'skip-missed' option for jobs
  fix #4053: enable 'skip-missed' behaviour also for change from
    disabled->enabled
  ui: dc/Backup: add 'skip-missed' checkbox

 PVE/API2/Backup.pm          | 33 +++++++++++++++++++++++++++++----
 PVE/Jobs.pm                 |  5 +++++
 PVE/Jobs/Plugin.pm          |  6 ++++++
 PVE/Jobs/VZDump.pm          |  1 +
 PVE/Service/pvescheduler.pm |  6 +++++-
 www/manager6/dc/Backup.js   |  9 +++++++++
 6 files changed, 55 insertions(+), 5 deletions(-)

pve-docs:

Dominik Csapak (1):
  vzdump: add section about 'skip-missed'

 vzdump.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.30.2

[pve-devel] [PATCH manager 1/3] fix #4026: add 'skip-missed' option for jobs

[Dominik Csapak]

like an inverse of systemd-timers 'persistent'.
so that the user can configure it to not be run after powering up
when it was previously missed

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Backup.pm          | 19 ++++++++++++++++++-
 PVE/Jobs.pm                 |  5 +++++
 PVE/Jobs/Plugin.pm          |  6 ++++++
 PVE/Jobs/VZDump.pm          |  1 +
 PVE/Service/pvescheduler.pm |  6 +++++-
 5 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 5d36789a..7b00567a 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -180,6 +180,12 @@ __PACKAGE__->register_method({
 		description => "Enable or disable the job.",
 		default => '1',
 	    },
+	    'skip-missed' => {
+		optional => 1,
+		type => 'boolean',
+		description => "If true, the job will not be run after boot if it was missed earlier.",
+		default => 0,
+	    },
 	    comment => {
 		optional => 1,
 		type => 'string',
@@ -381,6 +387,12 @@ __PACKAGE__->register_method({
 		description => "Enable or disable the job.",
 		default => '1',
 	    },
+	    'skip-missed' => {
+		optional => 1,
+		type => 'boolean',
+		description => "If true, the job will not be run after boot if it was missed earlier.",
+		default => 0,
+	    },
 	    comment => {
 		optional => 1,
 		type => 'string',
@@ -440,8 +452,13 @@ __PACKAGE__->register_method({
 		die "no such vzdump job\n" if !$job || $job->{type} ne 'vzdump';
 	    }
 
+	    my $deletable = {
+		comment => 1,
+		'skip-missed' => 1,
+	    };
+
 	    foreach my $k (@$delete) {
-		if (!PVE::VZDump::option_exists($k) && $k ne 'comment') {
+		if (!PVE::VZDump::option_exists($k) && !$deletable->{$k}) {
 		    raise_param_exc({ delete => "unknown option '$k'" });
 		}
 
diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm
index da648630..299fddd6 100644
--- a/PVE/Jobs.pm
+++ b/PVE/Jobs.pm
@@ -209,6 +209,7 @@ sub get_last_runtime {
 }
 
 sub run_jobs {
+    my ($first_run) = @_;
     synchronize_job_states_with_config();
 
     my $jobs_cfg = cfs_read_file('jobs.cfg');
@@ -228,6 +229,10 @@ sub run_jobs {
 	    next;
 	}
 
+	# update last_run_time on the first run when 'skip-missed' is 1, so that a missed job will not
+	# start immediately after boot
+	updated_job_schedule($id, $type) if $first_run && $cfg->{'skip-missed'};
+
 	next if defined($cfg->{enabled}) && !$cfg->{enabled}; # only schedule actually enabled jobs
 
 	my $last_run = get_last_runtime($id, $type);
diff --git a/PVE/Jobs/Plugin.pm b/PVE/Jobs/Plugin.pm
index 6098360b..f5111442 100644
--- a/PVE/Jobs/Plugin.pm
+++ b/PVE/Jobs/Plugin.pm
@@ -39,6 +39,12 @@ my $defaultData = {
 	    description => "Description for the Job.",
 	    maxLength => 512,
 	},
+	'skip-missed' => {
+	    optional => 1,
+	    type => 'boolean',
+	    description => "If true, the job will not be run after boot if it was missed earlier.",
+	    default => 0,
+	},
     },
 };
 
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index 44fe33dc..70e1e130 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -26,6 +26,7 @@ sub options {
 	enabled => { optional => 1 },
 	schedule => {},
 	comment => { optional => 1 },
+	'skip-missed' => { optional => 1 },
     };
     foreach my $opt (keys %$props) {
 	if ($props->{$opt}->{optional}) {
diff --git a/PVE/Service/pvescheduler.pm b/PVE/Service/pvescheduler.pm
index f05f3bb9..40be5977 100755
--- a/PVE/Service/pvescheduler.pm
+++ b/PVE/Service/pvescheduler.pm
@@ -97,6 +97,8 @@ sub run {
 	$jobs->{$type}->{$child} = 1;
     };
 
+    my $first_run = 1;
+
     my $run_jobs = sub {
 	# TODO: actually integrate replication in PVE::Jobs and do not always fork here, we could
 	# do the state lookup and check if there's new work scheduled before doing so, e.g., by
@@ -109,8 +111,10 @@ sub run {
 	});
 
 	$fork->('jobs', sub {
-	    PVE::Jobs::run_jobs();
+	    PVE::Jobs::run_jobs($first_run);
 	});
+
+	$first_run = 0;
     };
 
     PVE::Jobs::setup_dirs();
-- 
2.30.2

[pve-devel] [PATCH manager 2/3] fix #4053: enable 'skip-missed' behaviour also for change from disabled->enabled

[Dominik Csapak]

so that if 'persistent' is configured to off, we don't immediately run
the job

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Backup.pm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 7b00567a..251a892d 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -452,6 +452,8 @@ __PACKAGE__->register_method({
 		die "no such vzdump job\n" if !$job || $job->{type} ne 'vzdump';
 	    }
 
+	    my $old_enabled = $job->{enabled} // 1;
+
 	    my $deletable = {
 		comment => 1,
 		'skip-missed' => 1,
@@ -465,15 +467,21 @@ __PACKAGE__->register_method({
 		delete $job->{$k};
 	    }
 
-	    my $schedule_updated = 0;
+	    my $need_run_time_update = 0;
 	    if (defined($param->{schedule}) && $param->{schedule} ne $job->{schedule}) {
-		$schedule_updated = 1;
+		$need_run_time_update = 1;
 	    }
 
 	    foreach my $k (keys %$param) {
 		$job->{$k} = $param->{$k};
 	    }
 
+	    my $new_enabled = $job->{enabled} // 1;
+
+	    if ($new_enabled && !$old_enabled && $job->{'skip-missed'}) {
+		$need_run_time_update = 1;
+	    }
+
 	    $job->{all} = 1 if (defined($job->{exclude}) && !defined($job->{pool}));
 
 	    if (defined($param->{vmid})) {
@@ -491,7 +499,7 @@ __PACKAGE__->register_method({
 
 	    PVE::VZDump::verify_vzdump_parameters($job, 1);
 
-	    if ($schedule_updated) {
+	    if ($need_run_time_update) {
 		PVE::Jobs::updated_job_schedule($id, 'vzdump');
 	    }
 
-- 
2.30.2

[pve-devel] [PATCH manager 3/3] ui: dc/Backup: add 'skip-missed' checkbox

[Dominik Csapak]

so that the users can configure how to handle missed job runs

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/dc/Backup.js | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 3494aa54..6a7dc6b1 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -182,6 +182,14 @@ Ext.define('PVE.dc.BackupEdit', {
 	    },
 	    selModeField,
 	    selPool,
+	    {
+		xtype: 'proxmoxcheckbox',
+		fieldLabel: gettext('Skip missed'),
+		name: 'skip-missed',
+		uncheckedValue: 0,
+		defaultValue: 0,
+		deleteDefaultValue: 1,
+	    },
 	];
 
 	let column2 = [
@@ -581,6 +589,7 @@ Ext.define('PVE.dc.BackupView', {
 	    delete job.node;
 	    delete job.comment;
 	    delete job['next-run'];
+	    delete job['skip-missed'];
 	    job.all = job.all === true ? 1 : 0;
 
 	    if (job['prune-backups']) {
-- 
2.30.2

[pve-devel] [PATCH docs 1/1] vzdump: add section about 'skip-missed'

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 vzdump.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/vzdump.adoc b/vzdump.adoc
index 544ed5e..f475b50 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -54,6 +54,12 @@ will in turn be parsed and executed by the `pvescheduler` daemon.
 These jobs use the xref:chapter_calendar_events[calendar events] for
 defining the schedule.
 
+Since scheduled backups miss their execution when the host was offline or the
+job was disabled during the scheduled time, it is possible to configure the
+behaviour for catching up. By enabling the `Skip missed` option (`skip-missed`
+in the config), you can tell the scheduler that it should not catch up the
+missed runs in those two cases.
+
 Backup modes
 ------------
 
-- 
2.30.2

Re: [pve-devel] [PATCH manager/docs] handle missed jobs better

[Thomas Lamprecht]

Am 01/06/2022 um 12:23 schrieb Dominik Csapak:
> by adding a config option 'skip-missed' for jobs that skips the jobs
> on pvescheduler start and when changing from disabled -> enabled
> 
> i did not use the systemd name 'persistent' since the name does
> not really convey what it means, so i tried to come up with
> a better name for it (the logic is just reversed from the systemd
> variant, and our default is 0)
> 

for the record the off list exchange:

* skipping immediate "catch up" run on disable->enable change should be unconditionally
  off

* "skip-missed" is somewhat double negative, rather negate that and use "persistent",
   like systemd does, or "repeat-missed"

* the behaviour should then be off by default to avoid triggering lots of backups during
  boot up.

* I'd move the box into advanced section in the gui