Syncoid: Data loss because getoldestsnapshot() might not choose the first snapshot

[Deltik]

Bug Description

Due to an incorrect clock at the time, I have some snapshots where the creation property is in the past compared to earlier snapshots.

When syncoid performs an initial full transfer of a dataset, it could select a snapshot that isn't the first snapshot of the dataset because of the usage of getoldestsnapshot(). This leads to data loss, as some snapshots may be left behind.

How to Reproduce

See the bold parts of the output below:

root@box52:~# truncate -s64M /tmp/jimsalterjrs_sanoid_815.img
root@box52:~# zpool create jimsalterjrs_sanoid_815 /tmp/jimsalterjrs_sanoid_815.img
root@box52:~# zfs create jimsalterjrs_sanoid_815/before
root@box52:~# zfs snap jimsalterjrs_sanoid_815/before@this-snapshot-does-not-make-it-into-the-after-dataset
root@box52:~# date --set="2006-08-14T02:34:56-06:00" ; zfs snap jimsalterjrs_sanoid_815/before@oldest-snapshot ; date
Mon Aug 14 03:34:56 AM CDT 2006
Mon Aug 14 03:34:56 AM CDT 2006
root@box52:~# systemctl restart systemd-timedated.service
root@box52:~# date
Thu Apr 13 11:38:36 AM CDT 2023
root@box52:~# zfs snap jimsalterjrs_sanoid_815/before@another-snapshot-does-not-matter
root@box52:~# syncoid --sendoptions="Lec" jimsalterjrs_sanoid_815/{before,after}
INFO: Sending oldest full snapshot jimsalterjrs_sanoid_815/before@oldest-snapshot (~ 12 KB) to new target filesystem:
42.8KiB 0:00:00 [6.57MiB/s] [=============================================================================================] 339%            
INFO: Updating new target filesystem with incremental jimsalterjrs_sanoid_815/before@oldest-snapshot ... syncoid_box52.deltik.net_2023-04-13:11:38:51-GMT-05:00 (~ 4 KB):
2.13KiB 0:00:00 [ 136KiB/s] [================================================>                                             ] 53%            
root@box52:~# zfs list -rtall jimsalterjrs_sanoid_815
NAME                                                                                       USED  AVAIL     REFER  MOUNTPOINT
jimsalterjrs_sanoid_815                                                                    229K  23.8M       25K  /jimsalterjrs_sanoid_815
jimsalterjrs_sanoid_815/after                                                               24K  23.8M       24K  /jimsalterjrs_sanoid_815/after
jimsalterjrs_sanoid_815/after@oldest-snapshot                                                0B      -       24K  -
jimsalterjrs_sanoid_815/after@another-snapshot-does-not-matter                               0B      -       24K  -
jimsalterjrs_sanoid_815/after@syncoid_box52.deltik.net_2023-04-13:11:38:51-GMT-05:00         0B      -       24K  -
jimsalterjrs_sanoid_815/before                                                              24K  23.8M       24K  /jimsalterjrs_sanoid_815/before
jimsalterjrs_sanoid_815/before@this-snapshot-does-not-make-it-into-the-after-dataset         0B      -       24K  -
jimsalterjrs_sanoid_815/before@oldest-snapshot                                               0B      -       24K  -
jimsalterjrs_sanoid_815/before@another-snapshot-does-not-matter                              0B      -       24K  -
jimsalterjrs_sanoid_815/before@syncoid_box52.deltik.net_2023-04-13:11:38:51-GMT-05:00        0B      -       24K  -
root@box52:~# zfs get -r creation jimsalterjrs_sanoid_815
NAME                                                                                      PROPERTY  VALUE                  SOURCE
jimsalterjrs_sanoid_815                                                                   creation  Thu Apr 13 11:37 2023  -
jimsalterjrs_sanoid_815/after                                                             creation  Thu Apr 13 11:38 2023  -
jimsalterjrs_sanoid_815/after@oldest-snapshot                                             creation  Mon Aug 14  3:34 2006  -
jimsalterjrs_sanoid_815/after@another-snapshot-does-not-matter                            creation  Thu Apr 13 11:38 2023  -
jimsalterjrs_sanoid_815/after@syncoid_box52.deltik.net_2023-04-13:11:38:51-GMT-05:00      creation  Thu Apr 13 11:38 2023  -
jimsalterjrs_sanoid_815/before                                                            creation  Thu Apr 13 11:37 2023  -
jimsalterjrs_sanoid_815/before@this-snapshot-does-not-make-it-into-the-after-dataset      creation  Thu Apr 13 11:38 2023  -
jimsalterjrs_sanoid_815/before@oldest-snapshot                                            creation  Mon Aug 14  3:34 2006  -
jimsalterjrs_sanoid_815/before@another-snapshot-does-not-matter                           creation  Thu Apr 13 11:38 2023  -
jimsalterjrs_sanoid_815/before@syncoid_box52.deltik.net_2023-04-13:11:38:51-GMT-05:00     creation  Thu Apr 13 11:38 2023  -
root@box52:~# zpool export jimsalterjrs_sanoid_815
root@box52:~# rm -fv /tmp/jimsalterjrs_sanoid_815.img
removed '/tmp/jimsalterjrs_sanoid_815.img'

Expected Behavior

All snapshots are copied in the initial full sync, not just from the oldest-by-creation snapshot onwards.

[phreaker0]

imho we can require that the system time is correct and shouldn't work around configuration issues.

@Deltik but you can work around the described issue if you use the "--no-rollback" option. In you example syncoid will then exit with an error and you can fix the wrong system time.

[Deltik]

@phreaker0: I meant that if the system clock was wrong in the past, syncoid won't send all the snapshots on the initial sync. syncoid --no-rollback is irrelevant because the datasets don't exist on the destination yet.

[phreaker0]

I see but I think we still want to rely on the timestamps of snapshots. But if it would be possible without breaking anything else to switch to something like the transaction id of a snapshot or something stable we can consider it.

[Deltik]

Looking at the source, I see that getoldestsnapshot() is only used for the initial sync. I think the issue can be fixed simply by sorting by the createtxg property instead of the creation property. From man 8 zfsprops:

Native Properties

[…]

createtxg

The transaction group (txg) in which the dataset was created. Bookmarks have the same createtxg as the snapshot they are initially tied to. This property is suitable for ordering a list of snapshots, e.g. for incremental send and receive.