From 7341c3b52cbc09f66cf41f9b8ea8e134b0277781 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 May 2025 18:10:50 +0300 Subject: [PATCH 1/4] layer: do not list all object when removing bucket It makes zero sense, we only need one object to satisfy condition here. Signed-off-by: Roman Khimov --- api/layer/layer.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/api/layer/layer.go b/api/layer/layer.go index 05116c26..380dd42f 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -1051,11 +1051,21 @@ func (n *layer) ResolveBucket(ctx context.Context, name string) (cid.ID, error) } func (n *layer) DeleteBucket(ctx context.Context, p *DeleteBucketParams) error { - objects, err := n.searchAllVersionsInNeoFS(ctx, p.BktInfo, p.BktInfo.Owner, "", false) + var ( + filters = make(object.SearchFilters, 0, 2) + opts client.SearchObjectsOptions + ) + + opts.SetCount(1) // Enough for the purpose. + if bt := bearerTokenFromContext(ctx, p.BktInfo.Owner); bt != nil { + opts.WithBearerToken(*bt) + } + filters.AddTypeFilter(object.MatchStringEqual, object.TypeRegular) + filters.AddFilter(s3headers.MetaType, "", object.MatchNotPresent) + + objects, err := n.neoFS.SearchObjectsV2(ctx, p.BktInfo.CID, filters, []string{object.FilterType}, opts) if err != nil { - if !errors.Is(err, ErrNodeNotFound) { - return err - } + return err } // there are only Regular objects in slice. From e24ccceb635268d3efcb9fe540a4139c5064d6b8 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 May 2025 18:24:30 +0300 Subject: [PATCH 2/4] layer: deduplicate searches in GetExtendedObjectInfo() No functional changes. Signed-off-by: Roman Khimov --- api/layer/layer.go | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/api/layer/layer.go b/api/layer/layer.go index 380dd42f..6dd75b7c 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -581,31 +581,25 @@ func (n *layer) GetExtendedObjectInfo(ctx context.Context, p *HeadObjectParams) owner = n.Owner(ctx) ) - if len(p.VersionID) == 0 { - heads, err := n.searchAllVersionsInNeoFS(ctx, p.BktInfo, owner, p.Object, false) - if err != nil { - if errors.Is(err, ErrNodeNotFound) { - return nil, s3errors.GetAPIError(s3errors.ErrNoSuchKey) + versions, err := n.searchAllVersionsInNeoFS(ctx, p.BktInfo, owner, p.Object, p.VersionID == data.UnversionedObjectVersionID) + if err != nil { + if errors.Is(err, ErrNodeNotFound) { + if len(p.VersionID) == 0 { + err = s3errors.GetAPIError(s3errors.ErrNoSuchKey) + } else { + err = s3errors.GetAPIError(s3errors.ErrNoSuchVersion) } - - return nil, err } + return nil, err + } - if heads[0].IsDeleteMarker { + if len(p.VersionID) == 0 { + if versions[0].IsDeleteMarker { return nil, s3errors.GetAPIError(s3errors.ErrNoSuchKey) } - id = heads[0].ID + id = versions[0].ID } else if p.VersionID == data.UnversionedObjectVersionID { - versions, err := n.searchAllVersionsInNeoFS(ctx, p.BktInfo, owner, p.Object, true) - if err != nil { - if errors.Is(err, ErrNodeNotFound) { - return nil, s3errors.GetAPIError(s3errors.ErrNoSuchVersion) - } - - return nil, err - } - id = versions[0].ID } else { settings, err = n.GetBucketSettings(ctx, p.BktInfo) @@ -613,14 +607,6 @@ func (n *layer) GetExtendedObjectInfo(ctx context.Context, p *HeadObjectParams) return nil, fmt.Errorf("get bucket settings: %w", err) } - versions, err := n.searchAllVersionsInNeoFS(ctx, p.BktInfo, owner, p.Object, false) - if err != nil { - if errors.Is(err, ErrNodeNotFound) { - return nil, s3errors.GetAPIError(s3errors.ErrNoSuchVersion) - } - return nil, err - } - var foundVersion *allVersionsSearchResult if settings.VersioningEnabled() { From b81e9d70d3060cf7a8acb1dad8f2a4a7b3570c4c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 May 2025 19:00:57 +0300 Subject: [PATCH 3/4] layer: deduplicate logic in deleteObject() Unify logging and cache handling at the same time. Signed-off-by: Roman Khimov --- api/layer/layer.go | 106 ++++++++++++--------------------------------- 1 file changed, 28 insertions(+), 78 deletions(-) diff --git a/api/layer/layer.go b/api/layer/layer.go index 6dd75b7c..eb958e46 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -876,75 +876,35 @@ func (n *layer) CopyObject(ctx context.Context, p *CopyObjectParams) (*data.Exte } func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, settings *data.BucketSettings, obj *VersionedObject) *VersionedObject { - if settings.VersioningEnabled() { - if len(obj.VersionID) > 0 { - var deleteOID oid.ID - - if obj.VersionID == data.UnversionedObjectVersionID { - versions, err := n.searchAllVersionsInNeoFS(ctx, bkt, bkt.Owner, obj.Name, true) - if err != nil { - obj.Error = fmt.Errorf("search versions: %w", err) - if errors.Is(err, ErrNodeNotFound) { - obj.Error = nil - } - - return obj - } - - if len(versions) == 0 { - obj.Error = nil - return obj - } + // Specific version ID passed, drop it. + if len(obj.VersionID) > 0 && obj.VersionID != data.UnversionedObjectVersionID { + // Simple single ID deletion. + var deleteOID oid.ID - for _, version := range versions { - if obj.Error = n.objectDelete(ctx, bkt, version.ID); obj.Error != nil { - return obj - } - } - } else { - if err := deleteOID.DecodeString(obj.VersionID); err != nil { - obj.Error = fmt.Errorf("decode version: %w", err) - return obj - } + if err := deleteOID.DecodeString(obj.VersionID); err != nil { + obj.Error = fmt.Errorf("decode version: %w", err) + return obj + } - if obj.Error = n.objectDelete(ctx, bkt, deleteOID); obj.Error != nil { - return obj - } - } - } else { - var markerOID oid.ID - markerOID, obj.Error = n.putDeleteMarker(ctx, bkt, obj.Name) - obj.DeleteMarkVersion = markerOID.EncodeToString() + if obj.Error = n.objectDelete(ctx, bkt, deleteOID); obj.Error != nil { + return obj } n.cache.DeleteObjectName(bkt.CID, bkt.Name, obj.Name) return obj } - if settings.VersioningSuspended() { - obj.VersionID = data.UnversionedObjectVersionID - - versions, err := n.searchAllVersionsInNeoFS(ctx, bkt, bkt.Owner, obj.Name, true) - if err != nil { - if errors.Is(err, ErrNodeNotFound) { - obj.Error = nil - } else { - obj.Error = fmt.Errorf("search versions: %w", err) - } - - return obj - } - - for _, version := range versions { - if obj.Error = n.objectDelete(ctx, bkt, version.ID); obj.Error != nil { - return obj - } - } + if settings.VersioningEnabled() && len(obj.VersionID) == 0 { + // No version specified -> add deletion marker. + var markerOID oid.ID + markerOID, obj.Error = n.putDeleteMarker(ctx, bkt, obj.Name) + obj.DeleteMarkVersion = markerOID.EncodeToString() + n.cache.DeleteObjectName(bkt.CID, bkt.Name, obj.Name) return obj } - versions, err := n.searchAllVersionsInNeoFS(ctx, bkt, bkt.Owner, obj.Name, false) + versions, err := n.searchAllVersionsInNeoFS(ctx, bkt, bkt.Owner, obj.Name, settings.VersioningEnabled() || settings.VersioningSuspended()) if err != nil { if errors.Is(err, ErrNodeNotFound) { obj.Error = nil @@ -955,30 +915,20 @@ func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, settings return obj } - if obj.VersionID == "" { - for _, ver := range versions { - if obj.Error = n.objectDelete(ctx, bkt, ver.ID); obj.Error != nil { - n.log.Error("could not delete object", zap.Error(obj.Error), zap.Stringer("oid", ver.ID)) - if isErrObjectAlreadyRemoved(obj.Error) { - obj.Error = nil - continue - } - - return obj - } - } - } else { - for _, ver := range versions { - if ver.ID.EncodeToString() == obj.VersionID { - if obj.Error = n.objectDelete(ctx, bkt, ver.ID); obj.Error != nil { - return obj - } - - return obj + for _, version := range versions { + if obj.Error = n.objectDelete(ctx, bkt, version.ID); obj.Error != nil { + n.log.Error("could not delete object", zap.Error(obj.Error), zap.Stringer("oid", version.ID)) + if isErrObjectAlreadyRemoved(obj.Error) { + obj.Error = nil + continue } + return obj } } - + if settings.VersioningSuspended() { + // Return unversioned ID. + obj.VersionID = data.UnversionedObjectVersionID + } n.cache.DeleteObjectName(bkt.CID, bkt.Name, obj.Name) return obj From 012b4ec81c15104180391860c54fe7b6bee7dcb5 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 May 2025 21:12:00 +0300 Subject: [PATCH 4/4] layer: fix unversioned GetExtendedObjectInfo() result I believe delete marker is not a correct expected result in this case, it's 404 semantically, so it should be returned. Signed-off-by: Roman Khimov --- api/layer/layer.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/api/layer/layer.go b/api/layer/layer.go index eb958e46..b037afe1 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -593,13 +593,14 @@ func (n *layer) GetExtendedObjectInfo(ctx context.Context, p *HeadObjectParams) return nil, err } - if len(p.VersionID) == 0 { + if len(p.VersionID) == 0 || p.VersionID == data.UnversionedObjectVersionID { if versions[0].IsDeleteMarker { - return nil, s3errors.GetAPIError(s3errors.ErrNoSuchKey) + if len(p.VersionID) == 0 { + return nil, s3errors.GetAPIError(s3errors.ErrNoSuchKey) + } + return nil, s3errors.GetAPIError(s3errors.ErrNoSuchVersion) } - id = versions[0].ID - } else if p.VersionID == data.UnversionedObjectVersionID { id = versions[0].ID } else { settings, err = n.GetBucketSettings(ctx, p.BktInfo)