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

Tweak docs to update Timestamp definition to current behaviour #134

Merged
merged 3 commits into from
Sep 22, 2024

Conversation

Plecra
Copy link
Contributor

@Plecra Plecra commented Sep 19, 2024

The end goal with this PR is to adjust the documentation to make the semantics of the library as clear as possible.

I've started with just a tiny change to set a foundation, updating the documentation comment to define Timestamp wrt UNIX time.

citations, since my only msgs so far are on discord (I believe Timestamp = Unix is currently intentional, this is just a mistake in docs):

  • now uses the value constructed by duration_since(EPOCH), which is defined as POSIX time

    duration_since(UNIX_EPOCH).unwrap().as_secs() returns the number of non-leap seconds since the start of 1970 UTC. This is a POSIX time_t (as a u64), and is the same time representation as used in many Internet protocols.
    this can also be tested experimentally by printing the Timestamp::now().as_nanos()

  • Display for Timestamp prints the corresponding date to an input unix time.
    this can also be tested by printing format!("{}", Timestamp::new(24 * 60 * 60 * (365 * 30 + (30 / 4)) , 0)) to 2000-01-01T00:00:00Z, which is 22 seconds faster than it should be

@Plecra
Copy link
Contributor Author

Plecra commented Sep 19, 2024

On #7 (comment), I wanna make the most compelling case I can 😁

Which part of this should I elaborate on?

  • "Jiff is a datetime library for Rust that encourages you to jump into the pit of success", so I'm assuming the API should be structured in a way that makes it easy to find the right API when you want to do something
  • It is common for users to write the let start = now(); f(); now() - start pattern
  • The current API presents Timestamp as entirely appropriate (now() and Sub) for the measurement pattern
  • But this implementation is obviously wrong
    • in that it will return a completely incorrect result in normal operating conditions
    • and 1 second is a massive error for plenty of cases of this measurement pattern

Youve said that

I also think that a lot of the trouble and bugs that come from leap seconds aren't necessarily because of datetime libraries not handling them, but rather, because programmers have assumed that their system time is monotonically increasing. Jiff doesn't provide any monotonic guarantees because it's a library for dealing with human time.

so this is entirely fine semantically, but I do not agree that it's a "pit of success", the library immediately lures naive users down the wrong path by presenting them with the classic failure mode. We cant write code that calculates deltas because we can't "assume that our system time is monotonically increasing", but jiff offers that incorrect behaviour via the most accessible API!

src/timestamp.rs Outdated
@@ -23,7 +23,7 @@ use crate::{
/// An instant in time represented as the number of nanoseconds since the Unix
/// epoch.
///
/// A timestamp is always in UTC.
/// A timestamp is always in the UNIX timescale.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this on discord:

well... that's kinda the thing, right. the imprecision of language in this area, and especially with things like UTC, is so widespread that to not to conform to it can wind up being more misleading than not. i mentioned above that UTC is a time scale, and yet, it is seemingly commonly used as if it were a time zone. that is, UTC is used to describe the case where the offset is zero. from RFC 9557:

UTC Offset: Difference between a given local time and UTC, usually
given in negative or positive hours and minutes. For example,
local time in the city of New York, NY, USA in the wintertime in
2023 was 5 hours behind UTC, so its UTC offset was -05:00.

so when you talk about something like 2024-09-19T08:11:00Z, you can say that's a "specific instant in time, expressed in UTC." and in this context, it works correctly, because the representation is theoretically unambiguous. that is, there's no reason why you can't have 2024-06-30T23:59:60Z. but as soon as you parse that into a Unix timestamp, the notion of it being UTC doesn't really apply any more. instead, it's a Unix timestamp. and of course, Unix time forgos leap seconds. nevertheless, the concept of "offset from UTC" still carries over. indeed, this is a thing:

$ ls -l /usr/share/zoneinfo/UTC
-rw-r--r-- 8 root root 114 Sep  6 10:07 /usr/share/zoneinfo/UTC

despite the fact that tzdb uses Unix time just about everywhere (except for /usr/share/zoneinfo/right). because in this context, "a timestamp in UTC" is being used to say something about its offset and not its time scale definition.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe a clearer way of writing this is:

A timestamp is always in the Unix timescale with a UTC offset of zero.

@BurntSushi
Copy link
Owner

so this is entirely fine semantically, but I do not agree that it's a "pit of success"

Please consider that your opinion has been made clear on this matter. You've made this clear on Discord and now in this PR. I've also made my opinion clear that all models are wrong, but some are useful. The notion of models being wrong applies especially so when applied to the human invention of time keeping. Therefore, telling me it's "wrong" or "incorrect" is not something that I find constructive or a productive means towards making things better. Since as I've already noted, it's not that it's wrong that matters, but what the failure modes are and what their impact is.

Your points about how ignoring leap seconds lead to incorrect results are very clearly demonstrated in #7, so I'm in general not sure why you are repeating them here.

Moreover, please consider this from the top of #7:

The most effective thing you can do is leave a comment in this issue with a detailed account of why you want leap second support. Leaving a comment like, "I need to-the-second precision when calculating differences between instants" is not sufficient. You should say why you need that level of precision, and especially, what would happen if you didn't have it. You should also share what your current solution is and any relevant "industry standards" in your domain. (So for example, if your use case is, "I need to do astronomical math," then it would be nice to say why you can't or don't want to use a specialized library for this purpose.) The most important thing you can do is connect specific support of leap seconds to an end user experience. Nebulous "I want it because it's correct" comments are not especially helpful.

This is precisely why I keep trying to frame this discussion around concrete things instead of an abstract notion of correctness. The former is useful. The latter is not. I don't need to be taught that computing deltas can lead to durations that are off by 1 second. This is clearly something I am aware of if you read #7. Instead, I want to see real world problems that have resulted from this incorrectness. I can dream up scenarios just as well as you can. I want real bugs that have actually happened. And I want to understand whether or why using more specialized libraries like hifitime is not appropriate.

@BurntSushi
Copy link
Owner

so this is entirely fine semantically, but I do not agree that it's a "pit of success", the library immediately lures naive users down the wrong path by presenting them with the classic failure mode. We cant write code that calculates deltas because we can't "assume that our system time is monotonically increasing", but jiff offers that incorrect behaviour via the most accessible API!

Pit of success is a pithy goal of the library. It's a design principle. Jiff does not and will not meet that goal in every case, and it will sometimes be in tension with other design goals. You seem to be suggesting that Jiff not offer a way to ask for the current time and provide an easy way to calculate the difference between two times. Like, this seems completely bonkers to me.

@Plecra Plecra changed the title Tweak docs to guide users towards correct API usage Tweak docs to update Timestamp definition to current behaviour Sep 19, 2024
src/timestamp.rs Outdated Show resolved Hide resolved
@BurntSushi BurntSushi merged commit d3e0a16 into BurntSushi:master Sep 22, 2024
17 checks passed
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

Successfully merging this pull request may close these issues.

2 participants