- Notifications
You must be signed in to change notification settings - Fork925
chore: increase fileCache hit rate in autobuilds lifecycle#18507
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Emyrk commentedJun 23, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
coderd/files/cache.go Outdated
Help:"The total number of file references ever opened in the file cache. The label 'hit' indicates whether the file was already in the cache ('true') or not ('false').", | ||
}, []string{"hit"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Added a label for detecting our cache hit rate
d73e843
toae40278
Compareae40278
toa4c0df4
Comparec1d05a0
to129235b
Comparea4c0df4
toa97fd53
Comparea97fd53
to9945049
Compare9945049
toe5a33c1
Comparee5a33c1
to4f6244f
CompareUh oh!
There was an error while loading.Please reload this page.
// `wsbuilder` needs to load the terraform files for a given template version | ||
// into memory. If 2 workspaces are using the same template version, they will | ||
// share the same files in the FileCache. This only happens if the builds happen | ||
// in parallel. | ||
// TODO: Actually make sure the cache has the files in the cache for the full | ||
// set of identical template versions. Then unload the files when the builds | ||
// are done. Right now, this relies on luck for the 10 goroutine workers to | ||
// overlap and keep the file reference in the cache alive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yeah, it could be nice if we did more of a "mark and sweep" style release strategy. or maybe generational...
oh no, am I about to build a general purpose garbage collector for this? Oh No
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yea, I had a similar set of thoughts and decided this was a good enough idea for this release.
There is a better way to do this for sure, and I wonder where else in the codebase we will have similar things going on (prebuilds?). That being a large set of workspaces that have some work to be done in bulk.
4f6244f
tobc38b64
Compare7b152cd
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
wsbuilder
hits the file cache when running validation. This solution is imperfect, but by first sorting workspaces by their template version id, the cache hit rate should improve.If there is a large number of workspaces, this could save some memory impact of dynamic parameters.
Future Work
A lot of builds have overlapping queries to fetch template information:
tx.GetTemplateByID
GetTemplateVersionByID
We could also cache these between the go routines running the builds to reduce some db load.