]> git.proxmox.com Git - pve-manager.git/commitdiff
pvescheduler: reworking child pid tracking
authorDominik Csapak <d.csapak@proxmox.com>
Thu, 18 Nov 2021 13:28:30 +0000 (14:28 +0100)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Mon, 22 Nov 2021 16:19:12 +0000 (17:19 +0100)
previously, systemd timers were responsible for running replication jobs.
those timers would not restart if the previous one is still running.

though trying again while it is running does no harm really, it spams
the log with errors about not being able to acquire the correct lock

to fix this, we rework the handling of child processes such that we only
start one per loop if there is currently none running. for that,
introduce the types of forks we do and allow one child process per type
(for now, we have 'jobs' and 'replication' as types)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
PVE/Service/pvescheduler.pm

index d4f7370295b4312533ab5816a10d5771b0fcc5be..466cc59906237d32f650f0f09d27d2811b52d6f6 100755 (executable)
@@ -17,12 +17,16 @@ my $cmdline = [$0, @ARGV];
 my %daemon_options = (stop_wait_time => 180, max_workers => 0);
 my $daemon = __PACKAGE__->new('pvescheduler', $cmdline, %daemon_options);
 
+my @types = qw(replication jobs);
+
 my $finish_jobs = sub {
     my ($self) = @_;
-    foreach my $cpid (keys %{$self->{jobs}}) {
-       my $waitpid = waitpid($cpid, WNOHANG);
-       if (defined($waitpid) && ($waitpid == $cpid)) {
-           delete ($self->{jobs}->{$cpid});
+    for my $type (@types) {
+       if (my $cpid = $self->{jobs}->{$type}) {
+           my $waitpid = waitpid($cpid, WNOHANG);
+           if (defined($waitpid) && ($waitpid == $cpid)) {
+               $self->{jobs}->{$type} = undef;
+           }
        }
     }
 };
@@ -41,7 +45,11 @@ sub run {
     };
 
     my $fork = sub {
-       my ($sub) = @_;
+       my ($type, $sub) = @_;
+
+       # don't fork again if the previous iteration still runs
+       return if defined($self->{jobs}->{$type});
+
        my $child = fork();
        if (!defined($child)) {
            die "fork failed: $!\n";
@@ -56,16 +64,16 @@ sub run {
            POSIX::_exit(0);
        }
 
-       $jobs->{$child} = 1;
+       $jobs->{$type} = $child;
     };
 
     my $run_jobs = sub {
 
-       $fork->(sub {
+       $fork->('replication', sub {
            PVE::API2::Replication::run_jobs(undef, sub {}, 0, 1);
        });
 
-       $fork->(sub {
+       $fork->('jobs', sub {
            PVE::Jobs::run_jobs();
        });
     };
@@ -92,14 +100,16 @@ sub run {
        }
     }
 
-    # jobs have a lock timeout of 60s, wait a bit more for graceful termination
+    # replication jobs have a lock timeout of 60s, wait a bit more for graceful termination
     my $timeout = 0;
-    while (keys %$jobs > 0 && $timeout < 75) {
-       kill 'TERM', keys %$jobs;
-       $timeout += sleep(5);
+    for my $type (@types) {
+       while (defined($jobs->{$type}) && $timeout < 75) {
+           kill 'TERM', $jobs->{$type};
+           $timeout += sleep(5);
+       }
+       # ensure the rest gets stopped
+       kill 'KILL', $jobs->{$type} if defined($jobs->{$type});
     }
-    # ensure the rest gets stopped
-    kill 'KILL', keys %$jobs if (keys %$jobs > 0);
 }
 
 sub shutdown {
@@ -107,7 +117,9 @@ sub shutdown {
 
     syslog('info', 'got shutdown request, signal running jobs to stop');
 
-    kill 'TERM', keys %{$self->{jobs}};
+    for my $type (@types) {
+       kill 'TERM', $self->{jobs}->{$type} if $self->{jobs}->{$type};
+    }
     $self->{shutdown_request} = 1;
 }