Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update znapzend.default with autoCreation enabled #669

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

moetiker
Copy link
Contributor

@moetiker moetiker commented Jul 9, 2024

I don't see any reason to not have this enabled

I don't see any reason to not have this enabled
@oetiker oetiker merged commit 9e67f39 into oetiker:master Jul 10, 2024
5 checks passed
@jimklimov
Copy link
Contributor

jimklimov commented Dec 5, 2024

@oetiker @moetiker : Cheers, pulling back and forth in true C4 (https://rfc.zeromq.org/spec/42/) spirit? :-D

For clarity and context and cross-linking, as you found in #663, the change of "hard-coded default" (so kudos for NOT swapping it back in perl, but rather changing suggested defaults for init-script etc.) was part of PR #637 that was solving issues raised in #636, namely:

avoid unexpected creation of avoidably large replicas of root datasets where promotable clones of boot environments are used

So one PR (636) introduced the --noautoCreation CLI option to balance the earlier --autoCreation explicitly, among other goals, and another (637) allowed to set the defaults in "dst" configuration properties of a dataset being backed up.

The statement of #663 seems flawed however: autoCreation was never default as far as I can see - e.g. as of v0.21.0 (before changes in my PRs about this), it was 0 for 6-8 years at least:

So the negative option now that it can be explicitly selected is documented as script's own default (so only datasets that exist on destination are updated, unless explicitly requested otherwise):

I don't see any reason to not have this enabled

IMHO this is about balancing a mix of catering to new/casual users (enable, boom all is safely backed up and future datasets are covered too) vs. least-surprise and potentially overflowing the backup media with datasets you did not intend to back up to that location in the first place (maybe they are tracked in a different larger dst though).

I can't really say if any of the choices is generally best as the default (IMHO the predictable one). The change of default for a service/init-script maybe should be announced better though, so people installing a yet another znapzend are not bitten by the change (it is their responsibility to review and adapt configs as they provide /etc/default/znapzend based on that default though, so...)

@moetiker
Copy link
Contributor Author

moetiker commented Dec 6, 2024

Probably there is a misunderstanding about autoCreation it is not the destination zfs it is the thing blow that. So if I backup a home target src: zpool/home dest:host zpool/home it will autocreate all homes blow the home zfs ...
src: zpool/home/user1 dest: host zpool/home/user1 and this is not done without the option autoCreation at the moment.

@jimklimov
Copy link
Contributor

... and this is not done without the option autoCreation at the moment.

I'd wager a bet that it never did. Possibly that it did not even auto-create the replica of the dataset with a "retention schedule" itself (with znapzend configuration in its local properties) unless told to by a first run with explicit autocreation, with regular runs avoiding surprise appearance of new datasets. But would better stage an experiment rather than guesswork, to be sure :)

FWIW another part of the equation here is the recursive=on option in dataset props to walk into such child datasets at all (e.g. home dirs) vs. handling just the dataset with the schedule. Even further, we can toggle enabled=on/off along the way to ignore parts of that tree (e.g. scratch areas like build users' .ccache, of what bites me a lot) and perhaps proceed with further branches (say some config dir/dataset under what we ignored generally).

Finally note that (IIRC) the CLI option is a big lever for the whole script, and the newly added schedule-specific ("dst_N_autocreation") properties allow to tune this for each src/dst combo -- e.g. I don't want surprise full copies of rpool/ROOT/newBE+1 every time as I update the OS and znapzend does not juggle origins/clones yet, per #503, while I do want all newly appearing user homes following a different retention schedule of rpool/export/home to be replicated.

@jimklimov
Copy link
Contributor

TLDR: my point ought to be that IMHO this is a somewhat grey area, with site/admin-specific preferences of what is "right" and what is "least surprise".

If (gotta check) previously indeed there was never autocreation unless asked for, having some pre-packaged setting to have it enabled by default now might be a surprise for people who install znapzend time and again, and do not look much into pre-packaged configs.

Same applies, I suppose, if autocreation did happen by default in fact (whether it was a bug or not, compared to documented intentions) and my changes "broke" that - the least surprise would be to somehow continue doing what always was done, and a default service config is a decent way of overriding the newly better enforced script defaults :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants