Skip to content

Commit

Permalink
Refactor cache metadata interface.
Browse files Browse the repository at this point in the history 
There are a few goals with this refactor:
1. Remove external access to fields that no longer make sense and/or
   won't make sense soon due to other potential changes. For example,
   there can now be multiple blobs associated with a ref (for different
   compression types), so the fact that you could access the "Blob"
   field from the Info method on Ref incorrectly implied there was just
   a single blob for the ref. This is on top of the fact that there is
   no need for external access to blob digests.
2. Centralize use of cache metadata inside the cache package.
   Previously, many parts of the code outside the cache package could
   obtain the bolt storage item for any ref and read/write it directly.
   This made it hard to understand what fields are used and when. Now,
   the Metadata method has been removed from the Ref interface and
   replaced with getters+setters for metadata fields we want to expose
   outside the package, which makes it much easier to track and
   understand. Similar changes have been made to the metadata search
   interface.
3. Use a consistent getter+setter interface for metadata, replacing
   the mix of interfaces like Metadata(), Size(), Info() and other
   inconsistencies.

Signed-off-by: Erik Sipsma <[email protected]>
  • Loading branch information
@sipsma
sipsma committed Aug 25, 2021
1 parent 94e9d15 commit a9f1980
Show file tree
Hide file tree
Showing 28 changed files with 991 additions and 1,080 deletions.
32 changes: 16 additions & 16 deletions cache/blobs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func computeBlobChain(ctx context.Context, sr *immutableRef, createIfNeeded bool

eg.Go(func() error {
_, err := g.Do(ctx, fmt.Sprintf("%s-%t", sr.ID(), createIfNeeded), func(ctx context.Context) (interface{}, error) {
if getBlob(sr.md) != "" {
if sr.getBlob() != "" {
return nil, nil
}
if !createIfNeeded {
Expand Down Expand Up @@ -187,7 +187,7 @@ func (sr *immutableRef) setBlob(ctx context.Context, compressionType compression
sr.mu.Lock()
defer sr.mu.Unlock()

if getBlob(sr.md) != "" {
if sr.getBlob() != "" {
return nil
}

Expand All @@ -202,11 +202,11 @@ func (sr *immutableRef) setBlob(ctx context.Context, compressionType compression
return err
}

queueDiffID(sr.md, diffID.String())
queueBlob(sr.md, desc.Digest.String())
queueMediaType(sr.md, desc.MediaType)
queueBlobSize(sr.md, desc.Size)
if err := sr.md.Commit(); err != nil {
sr.queueDiffID(diffID)
sr.queueBlob(desc.Digest)
sr.queueMediaType(desc.MediaType)
sr.queueBlobSize(desc.Size)
if err := sr.commitMetadata(); err != nil {
return err
}

Expand All @@ -224,33 +224,33 @@ func (sr *immutableRef) setChains(ctx context.Context) error {
sr.mu.Lock()
defer sr.mu.Unlock()

if getChainID(sr.md) != "" {
if sr.getChainID() != "" {
return nil
}

var chainIDs []digest.Digest
var blobChainIDs []digest.Digest
if sr.parent != nil {
chainIDs = append(chainIDs, digest.Digest(getChainID(sr.parent.md)))
blobChainIDs = append(blobChainIDs, digest.Digest(getBlobChainID(sr.parent.md)))
chainIDs = append(chainIDs, digest.Digest(sr.parent.getChainID()))
blobChainIDs = append(blobChainIDs, digest.Digest(sr.parent.getBlobChainID()))
}
diffID := digest.Digest(getDiffID(sr.md))
diffID := digest.Digest(sr.getDiffID())
chainIDs = append(chainIDs, diffID)
blobChainIDs = append(blobChainIDs, imagespecidentity.ChainID([]digest.Digest{digest.Digest(getBlob(sr.md)), diffID}))
blobChainIDs = append(blobChainIDs, imagespecidentity.ChainID([]digest.Digest{digest.Digest(sr.getBlob()), diffID}))

chainID := imagespecidentity.ChainID(chainIDs)
blobChainID := imagespecidentity.ChainID(blobChainIDs)

queueChainID(sr.md, chainID.String())
queueBlobChainID(sr.md, blobChainID.String())
if err := sr.md.Commit(); err != nil {
sr.queueChainID(chainID)
sr.queueBlobChainID(blobChainID)
if err := sr.commitMetadata(); err != nil {
return err
}
return nil
}

func isTypeWindows(sr *immutableRef) bool {
if GetLayerType(sr) == "windows" {
if sr.GetLayerType() == "windows" {
return true
}
if parent := sr.parent; parent != nil {
Expand Down
66 changes: 33 additions & 33 deletions cache/contenthash/checksum.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ import (
"sync"

"github.com/docker/docker/pkg/fileutils"
"github.com/docker/docker/pkg/idtools"
iradix "github.com/hashicorp/go-immutable-radix"
"github.com/hashicorp/golang-lru/simplelru"
"github.com/moby/buildkit/cache"
"github.com/moby/buildkit/cache/metadata"
"github.com/moby/buildkit/session"
"github.com/moby/buildkit/snapshot"
"github.com/moby/locker"
Expand All @@ -31,8 +29,6 @@ var errNotFound = errors.Errorf("not found")
var defaultManager *cacheManager
var defaultManagerOnce sync.Once

const keyContentHash = "buildkit.contenthash.v0"

func getDefaultManager() *cacheManager {
defaultManagerOnce.Do(func() {
lru, _ := simplelru.NewLRU(20, nil) // error is impossible on positive size
Expand All @@ -58,15 +54,15 @@ func Checksum(ctx context.Context, ref cache.ImmutableRef, path string, opts Che
return getDefaultManager().Checksum(ctx, ref, path, opts, s)
}

func GetCacheContext(ctx context.Context, md *metadata.StorageItem, idmap *idtools.IdentityMapping) (CacheContext, error) {
return getDefaultManager().GetCacheContext(ctx, md, idmap)
func GetCacheContext(ctx context.Context, md cache.RefMetadata) (CacheContext, error) {
return getDefaultManager().GetCacheContext(ctx, md)
}

func SetCacheContext(ctx context.Context, md *metadata.StorageItem, cc CacheContext) error {
func SetCacheContext(ctx context.Context, md cache.RefMetadata, cc CacheContext) error {
return getDefaultManager().SetCacheContext(ctx, md, cc)
}

func ClearCacheContext(md *metadata.StorageItem) {
func ClearCacheContext(md cache.RefMetadata) {
getDefaultManager().clearCacheContext(md.ID())
}

Expand Down Expand Up @@ -99,14 +95,14 @@ func (cm *cacheManager) Checksum(ctx context.Context, ref cache.ImmutableRef, p
}
return "", errors.Errorf("%s: no such file or directory", p)
}
cc, err := cm.GetCacheContext(ctx, ensureOriginMetadata(ref.Metadata()), ref.IdentityMapping())
cc, err := cm.GetCacheContext(ctx, ensureOriginMetadata(ref))
if err != nil {
return "", nil
}
return cc.Checksum(ctx, ref, p, opts, s)
}

func (cm *cacheManager) GetCacheContext(ctx context.Context, md *metadata.StorageItem, idmap *idtools.IdentityMapping) (CacheContext, error) {
func (cm *cacheManager) GetCacheContext(ctx context.Context, md cache.RefMetadata) (CacheContext, error) {
cm.locker.Lock(md.ID())
cm.lruMu.Lock()
v, ok := cm.lru.Get(md.ID())
Expand All @@ -116,7 +112,7 @@ func (cm *cacheManager) GetCacheContext(ctx context.Context, md *metadata.Storag
v.(*cacheContext).linkMap = map[string][][]byte{}
return v.(*cacheContext), nil
}
cc, err := newCacheContext(md, idmap)
cc, err := newCacheContext(md)
if err != nil {
cm.locker.Unlock(md.ID())
return nil, err
Expand All @@ -128,14 +124,14 @@ func (cm *cacheManager) GetCacheContext(ctx context.Context, md *metadata.Storag
return cc, nil
}

func (cm *cacheManager) SetCacheContext(ctx context.Context, md *metadata.StorageItem, cci CacheContext) error {
func (cm *cacheManager) SetCacheContext(ctx context.Context, md cache.RefMetadata, cci CacheContext) error {
cc, ok := cci.(*cacheContext)
if !ok {
return errors.Errorf("invalid cachecontext: %T", cc)
}
if md.ID() != cc.md.ID() {
cc = &cacheContext{
md: md,
md: cacheMetadata{md},
tree: cci.(*cacheContext).tree,
dirtyMap: map[string]struct{}{},
linkMap: map[string][][]byte{},
Expand All @@ -159,7 +155,7 @@ func (cm *cacheManager) clearCacheContext(id string) {

type cacheContext struct {
mu sync.RWMutex
md *metadata.StorageItem
md cacheMetadata
tree *iradix.Tree
dirty bool // needs to be persisted to disk

Expand All @@ -168,7 +164,20 @@ type cacheContext struct {
node *iradix.Node
dirtyMap map[string]struct{}
linkMap map[string][][]byte
idmap *idtools.IdentityMapping
}

type cacheMetadata struct {
cache.RefMetadata
}

const keyContentHash = "buildkit.contenthash.v0"

func (md cacheMetadata) GetContentHash() ([]byte, error) {
return md.GetExternal(keyContentHash)
}

func (md cacheMetadata) SetContentHash(dt []byte) error {
return md.SetExternal(keyContentHash, dt)
}

type mount struct {
Expand Down Expand Up @@ -209,13 +218,12 @@ func (m *mount) clean() error {
return nil
}

func newCacheContext(md *metadata.StorageItem, idmap *idtools.IdentityMapping) (*cacheContext, error) {
func newCacheContext(md cache.RefMetadata) (*cacheContext, error) {
cc := &cacheContext{
md: md,
md: cacheMetadata{md},
tree: iradix.New(),
dirtyMap: map[string]struct{}{},
linkMap: map[string][][]byte{},
idmap: idmap,
}
if err := cc.load(); err != nil {
return nil, err
Expand All @@ -224,7 +232,7 @@ func newCacheContext(md *metadata.StorageItem, idmap *idtools.IdentityMapping) (
}

func (cc *cacheContext) load() error {
dt, err := cc.md.GetExternal(keyContentHash)
dt, err := cc.md.GetContentHash()
if err != nil {
return nil
}
Expand Down Expand Up @@ -265,7 +273,7 @@ func (cc *cacheContext) save() error {
return err
}

return cc.md.SetExternal(keyContentHash, dt)
return cc.md.SetContentHash(dt)
}

func keyPath(p string) string {
Expand Down Expand Up @@ -1016,20 +1024,12 @@ func addParentToMap(d string, m map[string]struct{}) {
addParentToMap(d, m)
}

func ensureOriginMetadata(md *metadata.StorageItem) *metadata.StorageItem {
v := md.Get("cache.equalMutable") // TODO: const
if v == nil {
return md
}
var mutable string
if err := v.Unmarshal(&mutable); err != nil {
return md
}
si, ok := md.Storage().Get(mutable)
if ok {
return si
func ensureOriginMetadata(md cache.RefMetadata) cache.RefMetadata {
em, ok := md.GetEqualMutable()
if !ok {
em = md
}
return md
return em
}

var pool32K = sync.Pool{
Expand Down
Loading

0 comments on commit a9f1980

Please sign in to comment.