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

Allow configuring the date and time formatting used in snapshot names #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chazapis
Copy link

@chazapis chazapis commented Jul 16, 2019

Two new, optional parameters in the configuration file date_format and time_format allow you to override the default strftime() values of %Y-%m-%d and %H:%M:%S respectively. This change may be really helpful if you want to expose snapshots over the network. Samba will "mangle" the names of files or directories containing the ":" character, as many operating systems do not allow it (in fact, Mac OS won't even show a directory containing a colon character).

While at it, the prefix used for snapshots (currently "pyznap"), has also been made configurable.

Note that the changes are not "backwards compatible". The snapshot prefix, as well as date and time formatting are part of pyznap's snapshot management code, so it will ignore snapshots taken with different values of the variables.

@yboetz
Copy link
Owner

yboetz commented Jul 16, 2019

I'll have to look into this a bit deeper on the weekend, to make sure it doesn't break anything. I'm a bit worried about backwards compatibility as you said, e.g. if at some point you change the format (but not the prefix), then pyznap will fail as it cannot match the date/time of old snaps anymore.

@chazapis
Copy link
Author

Yes, I agree that the backwards compatibility is concerning, but this is really useful for new deployments. For that reason, I have not changed the sample configuration file. As it is now, the new options are "undocumented features".

@yboetz
Copy link
Owner

yboetz commented Aug 22, 2019

Sorry for not getting back to you earlier, I only found time to work on pyznap again this week. I did a few tests with different prefixes and date/time formats, but I'm not convinced yet. Something can go wrong quickly if you choose other date/time formats, e.g. there cannot be any '_' in the formats. And you also always want hours, minutes and seconds in the time format, to accurately take snapshots. I need to think about how to make this a bit more robust.

@DeViLRuNNeR-dev
Copy link

I have done some testing myself and it turns out we can't use Windows Shadow Copies on Samba due to the naming of these snapshots. There is not option in SAMBA to catch the snapshottype (hourly, frequent) on the end of the snapshot name.

Does anybody know how we can change the code to put the snapshottype in front of the data/time?

@ShaRose
Copy link

ShaRose commented Nov 30, 2019

Hilariously late, but at least for verifying if a format would work you can simply loop and check if the needed parts are in the format string passed. Ditto for sanity checking invalid characters.

As a far more intrusive option, you could simply not include what kind of snapshot it is in the snapshot, only take one for all schedules, and include what schedules a snapshot counts for in a user property on the snapshot when taking it, and then use zfs set to expire options as needed.

For example,

zfs snap -o com.github.yboetz.pyznap:snapshotplans=frequent,hourly,daily,weekly themis-pool@pyznap_2019-11-29_22:00:00

and then as there are enough frequents, but it still counts as a daily / hourly / etc snapshot,

zfs set com.github.yboetz.pyznap:snapshotplans=hourly,daily,weekly themis-pool@pyznap_2019-11-29_22:00:00

You'd be able to see all snapshots in samba, and you'd be able to lower the number of total snapshots on the system considerably which would probably speed up almost everything fairly considerably (Fewer snapshots, so less time listing and sending: less made, so less time on actually taking them: expiring frequents now simply modifies metadata, but not the snapshot itself or the guid).

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.

4 participants