[pve-devel] [PATCH manager] fix #3731: pvescheduler: Improve fork behaviour

[pve-devel] [PATCH manager] fix #3731: pvescheduler: Improve fork behaviour

[Filip Schauer]

Instead of forking every minute, check whether a job has to be run and
only fork if there is a pending job.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
Backup jobs are registered in the jobs.cfg file, while replication jobs
are registered in the replication.cfg file. Integrating replication into
PVE::Jobs would move replication jobs into jobs.cfg, breaking backwards
compatibility.

 PVE/Jobs.pm                 | 21 ++++++++++++++++++++-
 PVE/Service/pvescheduler.pm | 11 ++++-------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm
index bd323332..5643bd15 100644
--- a/PVE/Jobs.pm
+++ b/PVE/Jobs.pm
@@ -258,13 +258,14 @@ sub get_last_runtime {
     return $state->{time} // 0;
 }
 
-sub run_jobs {
+sub get_pending {
     my ($first_run) = @_;
 
     synchronize_job_states_with_config();
 
     my $jobs_cfg = cfs_read_file('jobs.cfg');
     my $nodename = PVE::INotify::nodename();
+    my @pending_jobs = ();
 
     foreach my $id (sort keys %{$jobs_cfg->{ids}}) {
 	my $cfg = $jobs_cfg->{ids}->{$id};
@@ -292,6 +293,24 @@ sub run_jobs {
 
 	next if !defined($next_sync) || time() < $next_sync; # not yet its (next) turn
 
+	push @pending_jobs, $id;
+    }
+
+    return @pending_jobs;
+}
+
+sub run_jobs {
+    my (@scheduled_jobs) = @_;
+
+    synchronize_job_states_with_config();
+
+    my $jobs_cfg = cfs_read_file('jobs.cfg');
+
+    foreach my $id (@scheduled_jobs) {
+	my $cfg = $jobs_cfg->{ids}->{$id} or next;
+	my $type = $cfg->{type};
+	my $schedule = delete $cfg->{schedule};
+
 	my $plugin = PVE::Job::Registry->lookup($type);
 	if (starting_job($id, $type)) {
 	    PVE::Cluster::cfs_update();
diff --git a/PVE/Service/pvescheduler.pm b/PVE/Service/pvescheduler.pm
index 40be5977..7b6b3e99 100755
--- a/PVE/Service/pvescheduler.pm
+++ b/PVE/Service/pvescheduler.pm
@@ -100,19 +100,16 @@ sub run {
     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
-	# extending the PVE::Jobs interfacae e.g.;
-	# my $scheduled_jobs = PVE::Jobs::get_pending() or return;
-	# forked { PVE::Jobs::run_jobs($scheduled_jobs) }
+	# TODO: actually integrate replication in PVE::Jobs and do not always fork here
 
 	$fork->('replication', sub {
 	    PVE::API2::Replication::run_jobs(undef, sub {}, 0, 1);
 	});
 
+	my @pending_jobs = PVE::Jobs::get_pending($first_run);
 	$fork->('jobs', sub {
-	    PVE::Jobs::run_jobs($first_run);
-	});
+	    PVE::Jobs::run_jobs(@pending_jobs);
+	}) if (@pending_jobs);
 
 	$first_run = 0;
     };
-- 
2.39.2