[pve-devel] [PATCH-SERIES manager] small cleanups for scheduler/jobs

[pve-devel] [PATCH-SERIES manager] small cleanups for scheduler/jobs

[Fabian Ebner]

Can also wait until after the release. Just some very minor things
that popped up while looking through the patches.


Fabian Ebner (3):
  pvescheduler: simplify code for sleep time calculation
  jobs: started_job: rename variable to $msg
  jobs: rename function to better distinguish job state and task status

 PVE/Jobs.pm                 | 17 ++++++++---------
 PVE/Service/pvescheduler.pm | 22 ++++------------------
 2 files changed, 12 insertions(+), 27 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH manager 1/3] pvescheduler: simplify code for sleep time calculation

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Service/pvescheduler.pm | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/PVE/Service/pvescheduler.pm b/PVE/Service/pvescheduler.pm
index a8d548fb..86891827 100755
--- a/PVE/Service/pvescheduler.pm
+++ b/PVE/Service/pvescheduler.pm
@@ -25,20 +25,6 @@ my $finish_jobs = sub {
     }
 };
 
-my $get_sleep_time = sub {
-    my ($calculate_offset) = @_;
-    my $time = 60;
-
-    if ($calculate_offset) {
-	# try to run near minute boundaries, makes more sense to the user as he
-	# configures jobs with minute precision
-	my ($current_seconds) = localtime;
-	$time = (60 - $current_seconds) if (60 - $current_seconds >= 5);
-    }
-
-    return $time;
-};
-
 sub run {
     my ($self) = @_;
 
@@ -84,12 +70,12 @@ sub run {
 
 	$run_jobs->();
 
-	my $sleep_time;
+	my $sleep_time = 60;
 	if ($count >= 1000) {
-	    $sleep_time = $get_sleep_time->(1);
+	    # Job schedule has minute precision, so try running near the minute boundary.
+	    my ($current_seconds) = localtime;
+	    $sleep_time = (60 - $current_seconds) if (60 - $current_seconds >= 5);
 	    $count = 0;
-	} else {
-	    $sleep_time = $get_sleep_time->(0);
 	}
 
 	my $slept = 0; # SIGCHLD interrupts sleep, so we need to keep track
-- 
2.30.2

[pve-devel] [PATCH manager 2/3] jobs: started_job: rename variable to $msg

[Fabian Ebner]

it's not always an error, but can also be 'OK'.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Jobs.pm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm
index 2fe197d2..bfccf7f1 100644
--- a/PVE/Jobs.pm
+++ b/PVE/Jobs.pm
@@ -155,17 +155,18 @@ sub starting_job {
 }
 
 sub started_job {
-    my ($jobid, $type, $upid, $err) = @_;
+    my ($jobid, $type, $upid, $msg) = @_;
+
     lock_job_state($jobid, $type, sub {
 	my $state = read_job_state($jobid, $type);
 	return if !defined($state); # job was removed, do not update
 	die "unexpected state '$state->{state}'\n" if $state->{state} ne 'starting';
 
 	my $new_state;
-	if (defined($err)) {
+	if (defined($msg)) {
 	    $new_state = {
 		state => 'stopped',
-		msg => $err,
+		msg => $msg,
 		time => time(),
 	    };
 	} else {
-- 
2.30.2

[pve-devel] [PATCH manager 3/3] jobs: rename function to better distinguish job state and task status

[Fabian Ebner]

Also, default to 'internal error' if the task status within the lock
is undef, which shouldn't actually be possible.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Jobs.pm | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm
index bfccf7f1..b91d6158 100644
--- a/PVE/Jobs.pm
+++ b/PVE/Jobs.pm
@@ -54,7 +54,7 @@ sub lock_job_state {
     return $res;
 }
 
-my $get_job_status = sub {
+my $get_job_task_status = sub {
     my ($state) = @_;
 
     if (!defined($state->{upid})) {
@@ -82,17 +82,15 @@ sub update_job_stopped {
     my $state = read_job_state($jobid, $type);
     return if !defined($state) || $state->{state} ne 'started'; # removed or not started
 
-    if (defined($get_job_status->($state))) {
+    if (defined($get_job_task_status->($state))) {
 	lock_job_state($jobid, $type, sub {
 	    my $state = read_job_state($jobid, $type);
 	    return if !defined($state) || $state->{state} ne 'started'; # removed or not started
 
-	    my $status = $get_job_status->($state);
-
 	    my $new_state = {
 		state => 'stopped',
-		msg => $status,
-		upid => $state->{upid}
+		msg => $get_job_task_status->($state) // 'internal error',
+		upid => $state->{upid},
 	    };
 
 	    if ($state->{updated}) { # save updated time stamp
-- 
2.30.2

[pve-devel] applied-series: [PATCH-SERIES manager] small cleanups for scheduler/jobs

[Thomas Lamprecht]

On 11.11.21 16:17, Fabian Ebner wrote:
> Can also wait until after the release. Just some very minor things
> that popped up while looking through the patches.
> 
> 
> Fabian Ebner (3):
>   pvescheduler: simplify code for sleep time calculation
>   jobs: started_job: rename variable to $msg
>   jobs: rename function to better distinguish job state and task status
> 
>  PVE/Jobs.pm                 | 17 ++++++++---------
>  PVE/Service/pvescheduler.pm | 22 ++++------------------
>  2 files changed, 12 insertions(+), 27 deletions(-)
> 



applied all three patches, thanks!