vfs: cleanup on max_open_files threshold #533
Conversation
barraguda
requested review from
dr-frmr and
nick1udwig
and removed request for
dr-frmr
kinode/src/vfs.rs
Outdated
| let current_count = open_files.len(); | ||
| let target_count = max(current_count / 2, MAX_OPEN_FILES / 2); | ||
|
|
||
| // collect all entries, sort by last access time | ||
| let mut entries: Vec<_> = open_files | ||
| .iter() | ||
| .map(|entry| (entry.key().clone(), entry.value().1)) | ||
| .collect(); | ||
| entries.sort_by(|a, b| a.1.cmp(&b.1)); | ||
|
|
||
| // remove oldest entries until we reach the target count | ||
| for (path, _) in entries.into_iter().take(current_count - target_count) { | ||
| // use remove_if_present to avoid potential race conditions | ||
| open_files.remove_if(&path, |_, _| true); | ||
| } |
There was a problem hiding this comment.
is there a way to make this more efficient using retain on the dashmap with a fixed duration calculated by the average of the map? could be better than cloning every path. something like
- calculate average last access time by traversing map once
- retain entries only with a below average last access time
There was a problem hiding this comment.
this is a good point, I'll try something out!
|
some notes for the other suggestions during discussions:
|
|
|
Possible/reasonable to match on any error coming out of open_file()? |
I like this idea, but noting that keeping the open_files very close to the limit and relying on specific errors like this might be brittle. Another big note is, we need to save cursor positions of files we close, so that processes that expect them to be at a certain location won't notice a difference even if the vfs closed and reopened them. This is ok but leads to us not being able to use synchronous functions like dashmap.retain() and makes closing a lot of files considerable slower.. |
3 participants
Problem
#525
Solution
If open_files.len() is above threshold (180 right now based on my mac testing), run a cleanup that prunes 50% of the open files.
Interval cleanup of idle files still happens, but now in a tokio::select loop instead.
Notes
Linux seems to have a larger supported open filehandles amount per OS process (~1000 as opposed to mac's 256)