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

Changed (and simplified) DST handling #362

Merged
merged 2 commits into from May 22, 2019

Conversation

Projects
None yet
3 participants
@shodanshok
Copy link
Contributor

commented Mar 26, 2019

See #155 for background information.

This patch streamlines sanoid DST handling, reverting the "snapshot suffix" behavior introduced with #175.

The only downside of this approach is the missing of a single hourly snapshot (with duplicate 02:00:00 time). To avoid that, one should use UTC time (as clearly documented).

@jimsalterjrs

This comment has been minimized.

Copy link
Owner

commented Mar 29, 2019

@shodanshok rebase needed please, sorry!

@shodanshok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

@jimsalterjrs @phreaker0 I rebased my path, please give it a look.

@phreaker0
Copy link
Collaborator

left a comment

I tested this in a FreeBSD 11.2 VM with a sanoid invocation interval of one minute and I get two daily snapshots on the same day on the dst boundary:

sanoid-test-2@autosnap_2017-10-28_00:00:00_daily
sanoid-test-2@autosnap_2017-10-28_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_00:00:00_monthly
sanoid-test-2@autosnap_2017-10-28_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_23:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_23:59:00_daily
sanoid-test-2@autosnap_2017-10-29_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_23:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_23:59:00_daily
sanoid-test-2@autosnap_2017-10-30_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_23:00:00_hourly

see sanoid-test-2@autosnap_2017-10-28_00:00:00_daily and sanoid-test-2@autosnap_2017-10-28_23:59:00_daily

@shodanshok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

@phreaker0 Interesting, on a CentOS 7 VM I have no duplicate daily snapshot:

sanoid-test-2@autosnap_2017-10-28_00:00:00_daily
sanoid-test-2@autosnap_2017-10-28_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_00:00:00_monthly
sanoid-test-2@autosnap_2017-10-28_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_23:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_00:00:00_daily
sanoid-test-2@autosnap_2017-10-29_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_23:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_00:00:00_daily
sanoid-test-2@autosnap_2017-10-30_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_23:00:00_hourly

I had to spin up a FreeBSD VM to check. Do you have any clue?

EDIT: anyway, it does not seems to be a duplicate at all. Your output correctly has 3 daily snapshots, but the one called sanoid-test-2@autosnap_2017-10-28_23:59:00_daily should really be sanoid-test-2@autosnap_2017-10-29_00:00:00_daily

@shodanshok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

@phreaker0 I just installed a FreeBSD 11.2 VM (with perl 5.28) and I have no strange daily snapshots:

root@freebsd:~/sanoid/tests/2_dst_handling # uname -a
FreeBSD freebsd 11.2-RELEASE FreeBSD 11.2-RELEASE #0 r335510: Fri Jun 22 04:32:14 UTC 2018     root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC  amd64

root@freebsd:~/sanoid/tests/2_dst_handling # perl -v

This is perl 5, version 28, subversion 1 (v5.28.1) built for amd64-freebsd-thread-multi
sanoid-test-2@autosnap_2017-10-28_00:00:00_daily
sanoid-test-2@autosnap_2017-10-28_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_00:00:00_monthly
sanoid-test-2@autosnap_2017-10-28_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-28_23:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_00:00:00_daily
sanoid-test-2@autosnap_2017-10-29_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-29_23:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_00:00:00_daily
sanoid-test-2@autosnap_2017-10-30_00:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_01:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_02:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_03:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_04:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_05:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_06:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_07:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_08:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_09:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_10:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_11:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_12:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_13:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_14:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_15:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_16:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_17:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_18:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_19:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_20:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_21:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_22:00:00_hourly
sanoid-test-2@autosnap_2017-10-30_23:00:00_hourly
@phreaker0

This comment has been minimized.

Copy link
Collaborator

commented Apr 1, 2019

@shodanshok did you use a one minute interval? By default the test uses 900 seconds / 15 minute interval.

@shodanshok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

Ah ok, with a 60s interval I confirm to see the "wrong" snapshot on both Linux and FreeBDS. But I ask: is it really a problem? No snapshots are duplicate, rather a single one has 1 minute earlier timestamp. We don't lose anything, as a sanoid-test-2@autosnap_2017-10-30_00:00:00_hourly exists.

It seems a reasonable tradeoff to me, with a much simpler approach to DST. Am I missing something?

@shodanshok

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

@phreaker0 @jimsalterjrs Hi, did you have a chance to check my last update on the matter? Any thoughts?

@phreaker0

This comment has been minimized.

Copy link
Collaborator

commented Apr 8, 2019

@shodanshok didn't had time too look into the issue yet.

@phreaker0

This comment has been minimized.

Copy link
Collaborator

commented Apr 8, 2019

Sorry, I looked into it and It's not an error but expected behavior, because the tests starts at the start of the day. Daily snapshot get's created because there is no other yet. I will adapt the test case after this is merged.

@jimsalterjrs jimsalterjrs merged commit 0168ee3 into jimsalterjrs:master May 22, 2019

@jimsalterjrs

This comment has been minimized.

Copy link
Owner

commented May 22, 2019

Thank you @shodanshok :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.