From 951fe0cb7d0b4e254e491503670bcb66e09f7021 Mon Sep 17 00:00:00 2001 From: Dominik Csapak Date: Mon, 19 Apr 2021 10:32:16 +0200 Subject: [PATCH] server/jobstate: add 'updatd' to Finish variant 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 --- 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, + }, } /// 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 { 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 { - - 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); }