remotecache: refactor to new cache export interface#6129
Conversation
3b01157 to
e5ab081
Compare
d90a601 to
daa2537
Compare
949b652 to
9d27119
Compare
|
I gave this a test on my setup that regularily runs into the cache issues, but now instead of spinning forever until timeout, those same jobs all fail with: I guess that IS better than spinning forever though. I'll try deleting those questionable :cache tags from the registry next, and see how the builder behaves. |
|
@BtbN Is there a better error in the daemon logs? Looks like potentially a panic. |
|
It's on GitHub Actions, so unfortunately that log is all I got: https://github.com/BtbN/FFmpeg-Builds/actions/runs/17278597172/attempts/2 |
|
@BtbN Can you rerun it with the debug checkbox checked? |
|
It's currently re-running without it. If it still fails, I'll do that. |
|
Unfortunately, me deleting all the affected :cache-tags from the registry made all jobs finish without issue (and also cleanly push new :cache tags). |
|
@tonistiigi here are debug logs from a fork of BtbN's repo. Hope these help! https://github.com/yt-dlp/FFmpeg-Builds/actions/runs/17316380528/job/49168694925 https://github.com/yt-dlp/FFmpeg-Builds/actions/runs/17316380528/job/49168694929 (They're for 2 different builds, I'm not sure why one got so much further than the other) |
0ca3ee8 to
0b01d8b
Compare
0b01d8b to
ad11ea7
Compare
a4a428b to
aed2e4a
Compare
|
I'll make the builds use an image built from this PR. They've been using a slightly older version from a few days ago (after I saw the first "fix panic" commit), and I have not noticed any failures or ultra-long stalls or timeouts since. |
|
This PR appears to fix (and will close once merged) #6008 |
crazy-max
left a comment
There was a problem hiding this comment.
Very nice work @tonistiigi!
The normalization step before was hard to follow. Glad it got drastically reduced.
I also tested this PR on several projects locally and on GHA and good so far. I will make more tests using the new remote cache config debug #6076 (review).
| r.addResult(rr) | ||
| } | ||
|
|
||
| // make sure that none of the deps are childeren of r |
There was a problem hiding this comment.
| // make sure that none of the deps are childeren of r | |
| // make sure that none of the deps are children of r |
| Visit(target any) | ||
| // Vistited returns true if a target has previously been marked as visited. | ||
| Visited(target any) bool | ||
| Add(dgst digest.Digest, deps [][]CacheLink, results []CacheExportResult) (CacheExporterRecord, bool, error) |
There was a problem hiding this comment.
I guess the bool returns true if the CacheLink slice is not complete. Could we have that as comment maybe?
There was a problem hiding this comment.
No, incomplete dependency is an error
buildkit/cache/remotecache/v1/chains.go
Line 74 in aed2e4a
This is a complete refactor of the cache export Go interface. Main aspect is to provide a strictly ordered walk of the cache tree instead of previous one where modification could be added to cache tree at any time by any component. This should address subtle concurrency issues and remove large parts of complicated (and likely buggy) normalization and deduplication steps. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
aed2e4a to
292043f
Compare
closes #5005
This is a complete refactor of the cache export Go interface.
Main aspect is to provide a strictly ordered walk of the
cache tree instead of previous one where modification could be
added to cache tree at any time by any component.
This should address subtle concurrency issues and remove
large parts of complicated (and likely buggy) normalization
and deduplication steps.