all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant
@ 2021-04-19  8:32 Dominik Csapak
  2021-04-19  8:37 ` Dominik Csapak
  0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2021-04-19  8:32 UTC (permalink / raw)
  To: pbs-devel

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





^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant
  2021-04-19  8:32 [pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant Dominik Csapak
@ 2021-04-19  8:37 ` Dominik Csapak
  0 siblings, 0 replies; 2+ messages in thread
From: Dominik Csapak @ 2021-04-19  8:37 UTC (permalink / raw)
  To: pbs-devel

commit subject should be

server/jobstate: add 'updated' to Finish variant

ofc




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-04-19  8:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19  8:32 [pbs-devel] [PATCH proxmox-backup] server/jobstate: add 'updatd' to Finish variant Dominik Csapak
2021-04-19  8:37 ` Dominik Csapak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal