Skip to content

Commit

Permalink
Use BlobInfoCache2 instead of types.BlobInfoCache in internal/types
Browse files Browse the repository at this point in the history 
This should not change behavior, but it changes the overhead:
- Transports (like dockerImageDestination) now don't need to care about
  the old interface
- ... it's now handled by internal/imagedestination/impl - whether
  the transport uses the cache or not.

Overall, this should make the internal callers and internal implementations
a bit simpler and faster, at the cost of moving the overhead to the
compat infrastructure, and paying the cost for more external callers.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
@mtrmac
mtrmac committed Feb 15, 2022
1 parent 8c7520e commit fa67f89
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 13 deletions.
9 changes: 4 additions & 5 deletions docker/docker_image_dest.go
View file
Expand Up @@ -238,7 +238,7 @@ func (d *dockerImageDestination) SupportsPutBlobPartial() bool {
// It is available only if SupportsPutBlobPartial().
// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
// should fall back to PutBlobWithOptions.
func (d *dockerImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache types.BlobInfoCache) (types.BlobInfo, error) {
func (d *dockerImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error) {
return types.BlobInfo{}, fmt.Errorf("internal error: PutBlobPartial is not supported by the %q transport", d.Reference().Transport().Name())
}

Expand Down Expand Up @@ -317,7 +317,7 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc
// tryReusingExactBlob is a subset of TryReusingBlob which _only_ looks for exactly the specified
// blob in the current repository, with no cross-repo reuse or mounting; cache may be updated, it is not read.
// The caller must ensure info.Digest is set.
func (d *dockerImageDestination) tryReusingExactBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache) (bool, types.BlobInfo, error) {
func (d *dockerImageDestination) tryReusingExactBlob(ctx context.Context, info types.BlobInfo, cache blobinfocache.BlobInfoCache2) (bool, types.BlobInfo, error) {
exists, size, err := d.blobExists(ctx, d.ref.ref, info.Digest, nil)
if err != nil {
return false, types.BlobInfo{}, err
Expand Down Expand Up @@ -351,8 +351,7 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context,
}

// Then try reusing blobs from other locations.
bic := blobinfocache.FromBlobInfoCache(options.Cache)
candidates := bic.CandidateLocations2(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, options.CanSubstitute)
candidates := options.Cache.CandidateLocations2(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, options.CanSubstitute)
for _, candidate := range candidates {
candidateRepo, err := parseBICLocationReference(candidate.Location)
if err != nil {
Expand Down Expand Up @@ -406,7 +405,7 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context,
}
}

bic.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref))
options.Cache.RecordKnownLocation(d.ref.Transport(), bicTransportScope(d.ref), candidate.Digest, newBICLocationReference(d.ref))

compressionOperation, compressionAlgorithm, err := blobinfocache.OperationAndAlgorithmForCompressor(candidate.CompressorName)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions internal/imagedestination/impl/compat.go
View file
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"io"

"github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/types"
)
Expand Down Expand Up @@ -40,7 +41,7 @@ func AddCompat(dest private.ImageDestinationInternalOnly) Compat {
// If stream.Read() at any time, ESPECIALLY at end of input, returns an error, PutBlob MUST 1) fail, and 2) delete any data stored so far.
func (c *Compat) PutBlob(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, cache types.BlobInfoCache, isConfig bool) (types.BlobInfo, error) {
return c.dest.PutBlobWithOptions(ctx, stream, inputInfo, private.PutBlobOptions{
Cache: cache,
Cache: blobinfocache.FromBlobInfoCache(cache),
IsConfig: isConfig,
})
}
Expand All @@ -56,7 +57,7 @@ func (c *Compat) PutBlob(ctx context.Context, stream io.Reader, inputInfo types.
// May use and/or update cache.
func (c *Compat) TryReusingBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache, canSubstitute bool) (bool, types.BlobInfo, error) {
return c.dest.TryReusingBlobWithOptions(ctx, info, private.TryReusingBlobOptions{
Cache: cache,
Cache: blobinfocache.FromBlobInfoCache(cache),
CanSubstitute: canSubstitute,
})
}
3 changes: 2 additions & 1 deletion internal/imagedestination/wrapper.go
View file
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io"

"github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/types"
)
Expand Down Expand Up @@ -53,7 +54,7 @@ func (w *wrapped) PutBlobWithOptions(ctx context.Context, stream io.Reader, inpu
// It is available only if SupportsPutBlobPartial().
// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
// should fall back to PutBlobWithOptions.
func (w *wrapped) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache types.BlobInfoCache) (types.BlobInfo, error) {
func (w *wrapped) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error) {
return types.BlobInfo{}, fmt.Errorf("internal error: PutBlobPartial is not supported by the %q transport", w.Reference().Transport().Name())
}

Expand Down
9 changes: 5 additions & 4 deletions internal/private/private.go
View file
Expand Up @@ -5,6 +5,7 @@ import (
"io"

"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/types"
)

Expand Down Expand Up @@ -38,7 +39,7 @@ type ImageDestinationInternalOnly interface {
// It is available only if SupportsPutBlobPartial().
// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
// should fall back to PutBlobWithOptions.
PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, cache types.BlobInfoCache) (types.BlobInfo, error)
PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error)

// TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination
// (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree).
Expand All @@ -59,8 +60,8 @@ type ImageDestination interface {

// PutBlobOptions are used in PutBlobWithOptions.
type PutBlobOptions struct {
Cache types.BlobInfoCache // Cache to optionally update with the uploaded bloblook up blob infos.
IsConfig bool // True if the blob is a config
Cache blobinfocache.BlobInfoCache2 // Cache to optionally update with the uploaded bloblook up blob infos.
IsConfig bool // True if the blob is a config

// The following fields are new to internal/private. Users of internal/private MUST fill them in,
// but they also must expect that they will be ignored by types.ImageDestination transports.
Expand All @@ -74,7 +75,7 @@ type PutBlobOptions struct {

// TryReusingBlobOptions are used in TryReusingBlobWithOptions.
type TryReusingBlobOptions struct {
Cache types.BlobInfoCache // Cache to use and/or update.
Cache blobinfocache.BlobInfoCache2 // Cache to use and/or update.
// If true, it is allowed to use an equivalent of the desired blob;
// in that case the returned info may not match the input.
CanSubstitute bool
Expand Down
3 changes: 2 additions & 1 deletion storage/storage_image.go
View file
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/image"
"github.com/containers/image/v5/internal/blobinfocache"
"github.com/containers/image/v5/internal/imagedestination/impl"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/internal/putblobdigest"
Expand Down Expand Up @@ -569,7 +570,7 @@ func (f *zstdFetcher) GetBlobAt(chunks []chunked.ImageSourceChunk) (chan io.Read
// It is available only if SupportsPutBlobPartial().
// Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller
// should fall back to PutBlobWithOptions.
func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache types.BlobInfoCache) (types.BlobInfo, error) {
func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (types.BlobInfo, error) {
fetcher := zstdFetcher{
chunkAccessor: chunkAccessor,
ctx: ctx,
Expand Down

0 comments on commit fa67f89

Please sign in to comment.