fix(syncoid): Harden bookmark replication against timestamp non-monotony

* Replaced direct sorting on the `creation` property with calls to the
  `sortsnapshots()` helper subroutine. As with other usages, this
  ensures that when `syncoid` searches for the next snapshot to
  replicate from a bookmark, it preferentially uses the monotonic
  `createtxg` for sorting.
* Refactored the variables holding bookmark details from separate
  scalars (`$bookmark`, `$bookmarkcreation`) into a single hash
  (`%bookmark`). This allows for cleaner handling of all
  relevant bookmark properties (`name`, `creation`, `createtxg`).
* Fixed a code comment that incorrectly described the snapshot search
  order. The search for a matching target snapshot now correctly states
  it proceeds from newest-to-oldest to find the most recent common
  ancestor.
This commit is contained in:
Nick Liu 2025-06-11 12:40:57 -05:00
parent be52f8ab1e
commit 258a664dc0
No known key found for this signature in database
GPG Key ID: 1167C5F9C9897637
1 changed files with 17 additions and 16 deletions

33
syncoid
View File

@ -578,28 +578,26 @@ 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) {
# 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 { $snaps{'target'}{$b}{'creation'}<=>$snaps{'target'}{$a}{'creation'} } keys %{ $snaps{'target'} }) {
# check for matching guid of source bookmark and target snapshot (newest first)
foreach my $snap ( sort { sortsnapshots($snaps{'target'}, $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");
@ -672,15 +670,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{'source'}, $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;
}
@ -688,13 +689,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;
@ -708,13 +709,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;
@ -729,7 +730,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;