* [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask
@ 2024-01-24 10:34 Gabriel Goller
2024-01-24 13:17 ` Fiona Ebner
0 siblings, 1 reply; 5+ messages in thread
From: Gabriel Goller @ 2024-01-24 10:34 UTC (permalink / raw)
To: pbs-devel
`prune-group` is currently not a real workertask, ie it behaves like one
but doesn't start a thread nor a task to do its work.
Changed it to start a tokio-task, so that we can delete snapshots
asynchronously. The `dry-run` feature still behaves in the same way and
returns early.
This paves the way for the new logging infra (which uses `task_local` to
define a logger) and improves performance of bigger backup-groups.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
src/api2/admin/datastore.rs | 104 ++++++++++++++++++------------------
1 file changed, 53 insertions(+), 51 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index a95031e7..4801eb1a 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1007,62 +1007,64 @@ pub fn prune(
}
return Ok(json!(prune_result));
}
+ let upid = WorkerTask::spawn(
+ "prune",
+ Some(worker_id),
+ auth_id.to_string(),
+ true,
+ move |worker| async move {
+ if keep_all {
+ task_log!(worker, "No prune selection - keeping all files.");
+ } else {
+ let mut opts = Vec::new();
+ if !ns.is_root() {
+ opts.push(format!("--ns {ns}"));
+ }
+ crate::server::cli_keep_options(&mut opts, &keep_options);
- // We use a WorkerTask just to have a task log, but run synchrounously
- let worker = WorkerTask::new("prune", Some(worker_id), auth_id.to_string(), true)?;
-
- if keep_all {
- task_log!(worker, "No prune selection - keeping all files.");
- } else {
- let mut opts = Vec::new();
- if !ns.is_root() {
- opts.push(format!("--ns {ns}"));
- }
- crate::server::cli_keep_options(&mut opts, &keep_options);
-
- task_log!(worker, "retention options: {}", opts.join(" "));
- task_log!(
- worker,
- "Starting prune on {} group \"{}\"",
- print_store_and_ns(&store, &ns),
- group.group(),
- );
- }
-
- for (info, mark) in prune_info {
- let keep = keep_all || mark.keep();
-
- let backup_time = info.backup_dir.backup_time();
- let timestamp = info.backup_dir.backup_time_string();
- let group: &pbs_api_types::BackupGroup = info.backup_dir.as_ref();
-
- let msg = format!("{}/{}/{} {}", group.ty, group.id, timestamp, mark,);
-
- task_log!(worker, "{}", msg);
-
- prune_result.push(json!({
- "backup-type": group.ty,
- "backup-id": group.id,
- "backup-time": backup_time,
- "keep": keep,
- "protected": mark.protected(),
- }));
-
- if !(dry_run || keep) {
- if let Err(err) = info.backup_dir.destroy(false) {
- task_warn!(
+ task_log!(worker, "retention options: {}", opts.join(" "));
+ task_log!(
worker,
- "failed to remove dir {:?}: {}",
- info.backup_dir.relative_path(),
- err,
+ "Starting prune on {} group \"{}\"",
+ print_store_and_ns(&store, &ns),
+ group.group(),
);
}
- }
- }
- worker.log_result(&Ok(()));
-
- Ok(json!(prune_result))
+ for (info, mark) in prune_info {
+ let keep = keep_all || mark.keep();
+
+ let backup_time = info.backup_dir.backup_time();
+ let timestamp = info.backup_dir.backup_time_string();
+ let group: &pbs_api_types::BackupGroup = info.backup_dir.as_ref();
+
+ let msg = format!("{}/{}/{} {}", group.ty, group.id, timestamp, mark,);
+
+ task_log!(worker, "{}", msg);
+
+ prune_result.push(json!({
+ "backup-type": group.ty,
+ "backup-id": group.id,
+ "backup-time": backup_time,
+ "keep": keep,
+ "protected": mark.protected(),
+ }));
+
+ if !(dry_run || keep) {
+ if let Err(err) = info.backup_dir.destroy(false) {
+ task_warn!(
+ worker,
+ "failed to remove dir {:?}: {}",
+ info.backup_dir.relative_path(),
+ err,
+ );
+ }
+ }
+ }
+ Ok(())
+ },
+ )?;
+ Ok(json!(upid))
}
#[api(
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask
2024-01-24 10:34 [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask Gabriel Goller
@ 2024-01-24 13:17 ` Fiona Ebner
2024-01-24 13:28 ` Thomas Lamprecht
0 siblings, 1 reply; 5+ messages in thread
From: Fiona Ebner @ 2024-01-24 13:17 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Gabriel Goller
Am 24.01.24 um 11:34 schrieb Gabriel Goller:
> - Ok(json!(prune_result))
> + Ok(json!(upid))
That seems like a breaking API change at a first glance and thus has to
wait for the next major release unfortunately.
AFAIU the same rules as for PVE apply, see:
https://pve.proxmox.com/wiki/Proxmox_VE_API#API_Stability_.26_Breakage
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask
2024-01-24 13:17 ` Fiona Ebner
@ 2024-01-24 13:28 ` Thomas Lamprecht
2024-01-24 13:42 ` Gabriel Goller
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Lamprecht @ 2024-01-24 13:28 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fiona Ebner,
Gabriel Goller
Am 24/01/2024 um 14:17 schrieb Fiona Ebner:
> Am 24.01.24 um 11:34 schrieb Gabriel Goller:
>> - Ok(json!(prune_result))
>
>> + Ok(json!(upid))
>
> That seems like a breaking API change at a first glance and thus has to
> wait for the next major release unfortunately.
>
> AFAIU the same rules as for PVE apply, see:
> https://pve.proxmox.com/wiki/Proxmox_VE_API#API_Stability_.26_Breakage
>
Yes, if applied strictly, and this here might be indeed a breakage that
matters in practice.
But you can work around that by making it use a worker task opt-in via
a new parameter boolean flag.
Oh, and isn't prune_result now unused? We're still lacking a simple
and standardized method to save a structured result alongside a task
UPID, otherwise it could be saved there, but that's another topic.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask
2024-01-24 13:28 ` Thomas Lamprecht
@ 2024-01-24 13:42 ` Gabriel Goller
2024-01-25 11:49 ` Gabriel Goller
0 siblings, 1 reply; 5+ messages in thread
From: Gabriel Goller @ 2024-01-24 13:42 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox Backup Server development discussion,
Fiona Ebner
On 24-01-2024 14:28, Thomas Lamprecht wrote:
> Am 24/01/2024 um 14:17 schrieb Fiona Ebner:
>> Am 24.01.24 um 11:34 schrieb Gabriel Goller:
>>> - Ok(json!(prune_result))
>>> + Ok(json!(upid))
>> That seems like a breaking API change at a first glance and thus has to
>> wait for the next major release unfortunately.
>>
>> AFAIU the same rules as for PVE apply, see:
>> https://pve.proxmox.com/wiki/Proxmox_VE_API#API_Stability_.26_Breakage
>>
> Yes, if applied strictly, and this here might be indeed a breakage that
> matters in practice.
>
> But you can work around that by making it use a worker task opt-in via
> a new parameter boolean flag.
Will do that.
> Oh, and isn't prune_result now unused? We're still lacking a simple
> and standardized method to save a structured result alongside a task
> UPID, otherwise it could be saved there, but that's another topic.
Hmm, yeah noticed that... I think I'll convert it to a structure and
pretty-print it to the task-log.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask
2024-01-24 13:42 ` Gabriel Goller
@ 2024-01-25 11:49 ` Gabriel Goller
0 siblings, 0 replies; 5+ messages in thread
From: Gabriel Goller @ 2024-01-25 11:49 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox Backup Server development discussion,
Fiona Ebner
Sent a v2!
On 24-01-2024 14:42, Gabriel Goller wrote:
> On 24-01-2024 14:28, Thomas Lamprecht wrote:
>> Am 24/01/2024 um 14:17 schrieb Fiona Ebner:
>>> Am 24.01.24 um 11:34 schrieb Gabriel Goller:
>>>> - Ok(json!(prune_result))
>>>> + Ok(json!(upid))
>>> That seems like a breaking API change at a first glance and thus has to
>>> wait for the next major release unfortunately.
>>>
>>> AFAIU the same rules as for PVE apply, see:
>>> https://pve.proxmox.com/wiki/Proxmox_VE_API#API_Stability_.26_Breakage
>>>
>> Yes, if applied strictly, and this here might be indeed a breakage that
>> matters in practice.
>>
>> But you can work around that by making it use a worker task opt-in via
>> a new parameter boolean flag.
> Will do that.
>> Oh, and isn't prune_result now unused? We're still lacking a simple
>> and standardized method to save a structured result alongside a task
>> UPID, otherwise it could be saved there, but that's another topic.
> Hmm, yeah noticed that... I think I'll convert it to a structure and
> pretty-print it to the task-log.
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-25 11:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-24 10:34 [pbs-devel] [PATCH proxmox-backup] api: make prune-group a real workertask Gabriel Goller
2024-01-24 13:17 ` Fiona Ebner
2024-01-24 13:28 ` Thomas Lamprecht
2024-01-24 13:42 ` Gabriel Goller
2024-01-25 11:49 ` Gabriel Goller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox