-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
pragmatically compare timestamps (as suggested by kornelski) #13955
base: master
Are you sure you want to change the base?
Conversation
r? @weihanglo rustbot has assigned @weihanglo. Use |
@rfcbot fcp merge This helps mitigate the needless rebuild when a filesystem supports only low-precision mtime, with a caveat:
I propose to merge without any nightly flag, as the probability is relatively low. |
Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
@rfcbot concern one-in-a-billion-chances-happen-at-scale I'm all for handling timestamp truncation in a reasonable way. But I don't want us making the assumption that one-in-a-billion chances don't happen. If you're running a large fleet of systems doing many many operations per day, one-in-a-billion chances can be a regular occurrence. Can we guard against this to ensure we only hit this case due to timestamp granularity, and not by random chance? |
I don't think 1 billion is an accurate representation of the precision or probability that the sub-second time will be 0. Different systems and filesystems have different precision. For example, NTFS is 100-nanosecond, some virtualization environments or limited hardware might be much less. I'm not sure why the updated patch here only checks for one file. I would assume if one side or the other is stored on a truncated filesystem, then all files would be truncated, and if not some number from 0 to less than all could possibly be 0. |
Doing more experimentation, I've seen drift from 0ns to 14ns on a large codebase. |
☔ The latest upstream changes (presumably #14137) made this pull request unmergeable. Please resolve the merge conflicts. |
I've noticed that the timestamps are wrong only on |
☔ The latest upstream changes (possibly 081d7ba) made this pull request unmergeable. Please resolve the merge conflicts. |
AKA 'make docker great again'.
Fixes #12060 (as coded here: master...kornelski:cargo:nanotime )
Let's cut out needless rebuilds. (Thank you @kornelski for suggesting this)
If we are concerned about backward compatibility, we can put this behind a nightly only flag so this slight behaviour change is not the default.