From 24dbb79767ff5c35315b652e2b310bf58d97761a Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Tue, 9 Oct 2018 11:29:58 +0800 Subject: [PATCH] enhance: cache image size and oci image spec in pouchd when the containerd manages many big images, the pouchd ListImage will consume a lot of CPU resources caused by containerd.Content.Read stream grpc call. In the k8s, the kubelet gc image manager will send the ListImage request every 30s. If the containerd consumes too many CPU resources, it will impact the container. Therefore, pouchd needs to cache the data. Signed-off-by: Wei Fu --- daemon/mgr/image.go | 95 ++++++++++++++++++++------------------- daemon/mgr/image_store.go | 59 +++++++++++++++++++----- 2 files changed, 96 insertions(+), 58 deletions(-) diff --git a/daemon/mgr/image.go b/daemon/mgr/image.go index 1843046a4..92d0e9b91 100644 --- a/daemon/mgr/image.go +++ b/daemon/mgr/image.go @@ -145,17 +145,12 @@ func (mgr *ImageManager) PullImage(ctx context.Context, ref string, authConfig * // GetImage returns imageInfo by reference. func (mgr *ImageManager) GetImage(ctx context.Context, idOrRef string) (*types.ImageInfo, error) { - _, _, ref, err := mgr.CheckReference(ctx, idOrRef) - if err != nil { - return nil, err - } - - img, err := mgr.client.GetImage(ctx, ref.String()) + id, _, _, err := mgr.CheckReference(ctx, idOrRef) if err != nil { return nil, err } - imgInfo, err := mgr.containerdImageToImageInfo(ctx, img) + imgInfo, err := mgr.containerdImageToImageInfo(ctx, id) if err != nil { return nil, err } @@ -164,34 +159,17 @@ func (mgr *ImageManager) GetImage(ctx context.Context, idOrRef string) (*types.I // ListImages lists images stored by containerd. func (mgr *ImageManager) ListImages(ctx context.Context, filter ...string) ([]types.ImageInfo, error) { - imgs, err := mgr.client.ListImages(ctx, filter...) - if err != nil { - return nil, err - } - - imgInfosIndexByID := make(map[string]types.ImageInfo) - for _, img := range imgs { - imgCfg, err := img.Config(ctx) - if err != nil { - logrus.Warnf("failed to get image config info during list images: %v", err) - continue - } - - if _, ok := imgInfosIndexByID[imgCfg.Digest.String()]; ok { - continue - } + // TODO: support filter functionality + ctrdImageInfos := mgr.localStore.ListCtrdImageInfo() + imgInfos := make([]types.ImageInfo, 0, len(ctrdImageInfos)) - imgInfo, err := mgr.containerdImageToImageInfo(ctx, img) + for _, img := range ctrdImageInfos { + imgInfo, err := mgr.containerdImageToImageInfo(ctx, img.ID) if err != nil { - logrus.Warnf("failed to convert containerd image(%v) to ImageInfo during list images: %v", img.Name(), err) + logrus.Warnf("failed to convert containerd image(%v) to ImageInfo during list images: %v", img.ID, err) continue } - imgInfosIndexByID[imgInfo.ID] = imgInfo - } - - imgInfos := make([]types.ImageInfo, 0, len(imgInfosIndexByID)) - for _, v := range imgInfosIndexByID { - imgInfos = append(imgInfos, v) + imgInfos = append(imgInfos, imgInfo) } return imgInfos, nil } @@ -211,6 +189,16 @@ func (mgr *ImageManager) RemoveImage(ctx context.Context, idOrRef string, force return err } + // since there is no rollback functionality, no guarantee that the + // containerd.RemoveImage must success. so if the localStore has been + // remove all the primary references, we should clear the CtrdImageInfo + // cache. + defer func() { + if len(mgr.localStore.GetPrimaryReferences(id)) == 0 { + mgr.localStore.ClearCtrdImageInfo(id) + } + }() + // should remove all the references if the reference is ID (Named Only) // or Digest ID (Tagged Named) if reference.IsNamedOnly(namedRef) || @@ -462,7 +450,26 @@ func (mgr *ImageManager) StoreImageReference(ctx context.Context, img containerd return err } - return mgr.addReferenceIntoStore(imgCfg.Digest, namedRef, img.Target().Digest) + size, err := img.Size(ctx) + if err != nil { + return err + } + + ociImage, err := containerdImageToOciImage(ctx, img) + if err != nil { + return err + } + + if err := mgr.addReferenceIntoStore(imgCfg.Digest, namedRef, img.Target().Digest); err != nil { + return err + } + + mgr.localStore.CacheCtrdImageInfo(imgCfg.Digest, CtrdImageInfo{ + ID: imgCfg.Digest, + Size: size, + OCISpec: ociImage, + }) + return nil } func (mgr *ImageManager) addReferenceIntoStore(id digest.Digest, ref reference.Named, dig digest.Digest) error { @@ -486,28 +493,22 @@ func (mgr *ImageManager) addReferenceIntoStore(id digest.Digest, ref reference.N return nil } -func (mgr *ImageManager) containerdImageToImageInfo(ctx context.Context, img containerd.Image) (types.ImageInfo, error) { - desc, err := img.Config(ctx) - if err != nil { - return types.ImageInfo{}, err - } - - size, err := img.Size(ctx) - if err != nil { - return types.ImageInfo{}, err - } - - ociImage, err := containerdImageToOciImage(ctx, img) +func (mgr *ImageManager) containerdImageToImageInfo(ctx context.Context, id digest.Digest) (types.ImageInfo, error) { + ctrdImageInfo, err := mgr.localStore.GetCtrdImageInfo(id) if err != nil { + if err == errCtrdImageInfoNotExist { + return types.ImageInfo{}, pkgerrors.Wrapf(errtypes.ErrNotfound, "failed to get ctrd image info from cache by imageID: %v", id) + } return types.ImageInfo{}, err } var ( + ociImage = ctrdImageInfo.OCISpec repoTags = make([]string, 0) repoDigests = make([]string, 0) ) - for _, ref := range mgr.localStore.GetReferences(desc.Digest) { + for _, ref := range mgr.localStore.GetReferences(ctrdImageInfo.ID) { switch ref.(type) { case reference.Tagged: repoTags = append(repoTags, ref.String()) @@ -520,7 +521,7 @@ func (mgr *ImageManager) containerdImageToImageInfo(ctx context.Context, img con Architecture: ociImage.Architecture, Config: getImageInfoConfigFromOciImage(ociImage), CreatedAt: ociImage.Created.Format(utils.TimeLayout), - ID: desc.Digest.String(), + ID: ctrdImageInfo.ID.String(), Os: ociImage.OS, RepoDigests: repoDigests, RepoTags: repoTags, @@ -528,7 +529,7 @@ func (mgr *ImageManager) containerdImageToImageInfo(ctx context.Context, img con Type: ociImage.RootFS.Type, Layers: digestSliceToStringSlice(ociImage.RootFS.DiffIDs), }, - Size: size, + Size: ctrdImageInfo.Size, }, nil } diff --git a/daemon/mgr/image_store.go b/daemon/mgr/image_store.go index 05b309106..0111dd1e6 100644 --- a/daemon/mgr/image_store.go +++ b/daemon/mgr/image_store.go @@ -9,10 +9,13 @@ import ( "github.com/alibaba/pouch/pkg/reference" digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" pkgerrors "github.com/pkg/errors" "github.com/tchap/go-patricia/patricia" ) +var errCtrdImageInfoNotExist = fmt.Errorf("ctrd image info does not exist") + // imageStore stores the relationship between references. // // Primary reference is the reference used for pulling the image. For example, @@ -62,6 +65,18 @@ type imageStore struct { // primaryRefsIndexByID stores primay references, index by image ID primaryRefsIndexByID map[digest.Digest]referenceMap + + // cache size, ociImage to avoid open stream grpc to connect containerd, + // because it's too expensive if open/read file too often + // cache index by image ID + imageInfoCache map[digest.Digest]CtrdImageInfo +} + +// CtrdImageInfo is used to cache the id, size and oci image information. +type CtrdImageInfo struct { + ID digest.Digest + Size int64 + OCISpec ocispec.Image } // referenceMap represents reference string to corresponding reference.Named @@ -76,6 +91,8 @@ func newImageStore() (*imageStore, error) { primaryRefsIndexByID: make(map[digest.Digest]referenceMap), refsIndexByPrimaryRef: make(map[string]referenceMap), + + imageInfoCache: make(map[digest.Digest]CtrdImageInfo), }, nil } @@ -276,23 +293,43 @@ func (store *imageStore) RemoveReference(id digest.Digest, ref reference.Named) return nil } -// RemoveAllReferences removes all the reference by the given imageID. -func (store *imageStore) RemoveAllReferences(id digest.Digest) error { +// ListCtrdImageInfo returns all the CtrdImageInfo. +func (store *imageStore) ListCtrdImageInfo() []CtrdImageInfo { store.Lock() defer store.Unlock() - for pRefStr := range store.primaryRefsIndexByID[id] { - for ref := range store.refsIndexByPrimaryRef[pRefStr] { - delete(store.primaryRefIndexByRef, ref) - } + res := make([]CtrdImageInfo, 0, len(store.imageInfoCache)) + for _, val := range store.imageInfoCache { + res = append(res, val) + } + return res +} - delete(store.refsIndexByPrimaryRef, pRefStr) - delete(store.idIndexByPrimaryRef, pRefStr) +// GetCtrdImageInfo returns CtrdImageInfo by specific id. +func (store *imageStore) GetCtrdImageInfo(id digest.Digest) (CtrdImageInfo, error) { + store.Lock() + defer store.Unlock() + + if i, ok := store.imageInfoCache[id]; ok { + return i, nil } + return CtrdImageInfo{}, errCtrdImageInfoNotExist +} - delete(store.primaryRefsIndexByID, id) - store.idSet.Delete(patricia.Prefix(id.String())) - return nil +// CacheCtrdImageInfo caches the oci image by image ID. +func (store *imageStore) CacheCtrdImageInfo(id digest.Digest, img CtrdImageInfo) { + store.Lock() + defer store.Unlock() + + store.imageInfoCache[id] = img +} + +// ClearCtrdImageInfo caches the oci image by image ID. +func (store *imageStore) ClearCtrdImageInfo(id digest.Digest) { + store.Lock() + defer store.Unlock() + + delete(store.imageInfoCache, id) } // getLastComponentInReferenceName will return the last component in the reference.Named().