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

time: document that package does not support leap seconds #15247

Closed
phayes opened this issue Apr 12, 2016 · 7 comments
Closed

time: document that package does not support leap seconds #15247

phayes opened this issue Apr 12, 2016 · 7 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@phayes
Copy link

phayes commented Apr 12, 2016

The time package does not support leap seconds. This is further explored in these two issues:
#15190
#8728

While it looks like this won't be fixed until Go 2.0, we can at least document the current behavior. Specifically the following things should be documented:

  1. The definition of a "second". Specifically, it should be clarified that go uses the "POSIX-second". A POSIX-second is equal to one standard second in all cases except when a POSIX-second is followed by a leap-second, in which case the POSIX-second is equal to two standard seconds, and the leap-second is skipped. This means that when calculating the Duration between two dates as a number of seconds, the unit being calculated is POSIX-seconds. To convert to standard seconds, add the number of leap seconds that occurred between the two dates in question.
  2. The time package does not support parsing datetimes that contain a leapsecond. For example, the package will not parse 2005-12-31T23:59:60Z.
  3. The behavior of time.Sleep is unaffected and uses standard seconds in all cases, since it uses a monotonic clock provided by the runtime package. (As far as I can figure it, this needs to be confirmed)
  4. The behavior of the above items on non-POSIX systems (like windows)
@bradfitz bradfitz added this to the Go1.7 milestone Apr 12, 2016
@bradfitz
Copy link
Contributor

/cc @ianlancetaylor @rsc @robpike

@ianlancetaylor
Copy link
Contributor

There are no good solutions here but I don't really agree with your proposed docs. As I see it, the time.Time value is not affected by leap seconds at all. The question is how we handle methods like Day and Second and Format, and functions like Parse.

We actually do have leap second information in the zoneinfo files. We could even use it, just as we use the zone offset. We would have to subtract the current leap second offset to gettimeofday when calling time.now. We would change time.abs to consider leap seconds. We would change Format and Parse accordingly. I don't know what the consequences of doing this would be.

Barring that, it seems to me that we should document that when we say UTC we actually mean TAI, and document that time.Now automatically adds the appropriate number of leap seconds.

(To be clear, in issue #15190 you say that there is a problem with time.Sub, but I think the problem is actually with time.Parse.)

@phayes
Copy link
Author

phayes commented Apr 12, 2016

Thanks for your comments @ianlancetaylor .

I completely agree with your assessment of where the problem actually lies and how best to fix it. Now() would also have to be updated (and unixToInternal would need to be a function, not a constant etc.)

The tricky thing is how to document current behavior in a way that makes sense. This could be best summarized as "go does it like linux". I've tried my best to do a compact and technically correct summary of this above, but maybe the best thing would be to say "go does it like linux" and link to an off-site URL with the gory details.

I think invoking TAI would actually make it more confusing. TAI is currently 36 seconds offset from UTC. 26 due to leap-seconds and an additional 10 seconds due to epoch differences. It's also an incorrect summary of current behavior, since TAI is a monotonic counting of seconds, and the current behavior uses the POSIX system, which is not strictly monotonic when crossing a leapsecond. However, having said that, using TAI as an internal representation as part of a larger fix seems like a good idea.

@rsc
Copy link
Contributor

rsc commented Apr 12, 2016

The calendrical calculations always assume a Gregorian calendar.

can be changed to say

The calendrical calculations always assume a Gregorian calendar, with no leap seconds.

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 5, 2016
@rsc
Copy link
Contributor

rsc commented Oct 6, 2016

Decision is what I commented on Apr 12. That's the CL anyone can send.

@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 6, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Oct 6, 2016

Done.

@gopherbot
Copy link

CL https://golang.org/cl/30595 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants