-
Notifications
You must be signed in to change notification settings - Fork 305
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
Proposal: Switch date and time handling to use CPAN DateTime modules #409
Comments
https://metacpan.org/pod/DateTime Most importantly, the most common questions: |
@gdevenyi do you intend to port the entire code? The hard part will be detection the DST crossover and handle the duplicate snapshot accordingly. |
@phreaker0 I don't yet use sanoid, only syncoid, so I'm not sure I can effectively work on dev of such a change, however I am interested in contributing that in the future. Regarding the DST crossovers and such, I believe DateTime already intrinsically handles this if you use UTC (default), and use the built-in date/time comparisons available in DateTime (see https://metacpan.org/pod/DateTime#Ambiguous-Local-Times ). This is one of the reasons I'm suggesting going this direction for the code. |
I specially meant the handling of the duplicate hour in the DST crossover case, DateTime handles it but it needs to be detected nevertheless because a duplicate snapshot name is not possible :-) and the later should be suffixed so no hourly snapshot is lost. |
I'm not sure what you mean here, if we use DateTime in UTC there is never a DST crossover because UTC doesn't have DST events. This would simplify checking further. There's still the issue of leap seconds but thats a pretty tiny corner case... |
sanoid currently supports UTC (which is the default for packages and recommend install instruction) and local timezones. In the later case DST crossover is possible and needs to be handled. Local timezones are handy if for example you have an network file server and you are exposing snapshots directly so users can restore older versions themselves. Having the snapshot name/time in the local timezone makes it a lot easier because most non technical users have a hard time with utc :-) |
Ah, okay, makes sense. This is also a pretty simple solution, we need to properly store the daylight savings status when storing localtime. Take, for example, "eastern time" https://en.wikipedia.org/wiki/Eastern_Time_Zone
iso8601 mandiates a storage format for local time:
So, as of right now, where I am, the localtime is: use DateTime;
use DateTime::Format::ISO8601::Format;
my $format = DateTime::Format::ISO8601::Format->new();
my $dt = DateTime->now->set_time_zone( 'local' );
$mydatetime = $format->format_datetime($dt);
print("$mydatetime \n"); > perl localtime.pl
2019-06-18T16:55:15-04:00 Then internally, we do all our math in UTC, after using the DateTime parsers and conversion to UTC. This will handle when daylight savings flips over, and guarantees that we never have identically named snapshots because we embed our timezone info. |
I'll update PR #410 to add a localtime option to demonstrate this further. |
I've updated PR #410 to demonstrate UTC vs localtime generation, here's the difference in how the datestamps are generated: use DateTime;
use DateTime::Format::ISO8601::Format;
my $format = DateTime::Format::ISO8601::Format->new();
my $dt = DateTime->now->set_time_zone( 'local' );
$mydatetime = $format->format_datetime($dt);
$dt->set_time_zone('UTC');
$myutctime = $format->format_datetime($dt);
print("$mydatetime \n");
print("$myutctime \n");
^As we can see here, when my timezone goes from EDT to EDT, the name of the snapshot will go from |
@gdevenyi I didn't think about changing the snapshot name format, this would make it really easy. Let's see if @jimsalterjrs is open for such a change. |
It's that time of the year when I'm reminded about this issue. |
Rather than create a new ticket, I just wanted to point out the datetime strings used in sanoid snaps vs. syncoid ephemeral snaps are oddly very different:
|
It looks like based on #406 and #395 and maybe others that the time/date handling for snapshot naming has become a bit of a problem.
Perl offers a particularly useful module DateTime, for both handling dates and times, generating standard formatting, and doing math with these objects in a sensible and standards-compliant way. It also supports sorting and comparisons between DateTime objects in perl compatible ways for tests.
Short example, get the current time in ISO UTC format as a string, then re-parse that string back into a DateTime object and extract a bunch of sub-date details
What's the exact difference in time between now and ~ a year from now.
The text was updated successfully, but these errors were encountered: