[pbs-devel] [PATCH proxmox-backup] client/pull: log snapshots that are skipped because of time

[pbs-devel] [PATCH proxmox-backup] client/pull: log snapshots that are skipped because of time

[Dominik Csapak]

we skip snapshots that are older than the last sync-time,
log that so the user can know why it is not synced

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/client/pull.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/client/pull.rs b/src/client/pull.rs
index 95720973..96414a7a 100644
--- a/src/client/pull.rs
+++ b/src/client/pull.rs
@@ -14,6 +14,7 @@ use crate::{
     backup::*,
     client::*,
     server::WorkerTask,
+    task_log,
     tools::{compute_file_csum, ParallelHandler},
 };
 use proxmox::api::error::{HttpError, StatusCode};
@@ -495,6 +496,11 @@ pub async fn pull_group(
 
         if let Some(last_sync_time) = last_sync {
             if last_sync_time > backup_time {
+                task_log!(
+                    worker,
+                    "skipping snapshot {} - older than last sync",
+                    snapshot
+                );
                 continue;
             }
         }
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup] client/pull: log snapshots that are skipped because of time

[Fabian Grünbichler]

On June 4, 2021 9:16 am, Dominik Csapak wrote:
> we skip snapshots that are older than the last sync-time,
> log that so the user can know why it is not synced

well, technically last sync is a bit of a misnomer here anyway, since it 
could also just be last backup made directly to the target datastore. we 
might want to make this behaviour more explicit as well? or allow 
marking groups/datastore as "sync target" to prevent manual backups from 
interfering with syncs?

> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/client/pull.rs | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/client/pull.rs b/src/client/pull.rs
> index 95720973..96414a7a 100644
> --- a/src/client/pull.rs
> +++ b/src/client/pull.rs
> @@ -14,6 +14,7 @@ use crate::{
>      backup::*,
>      client::*,
>      server::WorkerTask,
> +    task_log,
>      tools::{compute_file_csum, ParallelHandler},
>  };
>  use proxmox::api::error::{HttpError, StatusCode};
> @@ -495,6 +496,11 @@ pub async fn pull_group(
>  
>          if let Some(last_sync_time) = last_sync {
>              if last_sync_time > backup_time {
> +                task_log!(
> +                    worker,
> +                    "skipping snapshot {} - older than last sync",
> +                    snapshot
> +                );

adding this would mean that sync logs would grow and grow and grow over 
time..

maybe a single:

skipped 32 remote snapshots (2021-MM-DDTHH:MM::SSZ..2021-MM-DDTHH:MM::SSZ) older than newest local snapshot.

it would mean only adding a single line to the output, but contains 
almost all the information?

>                  continue;
>              }
>          }
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 

Re: [pbs-devel] [PATCH proxmox-backup] client/pull: log snapshots that are skipped because of time

[Dietmar Maurer]

On 6/4/21 10:21 AM, Fabian Grünbichler wrote:
>
>> @@ -495,6 +496,11 @@ pub async fn pull_group(
>>   
>>           if let Some(last_sync_time) = last_sync {
>>               if last_sync_time > backup_time {
>> +                task_log!(
>> +                    worker,
>> +                    "skipping snapshot {} - older than last sync",
>> +                    snapshot
>> +                );
> adding this would mean that sync logs would grow and grow and grow over
> time..
>
> maybe a single:
>
> skipped 32 remote snapshots (2021-MM-DDTHH:MM::SSZ..2021-MM-DDTHH:MM::SSZ) older than newest local snapshot.
>
> it would mean only adding a single line to the output, but contains
> almost all the information?
>
I would also prefer a single log line.

Re: [pbs-devel] [PATCH proxmox-backup] client/pull: log snapshots that are skipped because of time

[Thomas Lamprecht]

On 04.06.21 10:21, Fabian Grünbichler wrote:
> On June 4, 2021 9:16 am, Dominik Csapak wrote:
>> @@ -495,6 +496,11 @@ pub async fn pull_group(
>>  
>>          if let Some(last_sync_time) = last_sync {
>>              if last_sync_time > backup_time {
>> +                task_log!(
>> +                    worker,
>> +                    "skipping snapshot {} - older than last sync",
>> +                    snapshot
>> +                );
> 
> adding this would mean that sync logs would grow and grow and grow over 
> time..
> 
> maybe a single:
> 
> skipped 32 remote snapshots (2021-MM-DDTHH:MM::SSZ..2021-MM-DDTHH:MM::SSZ) older than newest local snapshot.
> 
> it would mean only adding a single line to the output, but contains 
> almost all the information?

If we want to just log that for now that this is def. the more sensible approach,
task logs should be always as readable, where less to read without losing any, or
at least not much, information is always nice, so:

+1