[pmg-devel] [PATCH pmg-api 0/2] PBS integration: include statdb

[pmg-devel] [PATCH pmg-api 0/2] PBS integration: include statdb

[Stoiko Ivanov]

These two patches fix the missing statistics database in the PBS-backup.

Removal of the old backup-state (2/2) is done to not include the statistic
database if not selected (and also potential left-over files from a failed
backup)

Stoiko Ivanov (2):
  pbs: api: add statistic parameter to run_backup
  api: pbs: remove state after backup

 src/PMG/API2/PBS/Job.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)

-- 
2.20.1

[pmg-devel] [PATCH pmg-api 1/2] pbs: api: add statistic parameter to run_backup

[Stoiko Ivanov]

Without it the backup does not include the statistics database

Reported-by: Martin Maurer <martin@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/PBS/Job.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
index 793ad88..a0d942a 100644
--- a/src/PMG/API2/PBS/Job.pm
+++ b/src/PMG/API2/PBS/Job.pm
@@ -266,6 +266,12 @@ __PACKAGE__->register_method ({
 		description => "Proxmox Backup Server ID.",
 		type => 'string', format => 'pve-configid',
 	    },
+	    statistic => {
+		description => "Backup statistic databases.",
+		type => 'boolean',
+		optional => 1,
+		default => 1,
+	    },
 	},
     },
     returns => { type => "string" },
@@ -284,6 +290,8 @@ __PACKAGE__->register_method ({
 	die "PBS remote '$remote' does not exist\n" if !$remote_config;
 	die "PBS remote '$remote' is disabled\n" if $remote_config->{disable};
 
+	$param->{statistic} //= 1;
+
 	my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
 	my $backup_dir = "/var/lib/pmg/backup/current";
 
-- 
2.20.1

[pmg-devel] [PATCH pmg-api 2/2] api: pbs: remove state after backup

[Stoiko Ivanov]

else left-over files will continue to be included (e.g. statistics database)

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/PBS/Job.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
index a0d942a..a885edd 100644
--- a/src/PMG/API2/PBS/Job.pm
+++ b/src/PMG/API2/PBS/Job.pm
@@ -305,6 +305,8 @@ __PACKAGE__->register_method ({
 
 	    $pbs->backup_fs_tree($backup_dir, $node, 'pmgbackup');
 
+	    rmtree $backup_dir;
+
 	    print "backup finished\n";
 
 	    my $group = "host/$node";
-- 
2.20.1

[pmg-devel] applied-series: [PATCH pmg-api 0/2] PBS integration: include statdb

[Thomas Lamprecht]

On 18.11.20 18:59, Stoiko Ivanov wrote:
> These two patches fix the missing statistics database in the PBS-backup.
> 
> Removal of the old backup-state (2/2) is done to not include the statistic
> database if not selected (and also potential left-over files from a failed
> backup)
> 
> Stoiko Ivanov (2):
>   pbs: api: add statistic parameter to run_backup
>   api: pbs: remove state after backup
> 
>  src/PMG/API2/PBS/Job.pm | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 



applied series, thanks!

Re: [pmg-devel] [PATCH pmg-api 1/2] pbs: api: add statistic parameter to run_backup

[Dietmar Maurer]

why does this default => 1 ?

Statistics are large, and not really required for fully functional restore.
IMHO, I want backups as small as possible.
 
> On 11/18/2020 6:59 PM Stoiko Ivanov <s.ivanov@proxmox.com> wrote:
> 
>  
> Without it the backup does not include the statistics database
> 
> Reported-by: Martin Maurer <martin@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/API2/PBS/Job.pm | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
> index 793ad88..a0d942a 100644
> --- a/src/PMG/API2/PBS/Job.pm
> +++ b/src/PMG/API2/PBS/Job.pm
> @@ -266,6 +266,12 @@ __PACKAGE__->register_method ({
>  		description => "Proxmox Backup Server ID.",
>  		type => 'string', format => 'pve-configid',
>  	    },
> +	    statistic => {
> +		description => "Backup statistic databases.",
> +		type => 'boolean',
> +		optional => 1,
> +		default => 1,
> +	    },
>  	},
>      },
>      returns => { type => "string" },
> @@ -284,6 +290,8 @@ __PACKAGE__->register_method ({
>  	die "PBS remote '$remote' does not exist\n" if !$remote_config;
>  	die "PBS remote '$remote' is disabled\n" if $remote_config->{disable};
>  
> +	$param->{statistic} //= 1;
> +
>  	my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
>  	my $backup_dir = "/var/lib/pmg/backup/current";
>  
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api 1/2] pbs: api: add statistic parameter to run_backup

[Stoiko Ivanov]

On Wed, 18 Nov 2020 19:34:23 +0100 (CET)
Dietmar Maurer <dietmar@proxmox.com> wrote:

> why does this default => 1 ?
mostly to mirror the way it is with the local-backup (to tar.gz):
https://git.proxmox.com/?p=pmg-api.git;a=blob;f=src/PMG/API2/Backup.pm;h=4ea28d13e28f23956ec7e0ff64e77ea04a333a53;hb=HEAD#l122

I also think that we should rather backup more than less in the default
case - I personally be more disappointed to lose my year long statistics
in case of disaster than about ~100 M (estimated based on our
productive instance) more space on my backupstorage.
You do have a point regarding size (backup without statistics is usually <
500k vs 100M) though - so making this comfortably configurable would be a
nice enhancement! 

Making this actually user-selectable (apart from a direct API-call or
using `pmgbackup` manually) would need exposing this in the GUI (and
somehow getting it in the systemd-service units).

Would recording this in bugzilla work for you?


> 
> Statistics are large, and not really required for fully functional restore.
> IMHO, I want backups as small as possible.
>  
> > On 11/18/2020 6:59 PM Stoiko Ivanov <s.ivanov@proxmox.com> wrote:
> > 
> >  
> > Without it the backup does not include the statistics database
> > 
> > Reported-by: Martin Maurer <martin@proxmox.com>
> > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> > ---
> >  src/PMG/API2/PBS/Job.pm | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
> > index 793ad88..a0d942a 100644
> > --- a/src/PMG/API2/PBS/Job.pm
> > +++ b/src/PMG/API2/PBS/Job.pm
> > @@ -266,6 +266,12 @@ __PACKAGE__->register_method ({
> >  		description => "Proxmox Backup Server ID.",
> >  		type => 'string', format => 'pve-configid',
> >  	    },
> > +	    statistic => {
> > +		description => "Backup statistic databases.",
> > +		type => 'boolean',
> > +		optional => 1,
> > +		default => 1,
> > +	    },
> >  	},
> >      },
> >      returns => { type => "string" },
> > @@ -284,6 +290,8 @@ __PACKAGE__->register_method ({
> >  	die "PBS remote '$remote' does not exist\n" if !$remote_config;
> >  	die "PBS remote '$remote' is disabled\n" if $remote_config->{disable};
> >  
> > +	$param->{statistic} //= 1;
> > +
> >  	my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
> >  	my $backup_dir = "/var/lib/pmg/backup/current";
> >  
> > -- 
> > 2.20.1
> > 
> > 
> > 
> > _______________________________________________
> > pmg-devel mailing list
> > pmg-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel  

Re: [pmg-devel] [PATCH pmg-api 1/2] pbs: api: add statistic parameter to run_backup

[Dietmar Maurer]

> On 11/19/2020 9:40 AM Stoiko Ivanov <s.ivanov@proxmox.com> wrote:
> 
>  
> On Wed, 18 Nov 2020 19:34:23 +0100 (CET)
> Dietmar Maurer <dietmar@proxmox.com> wrote:
> 
> > why does this default => 1 ?
> mostly to mirror the way it is with the local-backup (to tar.gz):
> https://git.proxmox.com/?p=pmg-api.git;a=blob;f=src/PMG/API2/Backup.pm;h=4ea28d13e28f23956ec7e0ff64e77ea04a333a53;hb=HEAD#l122
> 
> I also think that we should rather backup more than less in the default
> case - I personally be more disappointed to lose my year long statistics
> in case of disaster than about ~100 M (estimated based on our
> productive instance) more space on my backupstorage.
> You do have a point regarding size (backup without statistics is usually <
> 500k vs 100M) though - so making this comfortably configurable would be a
> nice enhancement! 
> 
> Making this actually user-selectable (apart from a direct API-call or
> using `pmgbackup` manually) would need exposing this in the GUI (and
> somehow getting it in the systemd-service units).
> 
> Would recording this in bugzilla work for you?

sure