oetiker / znapzend

zfs backup with remote capabilities and mbuffer integration.
www.znapzend.org
GNU General Public License v3.0
604 stars 136 forks source link

Seems that for non-enabled mode we only delete snaps from the first child dataset #591

Closed jimklimov closed 1 year ago

jimklimov commented 1 year ago

Support for disabling snapshots of certain children goes as recursive snapshot of the parent and removal of these snaps for certain child datasets. In practice I see however only the first one cleaned up this way (both in oracleMode and without):

[2022-10-21 16:25:42.49053] [4980] [info] creating recursive snapshot on rpool/export
# zfs snapshot -r rpool/export@znapzend-auto-2022-10-21T16:25:42Z
[2022-10-21 16:25:43.05007] [4980] [info] Requesting removal of marked datasets: rpool/export/home/abuild/.ccache@znapzend-auto-2022-10-21T16:25:42Z, rpool/export/home/abuild/jenkins-nut@znapzend-auto-2022-10-21T16:25:42Z, rpool/export/home/abuild/jenkins-nut-doc@znapzend-auto-2022-10-21T16:25:42Z
# zfs destroy -r rpool/export/home/abuild/.ccache@znapzend-auto-2022-10-21T16:25:42Z
[2022-10-21 16:25:43.50717] [2915] [debug] snapshot worker for rpool/export done (4980)
[2022-10-21 16:25:43.51451] [2915] [debug] send/receive worker for rpool/export spawned (5215)
[2022-10-21 16:25:43.51698] [5215] [info] starting work on backupSet rpool/export
# zfs list -H -r -o name -t filesystem,volume rpool/export
...

and that's it - the only destroy in that run's log.

jimklimov commented 1 year ago

So, somehow this bit misfires:

# lib/ZnapZend.pm
createSnapshot = { ...
         $self->zZfs->destroySnapshots(@dataSetsExplicitlyDisabled, 0);

# lib/ZnapZend/ZFS.pm
sub destroySnapshots {
     my @toDestroy = ref($_[0]) eq "ARRAY" ? @{$_[0]} : ($_[0]);
     my @recursive = $_[1] ? ('-r') : ();
...

Actually the $_ contains a long array with each item from original dataSetsExplicitlyDisabled, and then the recursion option as a cherry on top.

I was among last to touch this code in 0bdfece7 actually, but built upon earlier existing spelling with @-varname. Other usages of the method pass a dollar-varname like:

$self->zZfs->destroySnapshots($toDestroy, 1);
jimklimov commented 1 year ago

Yep, that was it - REFERENCE to array \@ did it. Now there's some bug inside the destroy method :)

[2022-10-22 15:56:26.30814] [13253] [info] Requesting removal of marked datasets: rpool/export/home/abuild/.ccache@znapzend-auto-2022-10-22T15:56:25Z, rpool/export/home/abuild/jenkins-nut@znapzend-auto-2022-10-22T15:56:25Z, rpool/export/home/abuild/jenkins-nut-doc@znapzend-auto-2022-10-22T15:56:25Z

# zfs destroy rpool/export/home/abuild/.ccache@znapzend-auto-2022-10-22T15:56:25Z,znapzend-auto-2022-10-22T15:56:25Z,znapzend-auto-2022-10-22T15:56:25Z

Above in non-oracleMode it passes comma-separated list of snapshot names for one dataset, but not different datasets.

jimklimov commented 1 year ago

This: WHY? :)

https://github.com/oetiker/znapzend/blob/233636f1cad59aba92f266ac339cbed5f73e9c2d/lib/ZnapZend/ZFS.pm#L378

jimklimov commented 1 year ago

ok, so gotta map all snaps for each dataset, then glue stuff together