[pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant

[pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant

[Dominik Csapak]

when a user updates a job schedule, we want to save that point in time
to calculate future runs, otherwise when a user updates a schedule to
a time that would have been between the last run and 'now' the
schedule is triggered instantly

for example:
schedule 08:00
last run today 08:00
now it is 12:00

before this patch:
update schedule to 11:00
 -> triggered instantly since we calculate from 08:00

after this patch:
update schedule to 11:00
 -> triggered tomorrow 11:00 since we calculate from today 12:00

the change in the enum type is ok, since by default serde does not
error on unknown fields and the new field is optional

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/config/sync.rs            |  5 ++
 src/api2/config/tape_backup_job.rs |  5 ++
 src/api2/config/verify.rs          |  5 ++
 src/server/jobstate.rs             | 82 ++++++++++++++++++++++++------
 4 files changed, 82 insertions(+), 15 deletions(-)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 00a8c0b3..aa8369fd 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -333,6 +333,7 @@ pub fn update_sync_job(
     if let Some(remote_store) = remote_store { data.remote_store = remote_store; }
     if let Some(owner) = owner { data.owner = Some(owner); }
 
+    let schedule_changed = data.schedule != schedule;
     if schedule.is_some() { data.schedule = schedule; }
     if remove_vanished.is_some() { data.remove_vanished = remove_vanished; }
 
@@ -344,6 +345,10 @@ pub fn update_sync_job(
 
     sync::save_config(&config)?;
 
+    if schedule_changed {
+        crate::server::jobstate::try_update_state_file("syncjob", &id)?;
+    }
+
     Ok(())
 }
 
diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
index caea4e18..776b89e4 100644
--- a/src/api2/config/tape_backup_job.rs
+++ b/src/api2/config/tape_backup_job.rs
@@ -266,6 +266,7 @@ pub fn update_tape_backup_job(
     if latest_only.is_some() { data.setup.latest_only = latest_only; }
     if notify_user.is_some() { data.setup.notify_user = notify_user; }
 
+    let schedule_changed = data.schedule != schedule;
     if schedule.is_some() { data.schedule = schedule; }
 
     if let Some(comment) = comment {
@@ -281,6 +282,10 @@ pub fn update_tape_backup_job(
 
     config::tape_job::save_config(&config)?;
 
+    if schedule_changed {
+        crate::server::jobstate::try_update_state_file("tape-backup-job", &id)?;
+    }
+
     Ok(())
 }
 
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
index db5f4d83..dee4c669 100644
--- a/src/api2/config/verify.rs
+++ b/src/api2/config/verify.rs
@@ -274,12 +274,17 @@ pub fn update_verification_job(
 
     if ignore_verified.is_some() { data.ignore_verified = ignore_verified; }
     if outdated_after.is_some() { data.outdated_after = outdated_after; }
+    let schedule_changed = data.schedule != schedule;
     if schedule.is_some() { data.schedule = schedule; }
 
     config.set_data(&id, "verification", &data)?;
 
     verify::save_config(&config)?;
 
+    if schedule_changed {
+        crate::server::jobstate::try_update_state_file("verificationjob", &id)?;
+    }
+
     Ok(())
 }
 
diff --git a/src/server/jobstate.rs b/src/server/jobstate.rs
index 81e516d4..c62e58a2 100644
--- a/src/server/jobstate.rs
+++ b/src/server/jobstate.rs
@@ -69,8 +69,12 @@ pub enum JobState {
     Created { time: i64 },
     /// The Job was last started in 'upid',
     Started { upid: String },
-    /// The Job was last started in 'upid', which finished with 'state'
-    Finished { upid: String, state: TaskState },
+    /// The Job was last started in 'upid', which finished with 'state', and was last updated at 'updated'
+    Finished {
+        upid: String,
+        state: TaskState,
+        updated: Option<i64>,
+    },
 }
 
 /// Represents a Job and holds the correct lock
@@ -147,12 +151,46 @@ pub fn create_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
     job.write_state()
 }
 
+/// Tries to update the state file with the current time
+/// if the job is currently running, does nothing,
+pub fn try_update_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
+    let mut job = match Job::new(jobtype, jobname) {
+        Ok(job) => job,
+        Err(_) => return Ok(()), // was locked (running), so do not update
+    };
+    let time = proxmox::tools::time::epoch_i64();
+
+    job.state = match JobState::load(jobtype, jobname)? {
+        JobState::Created { .. } => JobState::Created { time },
+        JobState::Started { .. } => return Ok(()), // currently running (without lock?)
+        JobState::Finished {
+            upid,
+            state,
+            updated: _,
+        } => JobState::Finished {
+            upid,
+            state,
+            updated: Some(time),
+        },
+    };
+    job.write_state()
+}
+
 /// Returns the last run time of a job by reading the statefile
 /// Note that this is not locked
 pub fn last_run_time(jobtype: &str, jobname: &str) -> Result<i64, Error> {
     match JobState::load(jobtype, jobname)? {
         JobState::Created { time } => Ok(time),
-        JobState::Started { upid } | JobState::Finished { upid, .. } => {
+        JobState::Finished {
+            updated: Some(time),
+            ..
+        } => Ok(time),
+        JobState::Started { upid }
+        | JobState::Finished {
+            upid,
+            state: _,
+            updated: None,
+        } => {
             let upid: UPID = upid
                 .parse()
                 .map_err(|err| format_err!("could not parse upid from state: {}", err))?;
@@ -180,7 +218,11 @@ impl JobState {
                         let state = upid_read_status(&parsed)
                             .map_err(|err| format_err!("error reading upid log status: {}", err))?;
 
-                        Ok(JobState::Finished { upid, state })
+                        Ok(JobState::Finished {
+                            upid,
+                            state,
+                            updated: None,
+                        })
                     } else {
                         Ok(JobState::Started { upid })
                     }
@@ -240,7 +282,11 @@ impl Job {
         }
         .to_string();
 
-        self.state = JobState::Finished { upid, state };
+        self.state = JobState::Finished {
+            upid,
+            state,
+            updated: None,
+        };
 
         self.write_state()
     }
@@ -274,17 +320,25 @@ pub fn compute_schedule_status(
     job_state: &JobState,
     schedule: Option<&str>,
 ) -> Result<JobScheduleStatus, Error> {
-
-    let (upid, endtime, state, starttime) = match job_state {
+    let (upid, endtime, state, last) = match job_state {
         JobState::Created { time } => (None, None, None, *time),
         JobState::Started { upid } => {
             let parsed_upid: UPID = upid.parse()?;
             (Some(upid), None, None, parsed_upid.starttime)
-        },
-        JobState::Finished { upid, state } => {
-            let parsed_upid: UPID = upid.parse()?;
-            (Some(upid), Some(state.endtime()), Some(state.to_string()), parsed_upid.starttime)
-        },
+        }
+        JobState::Finished {
+            upid,
+            state,
+            updated,
+        } => {
+            let last = updated.unwrap_or_else(|| state.endtime());
+            (
+                Some(upid),
+                Some(state.endtime()),
+                Some(state.to_string()),
+                last,
+            )
+        }
     };
 
     let mut status = JobScheduleStatus::default();
@@ -292,10 +346,8 @@ pub fn compute_schedule_status(
     status.last_run_state = state;
     status.last_run_endtime = endtime;
 
-    let last = endtime.unwrap_or(starttime);
-
     if let Some(schedule) = schedule {
-        if let Ok(event) =  parse_calendar_event(&schedule) {
+        if let Ok(event) = parse_calendar_event(&schedule) {
             // ignore errors
             status.next_run = compute_next_event(&event, last, false).unwrap_or(None);
         }
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant

[Dominik Csapak]

commit subject should be

server/jobstate: add 'updated' to Finish variant

ofc