From f385755d423282e1e428a72eb9a3fe537a57c7cc Mon Sep 17 00:00:00 2001 From: 0xFelix Date: Sat, 20 Apr 2024 14:50:32 +0200 Subject: [PATCH 1/4] Fix bug when sorting snapshots Before this sortsnapshots always tried to sort source snapshots, also when it was requested to sort target snapshots. This led to errors when the list of target and source snapshots was not identical. Fix it by allowing to specify an index when sorting snapshots. --- syncoid | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/syncoid b/syncoid index b57aa43..285415c 100755 --- a/syncoid +++ b/syncoid @@ -592,7 +592,7 @@ sub syncdataset { my %bookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot); # check for matching guid of source bookmark and target snapshot (oldest first) - foreach my $snap ( sort { sortsnapshots(\%snaps, $b, $a) } keys %{ $snaps{'target'} }) { + foreach my $snap ( sort { sortsnapshots(\%snaps, $b, $a, 'target') } keys %{ $snaps{'target'} }) { my $guid = $snaps{'target'}{$snap}{'guid'}; if (defined $bookmarks{$guid}) { @@ -682,7 +682,7 @@ sub syncdataset { # if intermediate snapshots are needed we need to find the next oldest snapshot, # do an replication to it and replicate as always from oldest to newest # because bookmark sends doesn't support intermediates directly - foreach my $snap ( sort { sortsnapshots(\%snaps, $a, $b) } keys %{ $snaps{'source'} }) { + foreach my $snap ( sort { sortsnapshots(\%snaps, $a, $b, 'source') } keys %{ $snaps{'source'} }) { my $comparisonkey = 'creation'; if (defined $snaps{'source'}{$snap}{'createtxg'} && defined $bookmark{'createtxg'}) { $comparisonkey = 'createtxg'; @@ -865,7 +865,7 @@ sub syncdataset { # those that exist on the source. Remaining are the snapshots # that are only on the target. Then sort to remove the oldest # snapshots first. - my @to_delete = sort { sortsnapshots(\%snaps, $a, $b) } grep {!exists $snaps{'source'}{$_}} keys %{ $snaps{'target'} }; + my @to_delete = sort { sortsnapshots(\%snaps, $a, $b, 'source') } grep {!exists $snaps{'source'}{$_}} keys %{ $snaps{'target'} }; while (@to_delete) { # Create batch of snapshots to remove my $snaps = join ',', splice(@to_delete, 0, 50); @@ -1486,16 +1486,16 @@ sub readablebytes { } sub sortsnapshots { - my ($snaps, $left, $right) = @_; - if (defined $snaps->{'source'}{$left}{'createtxg'} && defined $snaps->{'source'}{$right}{'createtxg'}) { - return $snaps->{'source'}{$left}{'createtxg'} <=> $snaps->{'source'}{$right}{'createtxg'}; + my ($snaps, $left, $right, $idx) = @_; + if (defined $snaps->{$idx}{$left}{'createtxg'} && defined $snaps->{$idx}{$right}{'createtxg'}) { + return $snaps->{$idx}{$left}{'createtxg'} <=> $snaps->{$idx}{$right}{'createtxg'}; } - return $snaps->{'source'}{$left}{'creation'} <=> $snaps->{'source'}{$right}{'creation'}; + return $snaps->{$idx}{$left}{'creation'} <=> $snaps->{$idx}{$right}{'creation'}; } sub getoldestsnapshot { my $snaps = shift; - foreach my $snap (sort { sortsnapshots($snaps, $a, $b) } keys %{ $snaps{'source'} }) { + foreach my $snap (sort { sortsnapshots($snaps, $a, $b, 'source') } keys %{ $snaps{'source'} }) { # return on first snap found - it's the oldest return $snap; } @@ -1509,7 +1509,7 @@ sub getoldestsnapshot { sub getnewestsnapshot { my $snaps = shift; - foreach my $snap (sort { sortsnapshots($snaps, $b, $a) } keys %{ $snaps{'source'} }) { + foreach my $snap (sort { sortsnapshots($snaps, $b, $a, 'source') } keys %{ $snaps{'source'} }) { # return on first snap found - it's the newest writelog('INFO', "NEWEST SNAPSHOT: $snap"); return $snap; @@ -1688,7 +1688,7 @@ sub pruneoldsyncsnaps { sub getmatchingsnapshot { my ($sourcefs, $targetfs, $snaps) = @_; - foreach my $snap ( sort { sortsnapshots($snaps, $b, $a) } keys %{ $snaps{'source'} }) { + foreach my $snap ( sort { sortsnapshots($snaps, $b, $a, 'source') } keys %{ $snaps{'source'} }) { if (defined $snaps{'target'}{$snap}) { if ($snaps{'source'}{$snap}{'guid'} == $snaps{'target'}{$snap}{'guid'}) { return $snap; From 6b9fca6a76c4090067ae8af25658cd3d6d2db5e9 Mon Sep 17 00:00:00 2001 From: 0xFelix Date: Sun, 14 Mar 2021 17:45:52 +0100 Subject: [PATCH 2/4] Use bookmarks created after the latest snapshot This commit changes syncoid's behavior so it is always looking for a matching snapshot and a matching bookmark. If the bookmark was created after the snapshot it is used instead. This allows replication when the latest snapshot replicated was deleted on the source, a common snapshot was found but rollback on the target is not allowed. The matching bookmark is used instead for replication. This fixes https://github.com/jimsalterjrs/sanoid/issues/602 --- syncoid | 51 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/syncoid b/syncoid index 285415c..5c08120 100755 --- a/syncoid +++ b/syncoid @@ -410,6 +410,7 @@ sub syncdataset { my $newsyncsnap; my $matchingsnap; + my $usebookmark = 0; # skip snapshot checking/creation in case of resumed receive if (!defined($receivetoken)) { @@ -585,24 +586,27 @@ sub syncdataset { my $targetsize = getzfsvalue($targethost,$targetfs,$targetisroot,'-p used'); my %bookmark = (); + my $bookmarksnap = 0; $matchingsnap = getmatchingsnapshot($sourcefs, $targetfs, \%snaps); + + # find most recent matching bookmark + my %bookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot); + + # check for matching guid of source bookmark and target snapshot (oldest first) + foreach my $snap ( sort { sortsnapshots(\%snaps, $b, $a, 'target') } keys %{ $snaps{'target'} }) { + my $guid = $snaps{'target'}{$snap}{'guid'}; + + if (defined $bookmarks{$guid}) { + # found a match + %bookmark = %{ $bookmarks{$guid} }; + $bookmarksnap = $snap; + last; + } + } + if (! $matchingsnap) { # no matching snapshots, check for bookmarks as fallback - my %bookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot); - - # check for matching guid of source bookmark and target snapshot (oldest first) - foreach my $snap ( sort { sortsnapshots(\%snaps, $b, $a, 'target') } keys %{ $snaps{'target'} }) { - my $guid = $snaps{'target'}{$snap}{'guid'}; - - if (defined $bookmarks{$guid}) { - # found a match - %bookmark = %{ $bookmarks{$guid} }; - $matchingsnap = $snap; - last; - } - } - if (! %bookmark) { # force delete is not possible for the root dataset if ($args{'force-delete'} && index($targetfs, '/') != -1) { @@ -657,6 +661,18 @@ sub syncdataset { # return false now in case more child datasets need replication. return 0; + } else { + # use bookmark as fallback + $matchingsnap = $bookmarksnap; + $usebookmark = 1; + } + } elsif (%bookmark && $bookmark{'guid'} ne $snaps{'source'}{$matchingsnap}{'guid'}) { + my $comparetxg = defined $snaps{'source'}{$matchingsnap}{'createtxg'} && defined $bookmark{'createtxg'}; + if (($comparetxg && $bookmark{'createtxg'} > $snaps{'source'}{$matchingsnap}{'createtxg'}) || + $bookmark{'creation'} > substr($snaps{'source'}{$matchingsnap}{'creation'}, 0, -3)) { + writelog('DEBUG', "using bookmark $bookmark{'name'} because it was created after latest matching snapshot $matchingsnap"); + $matchingsnap = $bookmarksnap; + $usebookmark = 1; } } @@ -676,7 +692,7 @@ sub syncdataset { my $nextsnapshot = 0; - if (%bookmark) { + if ($usebookmark) { if (!defined $args{'no-stream'}) { # if intermediate snapshots are needed we need to find the next oldest snapshot, @@ -736,7 +752,7 @@ sub syncdataset { # do a normal replication if bookmarks aren't used or if previous # bookmark replication was only done to the next oldest snapshot - if (!%bookmark || $nextsnapshot) { + if (!$usebookmark || $nextsnapshot) { if ($matchingsnap eq $newsyncsnap) { # edge case: bookmark replication used the latest snapshot return 0; @@ -801,7 +817,7 @@ sub syncdataset { # if "--use-hold" parameter is used set hold on newsync snapshot and remove hold on matching snapshot both on source and target # hold name: "syncoid" + identifier + hostname -> in case of replication to multiple targets separate holds can be set for each target by assinging different identifiers to each target. Only if all targets have been replicated all syncoid holds are removed from the matching snapshot and it can be removed - if (defined $args{'use-hold'}) { + if (defined $args{'use-hold'} && !$usebookmark) { my $holdcmd; my $holdreleasecmd; my $hostid = hostname(); @@ -1964,6 +1980,7 @@ sub getbookmarks() { for my $bookmark (keys %bookmark_data) { my $guid = $bookmark_data{$bookmark}{'guid'}; + $bookmarks{$guid}{'guid'} = $guid; $bookmarks{$guid}{'name'} = $bookmark; $bookmarks{$guid}{'creation'} = $bookmark_data{$bookmark}{'creation'}; $bookmarks{$guid}{'createtxg'} = $bookmark_data{$bookmark}{'createtxg'}; From a669ba11ca662682f6805dd72ee3f7e11ae839f9 Mon Sep 17 00:00:00 2001 From: 0xFelix Date: Sun, 14 Mar 2021 17:46:16 +0100 Subject: [PATCH 3/4] Fix bug when comparing snapshot and bookmark creation times This commit fixes a bug introduced in commit 522bdecd53d221d9c937986c5de76b80d287d8bc Creation times of snapshots have a three digit suffix while bookmark creation times do not. This leads to the comparison always being true. --- syncoid | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/syncoid b/syncoid index 5c08120..bc7b3dc 100755 --- a/syncoid +++ b/syncoid @@ -699,11 +699,9 @@ sub syncdataset { # do an replication to it and replicate as always from oldest to newest # because bookmark sends doesn't support intermediates directly foreach my $snap ( sort { sortsnapshots(\%snaps, $a, $b, 'source') } keys %{ $snaps{'source'} }) { - my $comparisonkey = 'creation'; - if (defined $snaps{'source'}{$snap}{'createtxg'} && defined $bookmark{'createtxg'}) { - $comparisonkey = 'createtxg'; - } - if ($snaps{'source'}{$snap}{$comparisonkey} >= $bookmark{$comparisonkey}) { + my $comparetxg = defined $snaps{'source'}{$snap}{'createtxg'} && defined $bookmark{'createtxg'}; + if (($comparetxg && $snaps{'source'}{$snap}{'createtxg'} >= $bookmark{'createtxg'}) || + substr($snaps{'source'}{$snap}{'creation'}, 0, -3) >= $bookmark{'creation'}) { $nextsnapshot = $snap; last; } From 89b98ce11302406a6c9495de2b3e5e0e5e159645 Mon Sep 17 00:00:00 2001 From: 0xFelix Date: Sun, 14 Mar 2021 18:11:01 +0100 Subject: [PATCH 4/4] Add test for bookmarks created after the latest snapshot --- .../run.sh | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100755 tests/syncoid/11_use_bookmarks_created_after_latest_snapshot/run.sh diff --git a/tests/syncoid/11_use_bookmarks_created_after_latest_snapshot/run.sh b/tests/syncoid/11_use_bookmarks_created_after_latest_snapshot/run.sh new file mode 100755 index 0000000..20f2b5e --- /dev/null +++ b/tests/syncoid/11_use_bookmarks_created_after_latest_snapshot/run.sh @@ -0,0 +1,62 @@ +#!/bin/bash + +# test using bookmark created after last snapshot + +set -x +set -e + +. ../../common/lib.sh + +POOL_IMAGE="/tmp/syncoid-test-11.zpool" +MOUNT_TARGET="/tmp/syncoid-test-11.mount" +POOL_SIZE="1000M" +POOL_NAME="syncoid-test-11" +TARGET_CHECKSUM="9791444505ef5ab4ac8c943cdcbbb99b98fefc0ee658ac048505cc647e25a1f6 -" + +truncate -s "${POOL_SIZE}" "${POOL_IMAGE}" + +zpool create -m "${MOUNT_TARGET}" -f "${POOL_NAME}" "${POOL_IMAGE}" + +function cleanUp { + zpool export "${POOL_NAME}" +} + +# export pool in any case +trap cleanUp EXIT + +zfs create "${POOL_NAME}"/a +zfs snapshot "${POOL_NAME}"/a@s0 + +# This fully replicates a to b +../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b + +echo "Test 1" > "${MOUNT_TARGET}"/a/file1 +zfs snapshot "${POOL_NAME}"/a@s1 + +# This incrementally replicates from a@s0 to a@s1 +../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b + +echo "Test 2" > "${MOUNT_TARGET}"/a/file2 +zfs snapshot "${POOL_NAME}"/a@s2 + +# Destroy latest common snap between a and b +zfs destroy "${POOL_NAME}"/a@s1 + +# This uses a#s1 as base snap although common but older snap a@s0 exists +../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b + +echo "Test 3" > "${MOUNT_TARGET}"/a/file3 +zfs snapshot "${POOL_NAME}"/a@s3 + +# This uses a@s2 as base snap again +../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b + +# verify +output=$(zfs list -t snapshot -r -H -o name "${POOL_NAME}") +checksum=$(echo "${output}" | shasum -a 256) + +if [ "${checksum}" != "${TARGET_CHECKSUM}" ]; then + exit 1 +fi + +exit 0