Repository navigation
Make eviction strategy in dir cache link-aware - #3641
Conversation
| log.Warning("Failed to store cache file %s: %s", cachedFile, err) | ||
| } | ||
| // TODO(peterebden): This is a little inefficient, it would be better to track the size in | ||
| // RecursiveCopy rather than walking again. |
There was a problem hiding this comment.
I assume we are still keeping this comment because unsharedSize is still inefficient?
There was a problem hiding this comment.
It's a little out of date (since we do RecursiveLink rather than RecursiveCopy for some time now) but not fully inaccurate.
I'm just going to remove it though, I don't think anything is going to change here.
| } | ||
| id, links := fileInfo(info) | ||
| if _, present := files[id]; !present { | ||
| files[id] = &cacheFile{Size: uint64(info.Size()), Links: links} |
There was a problem hiding this comment.
I feel a slight unease with a function called readEntry writing to one of its input, but maybe this is just a functional programming pet peeves of mine.
There was a problem hiding this comment.
Yeah it's just not very easy to figure out a better way of handling it. Because things can error partway an iterator doesn't work super well either.
| continue | ||
| } | ||
| totalSize -= entry.Size | ||
| // A file is only freed once the last entry referring to it is removed. |
There was a problem hiding this comment.
Reference counting requires a global lock. Does this mean we can't merge this until #3644 is merged?
There was a problem hiding this comment.
Nope that's about concurrency across processes. The reference counts here are all in memory, we've collected them all into this map.
3dcae26 to
a0c07da
Compare
a0c07da to
b316ddd
Compare
First part of #3629
Accounts for links in cache files so when cleaning we don't free things from the cache that are actively used elsewhere. We bump mtime on retrieve and use that to evict instead of atime (which is not usually updated frequently, most things mount with relatime).