[pve-devel] [pve-zsync] Discussion/Feedback: Extending pve-zsync to handle intervening snapshots

[Bruce Wainer]

Hi Wolfgang,
Understood re: not using a flag and new copies of the functions. The reason
I thought to do it that way, is that this can be seen as a change in
behavior, even if the new behavior yields the same result. I come from an
environment where any change shouldn't happen without the administrator
asking for it to. But if the Proxmox developers/maintainers would accept
this change to the default behavior, I'm happy to reduce the complexity and
work needed for the change.
I'll start working on programming, and setting up some tests on real
servers. Are there any automated test cases related to pve-zync?
Thank you,
Bruce

On Tue, Jun 16, 2020 at 1:36 AM Wolfgang Link <w.link at proxmox.com> wrote:

> Hello,
>
> I think that's a good idea.
>
> However, I am not sure whether the additional flag described in point 1 is
> necessary,
> since old jobs already have an independent dest dataset.
> So the new logic should not conflict it.
>
> I think it would be great to rewrite the "snapshot_get"
> and "snapshot_exists" functions to fit this use case,
> In my opinion it makes little sense to copy the function
> and only change dest and source and give the function a new name.
>
> Regards,
> Wolfgang
>
> > On 06/15/2020 9:50 PM Bruce Wainer <brwainer at gmail.com> wrote:
> >
> >
> > Hello,
> >
> > I am considering making an improvement to pve-zsync and submitting it as
> a
> > patch, but I would like some feedback before doing so.
> >
> > Problem:
> > pve-zsync can't be used multiple times between the same source and
> > destination pool. For example, these two commands are allowed:
> > pve-zsync create --source 192.168.10.18:101 --name 101-daily --dest
> > 192.168.10.14:HDDs/replica-daily
> > pve-zsync create --source 192.168.10.18:101 --name 101-weekly --dest
> > 192.168.10.14:HDDs/replica-weekly
> > But these two are not:
> > pve-zsync create --source 192.168.10.18:101 --name 101-daily --dest
> > 192.168.10.14:HDDs/replica
> > pve-zsync create --source 192.168.10.18:101 --name 101-weekly --dest
> > 192.168.10.14:HDDs/replica
> >
> > The first set of commands work together because the destination volumes
> are
> > different. However, this means that the destination host will have
> > duplicate copies of the source. This is not ideal. The second set of
> > commands will not work because the last_snap used for the zfs send/recv
> > only takes into account the current job.
> >
> > Proposal
> > Flip the source and destination in snapshot_get() and snapshot_exists(),
> > because if the last snapshot on the destination doesn't exist on the
> > source, the sync is going to fail anyway (ZFS will error with "cannot
> > receive new filesystem stream: destination has snapshots").
> > 1. Add a new command option "use_last_dest_snapshot" (add to help, to
> cron
> > generation, etc). If this is set, the new logic will be used.
> > 2. Add "sub snapshot_get_dest" which is similar to snapshot_get() except
> it
> > performs its checks on the destination, not the source. Also, last_snap
> > would be the last snapshot of all, not just the last one related to the
> > current job.
> > 3. Add "sub snapshot_exists_source" which is the same as
> snapshot_exists()
> > except it performs its checks on the source, not the destination. I would
> > also improve the error message if the snapshot doesn't exist on the
> source.
> > 4. In "sub send_image", if " use_last_dest_snapshot " is set then the new
> > functions would be called, otherwise the old functions would be called.
> >
> > This will be my first time submitting changes to a project, but I
> believe I
> > am capable of writing code that meets your code standards. I am aware of
> > the style guide and will submit the CLA.
> >
> > Please let me know what you think about my proposed changes.
> >
> > Thank you,
> > Bruce Wainer
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel at pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>