From e301b5b153a33d37f0ab062a90531226a718e1a4 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 25 Apr 2023 13:58:40 -0500 Subject: [PATCH 1/6] refactor(syncoid): Simplify getsnaps to parse a hash rather than lines * The part that was "a little obnoxious" has been rewritten to extract the desired properties in a single loop after importing each line into a hash rather than processing line by line with a state tracking flag. * The `getsnapsfallback` subroutine had duplicated logic that has been absorbed into `getsnaps` with a recursion argument to enable the fallback mode. --- syncoid | 158 ++++++++++++-------------------------------------------- 1 file changed, 33 insertions(+), 125 deletions(-) diff --git a/syncoid b/syncoid index 577df56..3890dee 100755 --- a/syncoid +++ b/syncoid @@ -415,10 +415,10 @@ sub syncdataset { if (!defined($receivetoken)) { # build hashes of the snaps on the source and target filesystems. - %snaps = getsnaps('source',$sourcehost,$sourcefs,$sourceisroot); + %snaps = getsnaps('source',$sourcehost,$sourcefs,$sourceisroot,0); if ($targetexists) { - my %targetsnaps = getsnaps('target',$targethost,$targetfs,$targetisroot); + my %targetsnaps = getsnaps('target',$targethost,$targetfs,$targetisroot,0); my %sourcesnaps = %snaps; %snaps = (%sourcesnaps, %targetsnaps); } @@ -1803,21 +1803,22 @@ sub dumphash() { writelog('INFO', Dumper($hash)); } -sub getsnaps() { - my ($type,$rhost,$fs,$isroot,%snaps) = @_; +sub getsnaps { + my ($type,$rhost,$fs,$isroot,$use_fallback,%snaps) = @_; my $mysudocmd; my $fsescaped = escapeshellparam($fs); if ($isroot) { $mysudocmd = ''; } else { $mysudocmd = $sudocmd; } - my $rhostOriginal = $rhost; - if ($rhost ne '') { $rhost = "$sshcmd $rhost"; # double escaping needed $fsescaped = escapeshellparam($fsescaped); } - my $getsnapcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 -t snapshot guid,creation $fsescaped"; + my $getsnapcmd = $use_fallback + ? "$rhost $mysudocmd $zfscmd get -Hpd 1 type,guid,creation $fsescaped" + : "$rhost $mysudocmd $zfscmd get -Hpd 1 -t snapshot guid,creation $fsescaped"; + if ($debug) { $getsnapcmd = "$getsnapcmd |"; writelog('DEBUG', "getting list of snapshots on $fs using $getsnapcmd..."); @@ -1827,141 +1828,48 @@ sub getsnaps() { open FH, $getsnapcmd; my @rawsnaps = ; close FH or do { - # fallback (solaris for example doesn't support the -t option) - return getsnapsfallback($type,$rhostOriginal,$fs,$isroot,%snaps); + if (!$use_fallback) { + writelog('WARN', "snapshot listing failed, trying fallback command"); + return getsnaps($type, $rhost, $fs, $isroot, 1, %snaps); + } + die "CRITICAL ERROR: snapshots couldn't be listed for $fs (exit code $?)"; }; - # this is a little obnoxious. get guid,creation returns guid,creation on two separate lines - # as though each were an entirely separate get command. + my %snap_data; + my %creationtimes; - my %creationtimes=(); + for my $line (@rawsnaps) { + chomp $line; + my ($dataset, $property, $value) = split /\t/, $line; + my ($fs, $snap) = split /@/, $dataset; + if (!snapisincluded($snap)) { next; } + $snap_data{$snap}{$property} = $value; - foreach my $line (@rawsnaps) { - $line =~ /\Q$fs\E\@(\S*)/; - my $snapname = $1; - - if (!snapisincluded($snapname)) { next; } - - # only import snap guids from the specified filesystem - if ($line =~ /\Q$fs\E\@.*\tguid/) { - chomp $line; - my $guid = $line; - $guid =~ s/^.*\tguid\t*(\d*).*/$1/; - my $snap = $line; - $snap =~ s/^.*\@(.*)\tguid.*$/$1/; - $snaps{$type}{$snap}{'guid'}=$guid; - } - # only import snap creations from the specified filesystem - elsif ($line =~ /\Q$fs\E\@.*\tcreation/) { - chomp $line; - my $creation = $line; - $creation =~ s/^.*\tcreation\t*(\d*).*/$1/; - my $snap = $line; - $snap =~ s/^.*\@(.*)\tcreation.*$/$1/; - - # the accuracy of the creation timestamp is only for a second, but - # snapshots in the same second are highly likely. The list command - # has an ordered output so we append another three digit running number - # to the creation timestamp and make sure those are ordered correctly - # for snapshot with the same creation timestamp + # the accuracy of the creation timestamp is only for a second, but + # snapshots in the same second are highly likely. The list command + # has an ordered output so we append another three digit running number + # to the creation timestamp and make sure those are ordered correctly + # for snapshot with the same creation timestamp + if ($property eq 'creation') { my $counter = 0; my $creationsuffix; while ($counter < 999) { - $creationsuffix = sprintf("%s%03d", $creation, $counter); + $creationsuffix = sprintf("%s%03d", $value, $counter); if (!defined $creationtimes{$creationsuffix}) { $creationtimes{$creationsuffix} = 1; last; } $counter += 1; } - - $snaps{$type}{$snap}{'creation'}=$creationsuffix; + $snap_data{$snap}{'creation'} = $creationsuffix; } } - return %snaps; -} - -sub getsnapsfallback() { - # fallback (solaris for example doesn't support the -t option) - my ($type,$rhost,$fs,$isroot,%snaps) = @_; - my $mysudocmd; - my $fsescaped = escapeshellparam($fs); - if ($isroot) { $mysudocmd = ''; } else { $mysudocmd = $sudocmd; } - - if ($rhost ne '') { - $rhost = "$sshcmd $rhost"; - # double escaping needed - $fsescaped = escapeshellparam($fsescaped); - } - - my $getsnapcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 type,guid,creation $fsescaped |"; - writelog('WARN', "snapshot listing failed, trying fallback command"); - writelog('DEBUG', "FALLBACK, getting list of snapshots on $fs using $getsnapcmd..."); - open FH, $getsnapcmd; - my @rawsnaps = ; - close FH or die "CRITICAL ERROR: snapshots couldn't be listed for $fs (exit code $?)"; - - my %creationtimes=(); - - my $state = 0; - foreach my $line (@rawsnaps) { - if ($state < 0) { - $state++; - next; + for my $snap (keys %snap_data) { + if (!$use_fallback || $snap_data{$snap}{'type'} eq 'snapshot') { + $snaps{$type}{$snap}{'guid'} = $snap_data{$snap}{'guid'}; + $snaps{$type}{$snap}{'creation'} = $snap_data{$snap}{'creation'}; } - - if ($state eq 0) { - if ($line !~ /\Q$fs\E\@.*\ttype\s*snapshot/) { - # skip non snapshot type object - $state = -2; - next; - } - } elsif ($state eq 1) { - if ($line !~ /\Q$fs\E\@.*\tguid/) { - die "CRITICAL ERROR: snapshots couldn't be listed for $fs (guid parser error)"; - } - - chomp $line; - my $guid = $line; - $guid =~ s/^.*\tguid\t*(\d*).*/$1/; - my $snap = $line; - $snap =~ s/^.*\@(.*)\tguid.*$/$1/; - if (!snapisincluded($snap)) { next; } - $snaps{$type}{$snap}{'guid'}=$guid; - } elsif ($state eq 2) { - if ($line !~ /\Q$fs\E\@.*\tcreation/) { - die "CRITICAL ERROR: snapshots couldn't be listed for $fs (creation parser error)"; - } - - chomp $line; - my $creation = $line; - $creation =~ s/^.*\tcreation\t*(\d*).*/$1/; - my $snap = $line; - $snap =~ s/^.*\@(.*)\tcreation.*$/$1/; - if (!snapisincluded($snap)) { next; } - - # the accuracy of the creation timestamp is only for a second, but - # snapshots in the same second are highly likely. The list command - # has an ordered output so we append another three digit running number - # to the creation timestamp and make sure those are ordered correctly - # for snapshot with the same creation timestamp - my $counter = 0; - my $creationsuffix; - while ($counter < 999) { - $creationsuffix = sprintf("%s%03d", $creation, $counter); - if (!defined $creationtimes{$creationsuffix}) { - $creationtimes{$creationsuffix} = 1; - last; - } - $counter += 1; - } - - $snaps{$type}{$snap}{'creation'}=$creationsuffix; - $state = -1; - } - - $state++; } return %snaps; From 8fabaae5b8efd04c2df9e4f6fdcdb82720dd963e Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 25 Apr 2023 14:01:54 -0500 Subject: [PATCH 2/6] feat(syncoid): Add "createtxg" property to `getsnaps` The `getsnaps` subroutine now retrieves the "createtxg" property of the snapshot. This is necessary to support the fix for https://github.com/jimsalterjrs/sanoid/issues/815 (Syncoid: Data loss because getoldestsnapshot() might not choose the first snapshot). --- syncoid | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/syncoid b/syncoid index 3890dee..3a5a9e4 100755 --- a/syncoid +++ b/syncoid @@ -1816,8 +1816,8 @@ sub getsnaps { } my $getsnapcmd = $use_fallback - ? "$rhost $mysudocmd $zfscmd get -Hpd 1 type,guid,creation $fsescaped" - : "$rhost $mysudocmd $zfscmd get -Hpd 1 -t snapshot guid,creation $fsescaped"; + ? "$rhost $mysudocmd $zfscmd get -Hpd 1 all $fsescaped" + : "$rhost $mysudocmd $zfscmd get -Hpd 1 -t snapshot all $fsescaped"; if ($debug) { $getsnapcmd = "$getsnapcmd |"; @@ -1841,8 +1841,13 @@ sub getsnaps { for my $line (@rawsnaps) { chomp $line; my ($dataset, $property, $value) = split /\t/, $line; - my ($fs, $snap) = split /@/, $dataset; - if (!snapisincluded($snap)) { next; } + die "CRITICAL ERROR: Unexpected line format in $line" unless defined $value; + + my (undef, $snap) = split /@/, $dataset; + die "CRITICAL ERROR: Unexpected dataset format in $line" unless $snap; + + if (!snapisincluded($snap)) { next; } + $snap_data{$snap}{$property} = $value; # the accuracy of the creation timestamp is only for a second, but @@ -1868,6 +1873,7 @@ sub getsnaps { for my $snap (keys %snap_data) { if (!$use_fallback || $snap_data{$snap}{'type'} eq 'snapshot') { $snaps{$type}{$snap}{'guid'} = $snap_data{$snap}{'guid'}; + $snaps{$type}{$snap}{'createtxg'} = $snap_data{$snap}{'createtxg'}; $snaps{$type}{$snap}{'creation'} = $snap_data{$snap}{'creation'}; } } From 8907e0cb2f2be688743e9e004ccdbad3a613498c Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Fri, 28 Apr 2023 00:43:47 -0500 Subject: [PATCH 3/6] feat(syncoid): Sort snapshots by `createtxg` if possible It is possible for `creation` of a subsequent snapshot to be in the past compared to the current snapshot due to system clock discrepancies, which leads to earlier snapshots not being replicated in the initial syncoid sync. Also, `syncoid --no-sync-snap` might not pick up the most recently taken snapshot if the clock moved backwards before taking that snapshot. Sorting snapshots by the `createtxg` value is reliable and documented in `man 8 zfsprops` as the proper way to order snapshots, but it was not available in ZFS versions before 0.7. To maintain backwards compatibility, the sorting falls back to sorting by the `creation` property, which was the old behavior. Fixes: https://github.com/jimsalterjrs/sanoid/issues/815 --- syncoid | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/syncoid b/syncoid index 3a5a9e4..44e8486 100755 --- a/syncoid +++ b/syncoid @@ -862,9 +862,9 @@ sub syncdataset { if (defined $args{'delete-target-snapshots'}) { # Find the snapshots that exist on the target, filter with # those that exist on the source. Remaining are the snapshots - # that are only on the target. Then sort by creation date, as - # to remove the oldest snapshots first. - my @to_delete = sort { $snaps{'target'}{$a}{'creation'}<=>$snaps{'target'}{$b}{'creation'} } grep {!exists $snaps{'source'}{$_}} keys %{ $snaps{'target'} }; + # 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'} }; while (@to_delete) { # Create batch of snapshots to remove my $snaps = join ',', splice(@to_delete, 0, 50); @@ -1480,9 +1480,17 @@ sub readablebytes { return $disp; } +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'}; + } + return $snaps->{'source'}{$left}{'creation'} <=> $snaps->{'source'}{$right}{'creation'}; +} + sub getoldestsnapshot { my $snaps = shift; - foreach my $snap ( sort { $snaps{'source'}{$a}{'creation'}<=>$snaps{'source'}{$b}{'creation'} } keys %{ $snaps{'source'} }) { + foreach my $snap (sort { sortsnapshots($snaps, $a, $b) } keys %{ $snaps{'source'} }) { # return on first snap found - it's the oldest return $snap; } @@ -1496,7 +1504,7 @@ sub getoldestsnapshot { sub getnewestsnapshot { my $snaps = shift; - foreach my $snap ( sort { $snaps{'source'}{$b}{'creation'}<=>$snaps{'source'}{$a}{'creation'} } keys %{ $snaps{'source'} }) { + foreach my $snap (sort { sortsnapshots($snaps, $b, $a) } keys %{ $snaps{'source'} }) { # return on first snap found - it's the newest writelog('INFO', "NEWEST SNAPSHOT: $snap"); return $snap; @@ -1675,7 +1683,7 @@ sub pruneoldsyncsnaps { sub getmatchingsnapshot { my ($sourcefs, $targetfs, $snaps) = @_; - foreach my $snap ( sort { $snaps{'source'}{$b}{'creation'}<=>$snaps{'source'}{$a}{'creation'} } keys %{ $snaps{'source'} }) { + foreach my $snap ( sort { sortsnapshots($snaps, $b, $a) } keys %{ $snaps{'source'} }) { if (defined $snaps{'target'}{$snap}) { if ($snaps{'source'}{$snap}{'guid'} == $snaps{'target'}{$snap}{'guid'}) { return $snap; From ab361017e7d3eb5e140d5ce14eafbc41c91d44d7 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 25 Apr 2023 17:07:32 -0500 Subject: [PATCH 4/6] feat(syncoid): Match snapshots to bookmarks by `createtxg` if possible This is a continuation of a previous commit to sort snapshots by `createtxg` if possible. Now, we have to match the behavior when selecting an appropriate snapshot based on the transaction group of the relevant bookmark in `syncdataset()`. Supersedes: https://github.com/jimsalterjrs/sanoid/pull/667 --- syncoid | 89 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/syncoid b/syncoid index 44e8486..61814d2 100755 --- a/syncoid +++ b/syncoid @@ -438,7 +438,7 @@ sub syncdataset { # Don't send the sync snap if it's filtered out by --exclude-snaps or # --include-snaps if (!snapisincluded($newsyncsnap)) { - $newsyncsnap = getnewestsnapshot($sourcehost,$sourcefs,$sourceisroot); + $newsyncsnap = getnewestsnapshot(\%snaps); if ($newsyncsnap eq 0) { writelog('WARN', "CRITICAL: no snapshots exist on source $sourcefs, and you asked for --no-sync-snap."); if ($exitcode < 1) { $exitcode = 1; } @@ -447,7 +447,7 @@ sub syncdataset { } } else { # we don't want sync snapshots created, so use the newest snapshot we can find. - $newsyncsnap = getnewestsnapshot($sourcehost,$sourcefs,$sourceisroot); + $newsyncsnap = getnewestsnapshot(\%snaps); if ($newsyncsnap eq 0) { writelog('WARN', "CRITICAL: no snapshots exist on source $sourcefs, and you asked for --no-sync-snap."); if ($exitcode < 1) { $exitcode = 1; } @@ -584,8 +584,7 @@ sub syncdataset { my $targetsize = getzfsvalue($targethost,$targetfs,$targetisroot,'-p used'); - my $bookmark = 0; - my $bookmarkcreation = 0; + my %bookmark = (); $matchingsnap = getmatchingsnapshot($sourcefs, $targetfs, \%snaps); if (! $matchingsnap) { @@ -593,19 +592,18 @@ sub syncdataset { my %bookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot); # check for matching guid of source bookmark and target snapshot (oldest first) - foreach my $snap ( sort { $snaps{'target'}{$b}{'creation'}<=>$snaps{'target'}{$a}{'creation'} } keys %{ $snaps{'target'} }) { + foreach my $snap ( sort { sortsnapshots(\%snaps, $b, $a) } keys %{ $snaps{'target'} }) { my $guid = $snaps{'target'}{$snap}{'guid'}; if (defined $bookmarks{$guid}) { # found a match - $bookmark = $bookmarks{$guid}{'name'}; - $bookmarkcreation = $bookmarks{$guid}{'creation'}; + %bookmark = %{ $bookmarks{$guid} }; $matchingsnap = $snap; last; } } - if (! $bookmark) { + if (! %bookmark) { # force delete is not possible for the root dataset if ($args{'force-delete'} && index($targetfs, '/') != -1) { writelog('INFO', "Removing $targetfs because no matching snapshots were found"); @@ -678,15 +676,18 @@ sub syncdataset { my $nextsnapshot = 0; - if ($bookmark) { - my $bookmarkescaped = escapeshellparam($bookmark); + if (%bookmark) { if (!defined $args{'no-stream'}) { # 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 { $snaps{'source'}{$a}{'creation'}<=>$snaps{'source'}{$b}{'creation'} } keys %{ $snaps{'source'} }) { - if ($snaps{'source'}{$snap}{'creation'} >= $bookmarkcreation) { + foreach my $snap ( sort { sortsnapshots(\%snaps, $a, $b) } keys %{ $snaps{'source'} }) { + my $comparisonkey = 'creation'; + if (defined $snaps{'source'}{$snap}{'createtxg'} && defined $bookmark{'createtxg'}) { + $comparisonkey = 'createtxg'; + } + if ($snaps{'source'}{$snap}{$comparisonkey} >= $bookmark{$comparisonkey}) { $nextsnapshot = $snap; last; } @@ -694,13 +695,13 @@ sub syncdataset { } if ($nextsnapshot) { - ($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $nextsnapshot); + ($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $nextsnapshot); $exit == 0 or do { if (!$resume && $stdout =~ /\Qcontains partially-complete state\E/) { writelog('WARN', "resetting partially receive state"); resetreceivestate($targethost,$targetfs,$targetisroot); - (my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $nextsnapshot); + (my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $nextsnapshot); $ret == 0 or do { if ($exitcode < 2) { $exitcode = 2; } return 0; @@ -714,13 +715,13 @@ sub syncdataset { $matchingsnap = $nextsnapshot; $matchingsnapescaped = escapeshellparam($matchingsnap); } else { - ($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $newsyncsnap); + ($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $newsyncsnap); $exit == 0 or do { if (!$resume && $stdout =~ /\Qcontains partially-complete state\E/) { writelog('WARN', "resetting partially receive state"); resetreceivestate($targethost,$targetfs,$targetisroot); - (my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $newsyncsnap); + (my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $newsyncsnap); $ret == 0 or do { if ($exitcode < 2) { $exitcode = 2; } return 0; @@ -735,7 +736,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 (!%bookmark || $nextsnapshot) { if ($matchingsnap eq $newsyncsnap) { # edge case: bookmark replication used the latest snapshot return 0; @@ -1902,7 +1903,7 @@ sub getbookmarks() { } my $error = 0; - my $getbookmarkcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 -t bookmark guid,creation $fsescaped 2>&1 |"; + my $getbookmarkcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 -t bookmark all $fsescaped 2>&1 |"; writelog('DEBUG', "getting list of bookmarks on $fs using $getbookmarkcmd..."); open FH, $getbookmarkcmd; my @rawbookmarks = ; @@ -1917,48 +1918,46 @@ sub getbookmarks() { die "CRITICAL ERROR: bookmarks couldn't be listed for $fs (exit code $?)"; } - # this is a little obnoxious. get guid,creation returns guid,creation on two separate lines - # as though each were an entirely separate get command. + my %bookmark_data; + my %creationtimes; - my $lastguid; - my %creationtimes=(); + for my $line (@rawbookmarks) { + chomp $line; + my ($dataset, $property, $value) = split /\t/, $line; + die "CRITICAL ERROR: Unexpected line format in $line" unless defined $value; - foreach my $line (@rawbookmarks) { - # only import bookmark guids, creation from the specified filesystem - if ($line =~ /\Q$fs\E\#.*\tguid/) { - chomp $line; - $lastguid = $line; - $lastguid =~ s/^.*\tguid\t*(\d*).*/$1/; - my $bookmark = $line; - $bookmark =~ s/^.*\#(.*)\tguid.*$/$1/; - $bookmarks{$lastguid}{'name'}=$bookmark; - } elsif ($line =~ /\Q$fs\E\#.*\tcreation/) { - chomp $line; - my $creation = $line; - $creation =~ s/^.*\tcreation\t*(\d*).*/$1/; - my $bookmark = $line; - $bookmark =~ s/^.*\#(.*)\tcreation.*$/$1/; + my (undef, $bookmark) = split /#/, $dataset; + die "CRITICAL ERROR: Unexpected dataset format in $line" unless $bookmark; - # the accuracy of the creation timestamp is only for a second, but - # bookmarks in the same second are possible. The list command - # has an ordered output so we append another three digit running number - # to the creation timestamp and make sure those are ordered correctly - # for bookmarks with the same creation timestamp + $bookmark_data{$bookmark}{$property} = $value; + + # the accuracy of the creation timestamp is only for a second, but + # bookmarks in the same second are possible. The list command + # has an ordered output so we append another three digit running number + # to the creation timestamp and make sure those are ordered correctly + # for bookmarks with the same creation timestamp + if ($property eq 'creation') { my $counter = 0; my $creationsuffix; while ($counter < 999) { - $creationsuffix = sprintf("%s%03d", $creation, $counter); + $creationsuffix = sprintf("%s%03d", $value, $counter); if (!defined $creationtimes{$creationsuffix}) { $creationtimes{$creationsuffix} = 1; last; } $counter += 1; } - - $bookmarks{$lastguid}{'creation'}=$creationsuffix; + $bookmark_data{$bookmark}{'creation'} = $creationsuffix; } } + for my $bookmark (keys %bookmark_data) { + my $guid = $bookmark_data{$bookmark}{'guid'}; + $bookmarks{$guid}{'name'} = $bookmark; + $bookmarks{$guid}{'creation'} = $bookmark_data{$bookmark}{'creation'}; + $bookmarks{$guid}{'createtxg'} = $bookmark_data{$bookmark}{'createtxg'}; + } + return %bookmarks; } From b37092f37681467ae000b59be7ca61ef6971c586 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 25 Apr 2023 17:35:45 -0500 Subject: [PATCH 5/6] test(syncoid): Add test to verify out-of-order snapshot sync See https://github.com/jimsalterjrs/sanoid/issues/815 for the original test. --- .../815_sync_out-of-order_snapshots/run.sh | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100755 tests/syncoid/815_sync_out-of-order_snapshots/run.sh diff --git a/tests/syncoid/815_sync_out-of-order_snapshots/run.sh b/tests/syncoid/815_sync_out-of-order_snapshots/run.sh new file mode 100755 index 0000000..af67b36 --- /dev/null +++ b/tests/syncoid/815_sync_out-of-order_snapshots/run.sh @@ -0,0 +1,46 @@ +#!/bin/bash + +# test verifying snapshots with out-of-order snapshot creation datetimes + +set -x +set -e + +. ../../common/lib.sh + +POOL_IMAGE="/tmp/jimsalterjrs_sanoid_815.img" +POOL_SIZE="64M" +POOL_NAME="jimsalterjrs_sanoid_815" + +truncate -s "${POOL_SIZE}" "${POOL_IMAGE}" + +zpool create -m none -f "${POOL_NAME}" "${POOL_IMAGE}" + +function cleanUp { + zpool export "${POOL_NAME}" + rm -f "${POOL_IMAGE}" +} + +# export pool and remove the image in any case +trap cleanUp EXIT + +zfs create "${POOL_NAME}"/before +zfs snapshot "${POOL_NAME}"/before@this-snapshot-should-make-it-into-the-after-dataset + +disableTimeSync +setdate 1155533696 +zfs snapshot "${POOL_NAME}"/before@oldest-snapshot + +zfs snapshot "${POOL_NAME}"/before@another-snapshot-does-not-matter +../../../syncoid --sendoptions="Lec" "${POOL_NAME}"/before "${POOL_NAME}"/after + +# verify +saveSnapshotList "${POOL_NAME}" "snapshot-list.txt" + +grep "${POOL_NAME}/before@this-snapshot-should-make-it-into-the-after-dataset" "snapshot-list.txt" || exit $? +grep "${POOL_NAME}/after@this-snapshot-should-make-it-into-the-after-dataset" "snapshot-list.txt" || exit $? +grep "${POOL_NAME}/before@oldest-snapshot" "snapshot-list.txt" || exit $? +grep "${POOL_NAME}/after@oldest-snapshot" "snapshot-list.txt" || exit $? +grep "${POOL_NAME}/before@another-snapshot-does-not-matter" "snapshot-list.txt" || exit $? +grep "${POOL_NAME}/after@another-snapshot-does-not-matter" "snapshot-list.txt" || exit $? + +exit 0 From a904ba02f3158b971786f2e864fabe5227075cc8 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Fri, 28 Apr 2023 01:00:03 -0500 Subject: [PATCH 6/6] enh(run-tests.sh): Sort tests with "general numeric sort" The sort before tended to be alphabetical, which put test `8_force_delete_snapshot` after `815_sync_out-of-order_snapshots`, but `8` should come before `815`. Before: ``` root@demo:~/sanoid/tests/syncoid# ./run-tests.sh Running test 1_bookmark_replication_intermediate ... [PASS] Running test 2_bookmark_replication_no_intermediate ... [PASS] Running test 3_force_delete ... [PASS] Running test 4_bookmark_replication_edge_case ... [PASS] Running test 5_reset_resume_state ... mbuffer: error: outputThread: error writing to at offset 0x90000: Broken pipe mbuffer: warning: error during output to : Broken pipe [PASS] Running test 6_reset_resume_state2 ... [PASS] Running test 7_preserve_recordsize ... [PASS] Running test 815_sync_out-of-order_snapshots ... [PASS] Running test 8_force_delete_snapshot ... [PASS] ``` After: ``` root@demo:~/sanoid/tests/syncoid# ./run-tests.sh Running test 1_bookmark_replication_intermediate ... [PASS] Running test 2_bookmark_replication_no_intermediate ... [PASS] Running test 3_force_delete ... [PASS] Running test 4_bookmark_replication_edge_case ... [PASS] Running test 5_reset_resume_state ... mbuffer: error: outputThread: error writing to at offset 0xf0000: Broken pipe mbuffer: warning: error during output to : Broken pipe [PASS] Running test 6_reset_resume_state2 ... [PASS] Running test 7_preserve_recordsize ... [PASS] Running test 8_force_delete_snapshot ... [PASS] Running test 815_sync_out-of-order_snapshots ... [PASS] ``` --- tests/run-tests.sh | 2 +- tests/syncoid/run-tests.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 38054b0..418657c 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -2,7 +2,7 @@ # run's all the available tests -for test in */; do +for test in $(find . -mindepth 1 -maxdepth 1 -type d -printf "%P\n" | sort -g); do if [ ! -x "${test}/run.sh" ]; then continue fi diff --git a/tests/syncoid/run-tests.sh b/tests/syncoid/run-tests.sh index a9843a5..5564667 100755 --- a/tests/syncoid/run-tests.sh +++ b/tests/syncoid/run-tests.sh @@ -2,7 +2,7 @@ # run's all the available tests -for test in */; do +for test in $(find . -mindepth 1 -maxdepth 1 -type d -printf "%P\n" | sort -g); do if [ ! -x "${test}/run.sh" ]; then continue fi