From: Fabian Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH storage] prune mark: correctly keep track of already included backups
Date: Mon, 14 Dec 2020 15:03:17 +0000 [thread overview]
Message-ID: <20201214150317.28734-1-f.ebner@proxmox.com> (raw)
This needs to happen in a separate loop, because some time intervals are not
subsets of others, i.e. weeks and months. Previously, with a daily backup
schedule, having:
* a backup on Sun, 06 Dec 2020 kept by keep-daily
* a backup on Sun, 29 Nov 2020 kept by keep-weekly
would lead to the backup on Mon, 30 Nov 2020 to be selected for keep-monthly,
because the iteration did not yet reach the backup on Sun, 29 Nov 2020 that
would mark November as being covered.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
The PBS implementation is not affected by this as it uses two loops.
I don't see how this would cause bug #3199 though...
PVE/Storage.pm | 11 +++++----
test/prune_backups_test.pm | 49 ++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index aded60e..c1a21b4 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1646,13 +1646,14 @@ my $prune_mark = sub {
foreach my $prune_entry (@{$prune_entries}) {
my $mark = $prune_entry->{mark};
my $id = $id_func->($prune_entry->{ctime});
+ $already_included->{$id} = 1 if defined($mark) && $mark eq 'keep';
+ }
- next if $already_included->{$id};
+ foreach my $prune_entry (@{$prune_entries}) {
+ my $mark = $prune_entry->{mark};
+ my $id = $id_func->($prune_entry->{ctime});
- if (defined($mark)) {
- $already_included->{$id} = 1 if $mark eq 'keep';
- next;
- }
+ next if defined($mark) || $already_included->{$id};
if (!$newly_included->{$id}) {
last if scalar(keys %{$newly_included}) >= $keep_count;
diff --git a/test/prune_backups_test.pm b/test/prune_backups_test.pm
index 8bf564d..c69c467 100644
--- a/test/prune_backups_test.pm
+++ b/test/prune_backups_test.pm
@@ -74,6 +74,23 @@ push @{$mocked_backups_lists->{novmid}}, (
'ctime' => 1234,
},
);
+push @{$mocked_backups_lists->{threeway}}, (
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-7654-2019_12_25-12_18_21.tar.zst",
+ 'ctime' => $basetime - 7*24*60*60,
+ 'vmid' => 7654,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-7654-2019_12_31-12_18_21.tar.zst",
+ 'ctime' => $basetime - 24*60*60,
+ 'vmid' => 7654,
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-7654-2020_01_01-12_18_21.tar.zst",
+ 'ctime' => $basetime,
+ 'vmid' => 7654,
+ },
+);
my $current_list;
my $mock_plugin = Test::MockModule->new('PVE::Storage::Plugin');
$mock_plugin->redefine(list_volumes => sub {
@@ -361,6 +378,38 @@ my $tests = [
},
expected => generate_expected(\@vmids, undef, ['keep', 'keep', 'keep', 'keep', 'keep', 'keep']),
},
+ {
+ description => 'daily=weekly=monthly=1',
+ keep => {
+ 'keep-daily' => 1,
+ 'keep-weekly' => 1,
+ 'keep-monthly' => 1,
+ },
+ list => 'threeway',
+ expected => [
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-7654-2019_12_25-12_18_21.tar.zst",
+ 'ctime' => $basetime - 7*24*60*60,
+ 'type' => 'qemu',
+ 'vmid' => 7654,
+ 'mark' => 'keep',
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-7654-2019_12_31-12_18_21.tar.zst",
+ 'ctime' => $basetime - 24*60*60,
+ 'type' => 'qemu',
+ 'vmid' => 7654,
+ 'mark' => 'remove', # month is already covered by the backup kept by keep-weekly!
+ },
+ {
+ 'volid' => "$storeid:backup/vzdump-qemu-7654-2020_01_01-12_18_21.tar.zst",
+ 'ctime' => $basetime,
+ 'type' => 'qemu',
+ 'vmid' => 7654,
+ 'mark' => 'keep',
+ },
+ ],
+ },
];
plan tests => scalar @$tests;
--
2.20.1
next reply other threads:[~2020-12-14 15:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-14 15:03 Fabian Ebner [this message]
2020-12-14 15:13 ` [pve-devel] applied: " Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201214150317.28734-1-f.ebner@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox