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

Incompatibility with timezone-data 2020b? #120

Closed
Keruspe opened this issue Oct 13, 2020 · 11 comments
Closed

Incompatibility with timezone-data 2020b? #120

Keruspe opened this issue Oct 13, 2020 · 11 comments

Comments

@Keruspe
Copy link

Keruspe commented Oct 13, 2020

In a rails console with timezone-data 2020a:

irb(main):001:0>  Time.find_zone!("Europe/Paris").parse("2020-12-02")    
=> Wed, 02 Dec 2020 00:00:00 CET +01:00                                  

In a rails console with timezone-data 2020b

irb(main):001:0>  Time.find_zone!("Europe/Paris").parse("2020-12-02")    
=> Wed, 02 Dec 2020 00:00:00 CEST +02:00

Notice the CEST for a winter date

@philr
Copy link
Member

philr commented Oct 13, 2020

The 2020b version of the 'zic' time zone compiler has changed to output zoneinfo files using a new 'slim' format by default. The slim format requires users of the zoneinfo files to interpret rules instead of just reading a list of transitions.

TZInfo currently only supports reading the transitions and will require modification to use the rules. Consequently, it'll produce the wrong offset at some point after the final transition in the file.

In the mean time you can rebuild your time zone data package or zoneinfo files using the 'fat' format:

make ZFLAGS='-b fat' install

or:

zic -b fat /usr/share/zoneinfo/tzdata.zi

Alternatively, you can use the tzinfo-data gem as a data source instead (just add it to your Gemfile and it'll be used by default).

@Keruspe
Copy link
Author

Keruspe commented Oct 13, 2020

Thanks for the pointers, I indeed ended up fixing the package to pass ZFLAGS="-b fat"

@starsirius
Copy link

starsirius commented Oct 21, 2020

Thank you for the pointers! We started to notice similar time zone issues on our apps a few days ago (they were using Alpine and tzdata package got updated to 2020c). We can confirm the issue goes away after migrating to the tzinfo-data gem as the data source.

Looking forward to the TZInfo update!

# with tzdata 2020c-r0
irb(main):001:0> Time.parse('2020-12-01T00:00:00+00:00').in_time_zone('America/New_York')
=> Mon, 30 Nov 2020 20:00:00 EDT -04:00

# with tzdata 2020a-r0
irb(main):001:0> Time.parse('2020-12-01T00:00:00+00:00').in_time_zone('America/New_York')
=> Mon, 30 Nov 2020 19:00:00 EST -05:00

# with tzinfo-data gem
irb(main):001:0> Time.parse('2020-12-01T00:00:00+00:00').in_time_zone('America/New_York')
=> Mon, 30 Nov 2020 19:00:00 EST -05:00

@aardvarkk
Copy link

aardvarkk commented Oct 21, 2020

This bug corrupted tons of our production data. The lack of a proper DST conversion when used in combination with newer tzdata packages meant our customers created a bunch of data that's off by an hour. Finding and correcting it will be very challenging. I would strongly recommend tzinfo change its behaviour to throw some kind of error/warning if it knows it won't be able to properly parse the tzdata output. We are now changing our build to install the tzinfo-data gem, but any existing DB data remains corrupted unfortunately.

Like @starsirius, we were also using an Alpine image that was at tzdata version 2020c-r0.

@philr
Copy link
Member

philr commented Oct 22, 2020

@aardvarkk @starsirius I'm sorry that you've encountered problems due to this change in the default behaviour of the Time Zone Database zic command.

Prior to version 2020b, zic by default output zoneinfo files containing information about transitions between standard and daylight savings time that reached far into the future as well as a POSIX-style TZ string giving the rules from which further transitions can be derived. From version 2020b onwards, the default was changed to output the minimum number of transitions and rely on clients interpreting the rules. The old behaviour can still be selected with runtime and compile time options.

TZInfo only reads the transitions. It doesn't interpret the POSIX-style TZ string.

Had I known this change was going to be made in 2020b, I'd have planned to add support for the POSIX-style TZ string beforehand. I'm working on a change to add this support now for both v2.x and v1.x.

Please note that TZInfo is and always has been an unpaid side project for me. I am only able to work on it in my spare time and my spare time is finite.

Probably not helpful right now, but my preference for storing local times provided by users is to store the provided local time and the time zone identifier. The corresponding UTC time can be derived when needed or, if required for indexing purposes, stored and updated when time zone definition changes. Here's a relevant blog post by Jon Skeet: https://codeblog.jonskeet.uk/2019/03/27/storing-utc-is-not-a-silver-bullet/. What I've outlined is his Option 3.

Finally, I find it's always a good idea to test changes (including time zone data) prior to deploying to production! 😉 There have been other changes that have caught people out before. For example, the definition for Europe/Dublin changing in 2018f to correctly indicate that the Irish observe negative daylight savings time in the winter.

@aardvarkk
Copy link

Thanks for the response @philr . My intent is not to blame you for this as it seems like it came as a surprise to you too. It's just a nasty combination of events. I realize the changes that are the root cause of this are not under your control, it's just frustrating that this behaviour started happening completely silently.

The option to change the way we store times isn't really something we're looking at doing right now, and we're using Rails where I believe the "default" approach is to just store UTC times and then convert as necessary. Any option you choose has its pros and cons.

In terms of testing, we do have a very strong test suite. We test on CircleCI, but we use their provided images for the machine on which our test suite runs. That did not align with the Alpine image we used for deploying our code in production, so we had a mismatch there. We hadn't flagged that as an issue previously, but that's something we're going to have to look at as it prevented our test suite from catching this regression.

I'm honestly surprised more people aren't running into this. Maybe they'll start having problems only after the DST switchover happens, but I believe this would affect a default Rails install and give just plain "wrong" answers when converting to local times.

We have started pushing out code to add the tzinfo-data gem, and we're slowly working through the process of contacting clients and trying to help them correct their data.

Thanks for your work on TZInfo -- it's appreciated.

@toxin20
Copy link

toxin20 commented Oct 26, 2020

I'm honestly surprised more people aren't running into this. Maybe they'll start having problems only after the DST switchover happens, but I believe this would affect a default Rails install and give just plain "wrong" answers when converting to local times.

I ran into this problem after the DST time change yesterday. Suddenly the groupdate gem gave up and threw an error "Database and Ruby have inconsistent time zone info. Database returned 2020-10-26 01:00:00 +0200". (groupdate/issues/222) Also entries were missing that should have been published ("Post.where('published_at <= ?', Time.current.beginning_of_day").

Because of the groupdate error I looked closer into a database or system level error and read all the "timezones in rails" blog posts I could find, until I fortunately found this issue here.

@quorak
Copy link

quorak commented Oct 27, 2020

Gladly we discovered this before deployment.
This should definitely put onto the rails mailing group before more run into this issue in production.

@philr
Copy link
Member

philr commented Oct 27, 2020

I'm making progress on a change to support the "slim" format zoneinfo files. If my available time goes to plan I'll probably be able to release new versions at the weekend or early next week.

Not all distributions will be affected by the change.

Debian, Ubuntu, RHEL and Fedora's tzdata packages will be unaffected. They are all built using the system zic binary. That binary is provided by glibc and currently only supports producing "fat" format files.

Alpine's tzdata package and Gentoo's timezone-data package are affected because they are built using the latest zic binary and the default options.

@odlp
Copy link

odlp commented Oct 29, 2020

Thanks for this bug report. For any Alpine users out there, here's an example of recompiling tzdata based on @philr's comment:

FROM ruby:2.7.2-alpine

# Install tzdata because we need the zic binary
RUN apk add --no-cache tzdata

# Fix incompatibility with slim tzdata from 2020b onwards
RUN wget https://data.iana.org/time-zones/tzdb/tzdata.zi -O /usr/share/zoneinfo/tzdata.zi && \
    /usr/sbin/zic -b fat /usr/share/zoneinfo/tzdata.zi

philr added a commit that referenced this issue Nov 5, 2020
Use the POSIX-style TZ string to calculate transitions after the last
defined transition contained within the file. At the moment these
transitions are generated when the file is loaded. In a later release
this will likely be changed to calculate transitions on demand.

Resolves #120.
philr added a commit that referenced this issue Nov 5, 2020
Use the POSIX-style TZ string to calculate transitions after the last
defined transition contained within the file. At the moment these
transitions are generated when the file is loaded. In a later release
this will likely be changed to calculate transitions on demand.

Use the 64-bit section of zoneinfo files regardless of whether the
runtime supports 64-bit Times. The 32-bit section is empty in "slim"
zoneinfo files.

Resolves #120.
philr added a commit that referenced this issue Nov 8, 2020
Use the POSIX-style TZ string to calculate transitions after the last
defined transition contained within the file. At the moment these
transitions are generated when the file is loaded. In a later release
this will likely be changed to calculate transitions on demand.

Use the 64-bit section of zoneinfo files regardless of whether the
runtime supports 64-bit Times. The 32-bit section is empty in "slim"
zoneinfo files.

Resolves #120.
@philr philr closed this as completed in 1efcf6d Nov 8, 2020
@philr
Copy link
Member

philr commented Nov 8, 2020

Support for "slim" format zoneinfo files has now been added in releases v1.2.8 and v2.0.3.

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

No branches or pull requests

7 participants