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

Test cache trimming priority in the [copy] mode #4497

Merged

Conversation

snowleopard
Copy link
Collaborator

I wanted to fix trimming in the copy mode because I thought it was broken, but when adding a test I realised that it actually works fine! I added a comment to explain how prioritising trimming by ctime happens to work both in hardlink and copy modes.

@snowleopard snowleopard requested review from rgrinberg, a user and aalekseyev April 17, 2021 21:58
@snowleopard snowleopard force-pushed the improve-cache-trimming-in-copy-mode branch from 01fcd46 to 12ccc78 Compare April 17, 2021 22:00
@snowleopard
Copy link
Collaborator Author

Strangely, the CI fails on Mac OS: for some reason, there our trimming heuristic works in the opposite order. If anyone has any ideas why this is happening, please let me know.

@ghost
Copy link

ghost commented Apr 19, 2021

Could it be related to the FS times granularity on OSX? It's much more coarse than on Linux. We've had issues with this in the past and have special code to deal with this in cached_digest.ml.

@aalekseyev
Copy link
Collaborator

aalekseyev commented Apr 19, 2021

I don't know, but a wild guess is that perhaps ctime is not bumped on mac when you add or remove a hardlink.
(I don't really understand the test suite, just guessing from a blackbox perspective).

@jeremiedimino, I though that too, but I can see dune_cmd wait-for-fs-clock-to-advance pepperred around the test so I thought that idea was already explored.

@snowleopard
Copy link
Collaborator Author

Could it be related to the FS times granularity on OSX? It's much more coarse than on Linux.

@jeremiedimino Yes, that sounds plausible, which is why I used dune_cmd wait-for-fs-clock-to-advance after each ctime update. Perhaps, it's not enough.

I don't know, but a wild guess is that perhaps ctime is not bumped on mac when you add or remove a hardlink.

@aalekseyev The hardlink mode tests are fine. The problem happens only when testing the copy mode. So, perhaps, copying files on Mac OSX doesn't update the ctime as we expect.

@snowleopard snowleopard force-pushed the improve-cache-trimming-in-copy-mode branch 2 times, most recently from 3417d3b to bf20349 Compare April 21, 2021 10:48
@snowleopard snowleopard force-pushed the improve-cache-trimming-in-copy-mode branch from bf20349 to 6d4622b Compare April 21, 2021 12:08
@snowleopard
Copy link
Collaborator Author

@jeremiedimino I added an override for Mac OSX for problematic tests, as we discussed, so I think this is ready to be merged.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

@snowleopard snowleopard merged commit 5584146 into ocaml:main Apr 21, 2021
@snowleopard snowleopard deleted the improve-cache-trimming-in-copy-mode branch April 21, 2021 14:15
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