[PATCH proxmox-backup] api: gc status: pass schedule instead of UPID to schedule helper

[PATCH proxmox-backup] api: gc status: pass schedule instead of UPID to schedule helper

[Jonas Theisen]

Since the GC job status was introduced and then refactored in [1]
the last GC run's UPID has been passed as the schedule argument
of compute_schedule_status().
A UPID never parses as a calendar event, so the computed next run
was always None, and it was then recomputed from the GC schedule
in a separate step after the fact.

Pass the configured GC schedule instead and use the helper's result
directly. This drops the duplicated computation, and the next run
of GC jobs is now derived from the job state file in the same way as
for sync, verify, prune and tape backup jobs. Previously, the base
was the last run's end time from the in-memory GC status, or the
current time if there was none, which ignored the job state that
the proxy's scheduler relies on.

[1] https://lore.proxmox.com/pbs-devel/20260319110318.70346-2-m.koeppl@proxmox.com/

Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
---
 src/api2/admin/datastore.rs | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3367fb2c1..cfe976811 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -29,7 +29,6 @@ use proxmox_rrd_api_types::{RrdMode, RrdTimeframe};
 use proxmox_schema::*;
 use proxmox_sortable_macro::sortable;
 use proxmox_sys::fs::{CreateOptions, file_read_firstline, file_read_optional_string};
-use proxmox_time::CalendarEvent;
 use proxmox_worker_task::WorkerTaskContext;
 
 use pxar::EntryKind;
@@ -1270,42 +1269,21 @@ pub(crate) fn garbage_collection_status_unchecked(
     let datastore = DataStore::lookup_datastore(lookup_with(&store, Operation::Read))?;
     let status_in_memory = datastore.last_gc_status();
 
-    let mut last = proxmox_time::epoch_i64();
-
-    let jobtype = "garbage_collection";
+    let computed_schedule: JobScheduleStatus =
+        compute_schedule_status("garbage_collection", &store, info.schedule.as_deref())?;
 
     if let Some(ref upid) = status_in_memory.upid {
-        let computed_schedule: JobScheduleStatus =
-            compute_schedule_status(jobtype, &store, Some(upid))?;
-
         if let Some(endtime) = computed_schedule.last_run_endtime {
-            last = endtime;
             if let Ok(parsed_upid) = upid.parse::<UPID>() {
                 info.duration = Some(endtime - parsed_upid.starttime);
             }
         }
 
-        info.next_run = computed_schedule.next_run;
         info.last_run_endtime = computed_schedule.last_run_endtime;
         info.last_run_state = computed_schedule.last_run_state;
-    } else if let Err(err) = JobState::load(jobtype, &store) {
-        log::error!("could not open statefile for {store}: {err}");
     }
 
-    info.next_run = info
-        .schedule
-        .as_ref()
-        .and_then(|s| {
-            s.parse::<CalendarEvent>()
-                .map_err(|err| log::error!("{err}"))
-                .ok()
-        })
-        .and_then(|e| {
-            e.compute_next_event(last)
-                .map_err(|err| log::error!("{err}"))
-                .ok()
-        })
-        .and_then(|ne| ne);
+    info.next_run = computed_schedule.next_run;
 
     info.status = status_in_memory;
 
-- 
2.47.3

Re: [PATCH proxmox-backup] api: gc status: pass schedule instead of UPID to schedule helper

[Christian Ebner]

On 10/22/26 6:16 PM, Jonas Theisen wrote:
> Since the GC job status was introduced and then refactored in [1]
> the last GC run's UPID has been passed as the schedule argument
> of compute_schedule_status().
> A UPID never parses as a calendar event, so the computed next run
> was always None, and it was then recomputed from the GC schedule
> in a separate step after the fact.
> 
> Pass the configured GC schedule instead and use the helper's result
> directly. This drops the duplicated computation, and the next run
> of GC jobs is now derived from the job state file in the same way as
> for sync, verify, prune and tape backup jobs. Previously, the base
> was the last run's end time from the in-memory GC status, or the
> current time if there was none, which ignored the job state that
> the proxy's scheduler relies on.
> 
> [1] https://lore.proxmox.com/pbs-devel/20260319110318.70346-2-m.koeppl@proxmox.com/
> 
> Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>

Thanks for this cleanup!

Changes fix the incorrect upid passing and duplicate job state 
loading/computation when the datastore instance has an in-memory status.

They do however also change the error logging behavior on calendar event 
parsing and next runtime calculation, which are now silently ignored. 
This is however in line with the rest of the job types, so acceptable IMO.

Tested that this does not lead to changed behaviour when the .gc-status 
is not present yet and that `/admin/datastore/{store}/gc` and 
`/admin/gc` still return consistent information.

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>