public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabian Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH guest-common 1/2] replication: update last_sync before removing old replication snapshots
Date: Fri, 26 Nov 2021 11:52:30 +0100	[thread overview]
Message-ID: <20211126105232.436044-1-f.ebner@proxmox.com> (raw)

If pvesr was terminated after finishing with the new sync and after
removing old replication snapshots, but before it could write the new
state, the next replication would fail. It would wrongly interpret the
actual last replication snapshot as stale, remove it, and (if no other
snapshots are present) attempt a full sync, which would fail.

Reported in the community forum [0], this was brought to light by the
new pvescheduler before it learned graceful reload.

It's not possible to simply preserve a last remaining snapshot in
prepare(), because prepare() is also used for valid removals. Instead,
update last_sync early enough. Stale snapshots will still be removed
on the next run if there are any.

[0]: https://forum.proxmox.com/threads/100154

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/Replication.pm      | 3 +++
 src/PVE/ReplicationState.pm | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/src/PVE/Replication.pm b/src/PVE/Replication.pm
index 051dfd9..de652f2 100644
--- a/src/PVE/Replication.pm
+++ b/src/PVE/Replication.pm
@@ -372,6 +372,9 @@ sub replicate {
 	die $err;
     }
 
+    # Ensure that new sync is recorded before removing old replication snapshots.
+    PVE::ReplicationState::record_sync_end($jobcfg, $state, $start_time);
+
     # remove old snapshots because they are no longer needed
     $cleanup_local_snapshots->($last_snapshots, $last_sync_snapname);
 
diff --git a/src/PVE/ReplicationState.pm b/src/PVE/ReplicationState.pm
index 81a1b31..8efe0e2 100644
--- a/src/PVE/ReplicationState.pm
+++ b/src/PVE/ReplicationState.pm
@@ -159,6 +159,13 @@ sub delete_guest_states {
     PVE::Tools::lock_file($state_lock, 10, $code);
 }
 
+sub record_sync_end {
+    my ($jobcfg, $state, $start_time) = @_;
+
+    $state->{last_sync} = $start_time;
+    write_job_state($jobcfg, $state);
+}
+
 sub record_job_end {
     my ($jobcfg, $state, $start_time, $duration, $err) = @_;
 
-- 
2.30.2





             reply	other threads:[~2021-11-26 10:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-26 10:52 Fabian Ebner [this message]
2021-11-26 10:52 ` [pve-devel] [PATCH guest-common 2/2] replication: prepare: simplify code Fabian Ebner
2021-11-29 10:18   ` [pve-devel] applied-series: " Fabian Grünbichler

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=20211126105232.436044-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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal