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.

Sign up for GitHub

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

cache: Fix walkBlob to call callback only for unlazied contents #5595

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

cache: Fix walkBlob to call callback only for unlazied contents #5595

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7b2d353
solver: return errors hit during cache export
sipsma Nov 29, 2024
b375195
solver: fix handling of lazy blobs in cache export
sipsma Nov 29, 2024
30e1743
mountmanager: handle lazy blob ref loading in cache mounts
sipsma Nov 30, 2024
8f16a38
solver: allow LoadCache to set CacheOpts
sipsma Dec 5, 2024
fc4ecad
remove redundant activeOp interface
sipsma Dec 6, 2024
867e8fd
provide DescHandlers to GetMutable always
sipsma Dec 6, 2024
c00304f
Call callback only for unlazied contents
ktock Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cache/refs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,11 @@ func getBlobWithCompression(ctx context.Context, cs content.Store, desc ocispecs
}

func walkBlob(ctx context.Context, cs content.Store, desc ocispecs.Descriptor, f func(ocispecs.Descriptor) bool) error {
if _, err := cs.Info(ctx, desc.Digest); errors.Is(err, cerrdefs.ErrNotFound) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know a more defined call path for this case? I assume the blob is missing because it was not pulled and is missing the descHandlers. Is this expected or did we forget to pass the descHandlers? Or is this a callpath where we would want to skip a blob if we only have access to it remotely and not locally?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is called from getAvailableBlobs and in this context, this function doesn't have access to refs(and descHandlers) to compression variants thus GetRemtoes currently treats only local blobs as available compression variants. To change GetRemote to return all remotely accessible variants, we need to change getAvailableBlobs somehow accessible to all compression variant's refs and descHandlers to trigger unlazying of them.

return nil // this blob doesn't exist in the content store. Don't call the callback.
} else if err != nil {
return err
}
if !f(desc) {
return nil
}
Expand Down
Loading