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

CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile #47433

Merged
merged 2 commits into from
Nov 4, 2022
Merged

Conversation

nrontsis
Copy link
Contributor

@nrontsis nrontsis commented Nov 2, 2022

It appears that caching functionalities provided by CircleCi (a leading CI/CD provider) can truncate timestamps to full seconds, resulting in re-compilations as below:

┌ Debug: Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl (mtime 1.667396093e9) has changed
└ @ Base loading.jl:2152

This PR relaxes the is_stale check to be robust against truncating-to-seconds timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

@DilumAluthge
Copy link
Member

😬

@DilumAluthge
Copy link
Member

DilumAluthge commented Nov 3, 2022

Truncating to microseconds was one thing, but truncating to whole seconds seems way too coarse, in my opinion.

@DilumAluthge
Copy link
Member

I think it'd prefer it if we instead implemented #45541

In other words, we would first check mtimes, and if the mtime check failed, instead of immediately considering the cachefile stale, we would check the hashes of the source files.

Copy link
Member

@DilumAluthge DilumAluthge left a comment

Choose a reason for hiding this comment

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

See comments above.

@nrontsis
Copy link
Contributor Author

nrontsis commented Nov 3, 2022

Thanks for the review, I agree that this is hacky.

But I might argue that this is less coarse that existing logic:

  • For the new check to be a false positive, the modification time of the source code file must have occurred exactly at a "full-second timestamp", and is thus of "probability zero" (more accurately zero ->precision_of_mtime_in_seconds).
  • For the new check to report true, we must have ftime <= ftime_req. Thus, by definition, it excludes the common case of cache staling, which is for the source to been modified after it has been compiled. For the purposes of the is_stale check as a whole, the already existing check:
( ftime != floor(ftime_req) ) &&           # Issue #13606, PR #13613: compensate for Docker images rounding mtimes

seems much more prone to produce a false positive.

@nrontsis
Copy link
Contributor Author

nrontsis commented Nov 3, 2022

Having said the above your suggested approach sounds much better.

do you have a rough idea of how much of a slowdown it can incur in practice?

@KristofferC
Copy link
Member

I think this is fine since it is an exact check and not a range check like the microsecond check that was mentioned.

@DilumAluthge DilumAluthge dismissed their stale review November 3, 2022 12:55

Dismissing as stale per the conversation above

…hefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl 
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.
@KristofferC KristofferC added backport 1.6 Change should be backported to release-1.6 backport 1.8 Change should be backported to release-1.8 labels Nov 4, 2022
@KristofferC KristofferC merged commit bf92e83 into JuliaLang:master Nov 4, 2022
KristofferC pushed a commit that referenced this pull request Nov 8, 2022
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
@nrontsis nrontsis deleted the patch-1 branch November 8, 2022 22:36
DilumAluthge pushed a commit that referenced this pull request Nov 15, 2022
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
@KristofferC KristofferC mentioned this pull request Dec 14, 2022
26 tasks
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Dec 16, 2022
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
KristofferC pushed a commit that referenced this pull request Oct 10, 2023
…hefile (#47433)

* CircleCI caching: allow ftime to be ceil(ftime_req) in Base.stale_cachefile

It appears that [caching functionalities](https://circleci.com/docs/caching/) provided by CircleCi, a leading CI/CD provider, can truncate timestamps to full seconds, resulting in re-compilations as below:
```
Rejecting stale cache file /root/.julia/compiled/v1.8/ComponentArrays/cYHSD_3rQji.ji (mtime 1.6673960929277816e9) because file /root/.julia/packages/ComponentArrays/YyD7i/src/ComponentArrays.jl
```

This PR relaxes the `is_stale` check to be robust against rounding-to-second timestamp mutations.

I can provide a minimal CircleCI configuration file to reproduce if this is helpful.

(cherry picked from commit bf92e83)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.6 Change should be backported to release-1.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants