Add --include-snaps and --exclude-snaps options to syncoid #699
Conversation
|
I realized that my first push had some changes in the wrong commit, so I rebased to fix that. |
|
One thing you should clarify in the docs is whether the regex is matched against only the snapshot name or the entire string of |
|
@DarwinAwardWinner good point about the potential confusion between |
|
Note that you have a typo in a few places: rexeg instead of regex. Other than that, it looks good! |
|
Oops, thanks for catching that typo @DarwinAwardWinner. I edited the last commit to fix that. |
|
Would love to see this getting merged ... |
|
I've been testing this PR heavily on multiple ZFS hosts for the last 3 or 4 weeks. The new exclude functionality adds the only feature I was missing from sanoid/syncoid. Good work @mr-vinn |
|
I've also been using this branch on my systems as well. |
|
Running for one week now - no errors so far :)
Regards
Kilian
Am 08.01.2022 um 04:48 schrieb Ryan C. Thompson ***@***.***>:
I've also been using this branch on my systems as well.
—
Reply to this email directly, view it on GitHub<#699 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAT6Q7EJBZNJMW4OFHOVCJLUU6XYPANCNFSM5JDCCNNA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID: ***@***.***>
|
|
This would be a fantastic feature to have. Any chance this is getting merged soon? |
|
This is the number 1 feature that syncoid is missing. |
Replace `print` and `warn` statements with a logging function.
Build the zfs send and receive commands in a new subroutine, and implement other subroutines that can be called instead of building a zfs command and running it with system();
Process snapshots in one pass rather than looping separately for both guid and create time.
Add --include-snaps and --exclude-snaps options to filter the snapshots that syncoid uses.
Update the README with the new --include-snaps and --exclude-snaps syncoid options.
Test the new --include-snaps and --exclude-snaps options for syncoid.
Add a new option, --exclude-datasets, to replace --exclude. This makes the naming more consistent now that there are options to filter both snapshots and datasets. Also add more information to the README about the distinction between --exclude-datasets and --(in|ex)clude-snaps.
mr-vinn
force-pushed
the
filter-snaps
branch
from
c385248
to
0c577fc
Compare
|
Any updates on when this could get merged? I am also very excited about this functionality |
|
I just started using this branch for a couple datasets where I'm storing a lot of I'd like to suggest including some indication of the |
phreaker0
added
enhancement
revision needed
|
@mr-vinn i rebased your PR to the latest master to resolve conflicts. This looks really great and is quite a huge change but the restructuring looks really clean. |
| ($stdout, $exit) = tee_stdout { | ||
| system("$synccmd") | ||
| }; | ||
| ($exit, $stdout) = syncincremental($sourcehost, $sourcefs, $targethost, $targetfs, $matchingsnap, $newsyncsnap, defined($args{'no-stream'})); |
There was a problem hiding this comment.
here stdout doesn't seems to include the output needed which is later parsed for "contains partially-complete state" and "estination already exists" which likely lead to the test failure.
in the actual replication function capturing stdout or stderr is decided by the pv size which I don't quite get but I didn't looked to close. Can you look into it?
|
@mr-vinn any updates on this? |
|
I guess I'll take over here because the original author is inactive and this is a highly requested feature. |
|
Thanks @phreaker0 ! |
|
I hope it gets some broader testing in the master. I will roll it out over my systems in the next time to make sure there is no breakage. |
At the very least, I end up testing the master branch myself--whenever I deploy a new system, I use master, not the latest release. Mostly for this exact reason. :) |
|
Finally! Thanks for merging this at last. I'm already building+deploying master to some dev systems where they will be heavily tested - a mix of Ubuntu and Arch systems, x86_64 and aarch64. I shall report back. |
|
This is long since closed but nonetheless after two weeks I can report back that testing has gone flawlessly. Thanks again for pushing this over the finish line at last. |
This PR adds new options to syncoid that filter out snapshots from the source and prevent their transfer to the destination. I added an
--exclude-snapsoption and an--include-snapsoption; both accept a regex argument and both can be specified multiple times. If both options are used in the same syncoid call, then snapshots matching both patterns will be excluded.In contrast to PR #632, using
--exclude-snapsor--include-snapsdoes not imply--no-stream. That lets you start with a set of source snapshots like this:And end up with these snaps on the destination by using
--exclude-snaps='hourly':In other words, using one of the new options causes syncoid to simulate a
-Isync by doing a series of-isyncs between intermediate snapshots that are not filtered.A note on the
printstatement refactoring: I wanted to remove the manyif (debug)andif (!quiet)statements, but it does make this diff a lot bigger. Maintainers, please let me know if you'd like me to rebase this without commit 79aaf45.