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

Use different scratchspace dir for every session #1083

Closed
wants to merge 1 commit into from

Conversation

lgoettgens
Copy link
Member

This implements my suggestion from #1079 (comment).

The other parts of #1079 (e.g. regarding the sysinfo handling) are not covered here and instead may be split off into their own PR or rebased or...

@lgoettgens lgoettgens requested a review from fingolfin December 12, 2024 13:38
Copy link

codecov bot commented Dec 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.59%. Comparing base (9038df6) to head (6b46191).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1083      +/-   ##
==========================================
+ Coverage   74.47%   74.59%   +0.12%     
==========================================
  Files          55       55              
  Lines        4587     4593       +6     
==========================================
+ Hits         3416     3426      +10     
+ Misses       1171     1167       -4     
Files with missing lines Coverage Δ
src/setup.jl 86.61% <100.00%> (+3.96%) ⬆️

@lgoettgens
Copy link
Member Author

This currently does not create a new scratch folder for every session, but only every time precompilation runs. Do you have an idea on how to fix that?

@fingolfin
Copy link
Member

This is IMHO the wrong approach. First off, scratchspaces live too long. IIRC by default 30 days. I sometimes create dozens of these on a single day. They could and should all be deleted right afterwards, hence mktempdir is better.

And the second issue is precisely the one you run into: the scratch space is meant to be something that is created during precompilation.

Hence my idea to have a single long-lived scratch space which we just use as a kind of temp dir.

@lgoettgens
Copy link
Member Author

I see you point. The only problem with mktempdir is that the cleanup does not work properly if the process does not end gracefully, e.g. due to a segmentation fault or an oom kill

@fingolfin
Copy link
Member

Exactly. That's why I wanted to put the dates in there, so that at least I can do something like ls -d ~/.julia/scratchspaces/*/gap_* to see "old" scratch spaces and clean them manually.

But actually, perhaps we also use a scratch space name that involves the current month, so something like gap-2024-12 -- then that scratch space is only used during that month and cleaned out roughly a month later. At least assuming a recompilation happens. If not it'll stay on this scratch space for a longer time. But that's till OK, it still means that any update to GAP.jl or a dependency is going to cause "old" scratch spaces to be orphaned and eventually recycled.

@lgoettgens
Copy link
Member Author

But actually, perhaps we also use a scratch space name that involves the current month, so something like gap-2024-12 -- then that scratch space is only used during that month and cleaned out roughly a month later. At least assuming a recompilation happens. If not it'll stay on this scratch space for a longer time. But that's till OK, it still means that any update to GAP.jl or a dependency is going to cause "old" scratch spaces to be orphaned and eventually recycled.

sounds good to me

@lgoettgens
Copy link
Member Author

closing in favor of #1079

@lgoettgens lgoettgens closed this Dec 13, 2024
@lgoettgens lgoettgens deleted the lg/scratch-dirs branch December 13, 2024 09:42
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