Skip to content

Conversation

@stepancheg
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2018
@Mark-Simulacrum
Copy link
Member

r? @sfackler or @steveklabnik perhaps

@sfackler
Copy link
Member

SystemTime doesn't expose any concept of years - how is the behavior with respect to leap seconds visible to a user?

@stepancheg
Copy link
Contributor Author

stepancheg commented Aug 22, 2018

@sfackler when there are no leap seconds in SystemTime:

  • user can call SystemTime::now() at midnight on 1st of January 2018 in London, and subtract that value from epoch, and the result will be exactly 1514764800 (I checked there), and not 1514764800 + some number of seconds
  • SystemTime::now() - epoch can be assigned to time_t and used in gmtime on Linux without adjusting for leap seconds (gmtime also doesn't include leap seconds) (edit: gmtime)

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Aug 22, 2018

I think the concept of "leap second" is related rather to the underlying clock from which SystemTime is getting data from. For example during the event of an leap second, if the Windows system clock does not resync to external server, the returned SystemTime will contain the leap second.

@stepancheg
Copy link
Contributor Author

@WiSaGaN I’m not sure what resync you are talking about, but winapi FILETIME ignores leap second adjustments. There’s a good demo of that fact on stackoverflow.
https://stackoverflow.com/a/3603011
And SystemTime on Windows is FILETIME.
I’ve created this PR exactly to avoid these sorts of confusion.

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Aug 22, 2018

@stepancheg Say we had a leap second at the end of 2016-12-31. If we got SystemTime at UTC 23:59:00 on 2016-12-31, and waited 60 seconds plus one leap second, we would be at UTC 00:00:00 on 2017-01-01. If we took the SystemTime then and took the duration between these two, we would have 61 seconds, which was correctly accounting the leap second, instead of 60 seconds if we did not sync the machine time to another UTC source using NTP or other protocol. This was answered in here. So it seems to me the leap second issue is about the clock we are using, and falls under the category that SystemTime is not monotonic, like what's described in the doc below:

Distinct from the Instant type, this time measurement is not monotonic. This means that you can save a file to the file system, then save another file to the file system, and the second file has a SystemTime measurement earlier than the first.

@stepancheg
Copy link
Contributor Author

@WiSaGaN

If we took the SystemTime then and took the duration between these two, we would have 61 seconds, which was correctly accounting the leap second, instead of 60 seconds if

AFAIU after looking at how SystemTime implemented on Windows and reading WinAPI and internets, after we waited for 61 seconds, and computed Duration between SystemTime instances, resulting Duration will be 60 seconds, not 61.

SystemTime is not monotonic

Yes, SystemTime "skips" during a leap second (time is decremented by 1 second). So that even if UTC year is 31622401 seconds, SystemTime year is 31622400 Duration seconds.

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Aug 22, 2018

@stepancheg

AFAIU after looking at how SystemTime implemented on Windows and reading WinAPI and internets, after we waited for 61 seconds, and computed Duration between SystemTime instances, resulting Duration will be 60 seconds, not 61.

Are you sure this can happen even when there is no clock syncing between the host and an external clock source?

@retep998
Copy link
Contributor

retep998 commented Aug 22, 2018

Windows does not know about leap seconds. The clock simply ends up falling out of sync with the actual time until the next time Windows syncs to an external time server at which point the time resyncs jumping backwards in time. So when Windows syncs after a leap second occurs (assuming that the time is perfectly synced otherwise) there will be a minute where monotonic clocks report that 61 seconds have passed while SystemTime will report that only 60 seconds have passed. A day is always reported as the same number of seconds regardless of whether a leap second has occurred.

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2018
@stepancheg
Copy link
Contributor Author

@pietroalbini why it is now "waiting on author"? The last question by @WiSaGaN was answered by @retep998.

@pietroalbini pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2018
stepancheg added a commit to stepancheg/rust that referenced this pull request Sep 5, 2018
Time is formatted as ISO-8601 in UTC timezone.

Nanoseconds are printed by default, unless precision is specified:

```
1966-10-31T14:13:20.123456789Z // by default: {}
1966-10-31T14:13:20Z           // with format {.0}
1966-10-31T14:13:20.1Z         // with format {.1}
```

Implementation is partially copied from `chrono` crate (CC
@quodlibetor).

This is not fully-featured date-time implementation. The motivation
for this commit is to be able to quickly understand what system
time is (e. g. when printed in logs) without introducing external
dependencies.

Format is similar to `java.time.Instant.toString()` output except
that Java truncates trailing zeros by default:
https://repl.it/repls/NauticalSeveralWheel

I think it's better to not truncate zeros by default, because:

* data output (e. g. in logs) look better when field length is the same,
  e. g. with default Java formatter this is how log output may look like:

```
1966-10-31T14:13:20.123456789Z Server started
1966-10-31T14:13:21Z Connection accepted
1966-10-31T14:13:21.567Z Request started
```

* precision information is lost (e. g. when reading string
  1966-10-31T14:13:20Z it's not clear, is it exactly zero millis, or
  timestamp has seconds precision)

This patch depends on leap seconds clarification:
rust-lang#53579
@TimNN
Copy link
Contributor

TimNN commented Sep 11, 2018

Ping from triage @sfackler: I believe this PR requires your review.

@sfackler
Copy link
Member

The sentence being added seems to introduce more questions than it answers for me at least. It should probably include some concrete examples of how and when the leap second treatment is relevant to a user.

@XAMPPRocky XAMPPRocky added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 26, 2018
@XAMPPRocky
Copy link
Contributor

Triage; @stepancheg This PR requires your attention.

@TimNN
Copy link
Contributor

TimNN commented Oct 16, 2018

Ping from triage @stepancheg: I'm closing this due to inactivity per the triage procedure. Thank you for your contribution and please feel free to (re-)open this or another PR in the future.

@TimNN TimNN closed this Oct 16, 2018
@TimNN TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants