From 4ed0eb70129630cf0bc8fd72d588fbb1c5bfaf35 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 30 Oct 2021 08:22:04 -0700 Subject: [PATCH] remove double reads updating object metadata (#13542) Removes RLock/RUnlock for updating metadata, since we already take a write lock to update metadata, this change removes reading of xl.meta as well as an additional lock, the performance gain should increase 3x theoretically for - PutObjectRetention - PutObjectLegalHold This optimization is mainly for Veeam like workloads that require a certain level of iops from these API calls, we were losing iops. --- cmd/api-errors.go | 2 + cmd/bucket-object-lock.go | 54 +++++++++-------- cmd/bucket-replication.go | 34 +++++------ cmd/erasure-object.go | 20 ++++--- cmd/object-api-errors.go | 7 +++ cmd/object-api-interface.go | 4 ++ cmd/object-handlers.go | 115 ++++++++++++++---------------------- 7 files changed, 118 insertions(+), 118 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 7bbf3bfb3..a2372ef5d 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -1956,6 +1956,8 @@ func toAPIErrorCode(ctx context.Context, err error) (apiErr APIErrorCode) { apiErr = ErrNoSuchKey case MethodNotAllowed: apiErr = ErrMethodNotAllowed + case ObjectLocked: + apiErr = ErrObjectLocked case InvalidVersionID: apiErr = ErrInvalidVersionID case VersionNotFound: diff --git a/cmd/bucket-object-lock.go b/cmd/bucket-object-lock.go index 742a45dc5..6fa327c60 100644 --- a/cmd/bucket-object-lock.go +++ b/cmd/bucket-object-lock.go @@ -175,68 +175,76 @@ func enforceRetentionBypassForDelete(ctx context.Context, r *http.Request, bucke // For objects in "Governance" mode, overwrite is allowed if a) object retention date is past OR // governance bypass headers are set and user has governance bypass permissions. // Objects in compliance mode can be overwritten only if retention date is being extended. No mode change is permitted. -func enforceRetentionBypassForPut(ctx context.Context, r *http.Request, bucket, object string, getObjectInfoFn GetObjectInfoFn, objRetention *objectlock.ObjectRetention, cred auth.Credentials, owner bool) (ObjectInfo, APIErrorCode) { +func enforceRetentionBypassForPut(ctx context.Context, r *http.Request, oi ObjectInfo, objRetention *objectlock.ObjectRetention, cred auth.Credentials, owner bool) error { byPassSet := objectlock.IsObjectLockGovernanceBypassSet(r.Header) - opts, err := getOpts(ctx, r, bucket, object) - if err != nil { - return ObjectInfo{}, toAPIErrorCode(ctx, err) - } - - oi, err := getObjectInfoFn(ctx, bucket, object, opts) - if err != nil { - return oi, toAPIErrorCode(ctx, err) - } t, err := objectlock.UTCNowNTP() if err != nil { logger.LogIf(ctx, err) - return oi, ErrObjectLocked + return ObjectLocked{Bucket: oi.Bucket, Object: oi.Name, VersionID: oi.VersionID} } - // Pass in relative days from current time, to additionally to verify "object-lock-remaining-retention-days" policy if any. + // Pass in relative days from current time, to additionally + // to verify "object-lock-remaining-retention-days" policy if any. days := int(math.Ceil(math.Abs(objRetention.RetainUntilDate.Sub(t).Hours()) / 24)) ret := objectlock.GetObjectRetentionMeta(oi.UserDefined) if ret.Mode.Valid() { // Retention has expired you may change whatever you like. if ret.RetainUntilDate.Before(t) { - perm := isPutRetentionAllowed(bucket, object, + apiErr := isPutRetentionAllowed(oi.Bucket, oi.Name, days, objRetention.RetainUntilDate.Time, objRetention.Mode, byPassSet, r, cred, owner) - return oi, perm + switch apiErr { + case ErrAccessDenied: + return errAuthentication + } + return nil } switch ret.Mode { case objectlock.RetGovernance: - govPerm := isPutRetentionAllowed(bucket, object, days, + govPerm := isPutRetentionAllowed(oi.Bucket, oi.Name, days, objRetention.RetainUntilDate.Time, objRetention.Mode, byPassSet, r, cred, owner) // Governance mode retention period cannot be shortened, if x-amz-bypass-governance is not set. if !byPassSet { if objRetention.Mode != objectlock.RetGovernance || objRetention.RetainUntilDate.Before((ret.RetainUntilDate.Time)) { - return oi, ErrObjectLocked + return ObjectLocked{Bucket: oi.Bucket, Object: oi.Name, VersionID: oi.VersionID} } } - return oi, govPerm + switch govPerm { + case ErrAccessDenied: + return errAuthentication + } + return nil case objectlock.RetCompliance: // Compliance retention mode cannot be changed or shortened. // https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock-overview.html#object-lock-retention-modes if objRetention.Mode != objectlock.RetCompliance || objRetention.RetainUntilDate.Before((ret.RetainUntilDate.Time)) { - return oi, ErrObjectLocked + return ObjectLocked{Bucket: oi.Bucket, Object: oi.Name, VersionID: oi.VersionID} } - compliancePerm := isPutRetentionAllowed(bucket, object, + apiErr := isPutRetentionAllowed(oi.Bucket, oi.Name, days, objRetention.RetainUntilDate.Time, objRetention.Mode, false, r, cred, owner) - return oi, compliancePerm + switch apiErr { + case ErrAccessDenied: + return errAuthentication + } + return nil } - return oi, ErrNone + return nil } // No pre-existing retention metadata present. - perm := isPutRetentionAllowed(bucket, object, + apiErr := isPutRetentionAllowed(oi.Bucket, oi.Name, days, objRetention.RetainUntilDate.Time, objRetention.Mode, byPassSet, r, cred, owner) - return oi, perm + switch apiErr { + case ErrAccessDenied: + return errAuthentication + } + return nil } // checkPutObjectLockAllowed enforces object retention policy and legal hold policy diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index 0c22c0689..85ac08e1e 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -910,24 +910,24 @@ func replicateObject(ctx context.Context, ri ReplicateObjectInfo, objectAPI Obje // metadata should be updated with last resync timestamp. if objInfo.ReplicationStatusInternal != newReplStatusInternal || rinfos.ReplicationResynced() { popts := ObjectOptions{ - MTime: objInfo.ModTime, - VersionID: objInfo.VersionID, - UserDefined: make(map[string]string, len(objInfo.UserDefined)), - } - for k, v := range objInfo.UserDefined { - popts.UserDefined[k] = v - } - popts.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = newReplStatusInternal - popts.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) - popts.UserDefined[xhttp.AmzBucketReplicationStatus] = string(rinfos.ReplicationStatus()) - for _, rinfo := range rinfos.Targets { - if rinfo.ResyncTimestamp != "" { - popts.UserDefined[targetResetHeader(rinfo.Arn)] = rinfo.ResyncTimestamp - } - } - if objInfo.UserTags != "" { - popts.UserDefined[xhttp.AmzObjectTagging] = objInfo.UserTags + MTime: objInfo.ModTime, + VersionID: objInfo.VersionID, + EvalMetadataFn: func(oi ObjectInfo) error { + oi.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = newReplStatusInternal + oi.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) + oi.UserDefined[xhttp.AmzBucketReplicationStatus] = string(rinfos.ReplicationStatus()) + for _, rinfo := range rinfos.Targets { + if rinfo.ResyncTimestamp != "" { + oi.UserDefined[targetResetHeader(rinfo.Arn)] = rinfo.ResyncTimestamp + } + } + if objInfo.UserTags != "" { + oi.UserDefined[xhttp.AmzObjectTagging] = objInfo.UserTags + } + return nil + }, } + if _, err = objectAPI.PutObjectMetadata(ctx, bucket, object, popts); err != nil { logger.LogIf(ctx, fmt.Errorf("Unable to update replication metadata for %s/%s(%s): %w", bucket, objInfo.Name, objInfo.VersionID, err)) diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index 3b5405a2f..fd0064e43 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -1440,13 +1440,21 @@ func (er erasureObjects) PutObjectMetadata(ctx context.Context, bucket, object s return ObjectInfo{}, toObjectErr(err, bucket, object) } if fi.Deleted { - if opts.VersionID == "" { - return ObjectInfo{}, toObjectErr(errFileNotFound, bucket, object) - } return ObjectInfo{}, toObjectErr(errMethodNotAllowed, bucket, object) } - for k, v := range opts.UserDefined { + // if version-id is not specified retention is supposed to be set on the latest object. + if opts.VersionID == "" { + opts.VersionID = fi.VersionID + } + + objInfo := fi.ToObjectInfo(bucket, object) + if opts.EvalMetadataFn != nil { + if err := opts.EvalMetadataFn(objInfo); err != nil { + return ObjectInfo{}, err + } + } + for k, v := range objInfo.UserDefined { fi.Metadata[k] = v } fi.ModTime = opts.MTime @@ -1456,9 +1464,7 @@ func (er erasureObjects) PutObjectMetadata(ctx context.Context, bucket, object s return ObjectInfo{}, toObjectErr(err, bucket, object) } - objInfo := fi.ToObjectInfo(bucket, object) - return objInfo, nil - + return fi.ToObjectInfo(bucket, object), nil } // PutObjectTags - replace or add tags to an existing object diff --git a/cmd/object-api-errors.go b/cmd/object-api-errors.go index 772594ad1..7f34e7fea 100644 --- a/cmd/object-api-errors.go +++ b/cmd/object-api-errors.go @@ -291,6 +291,13 @@ func (e MethodNotAllowed) Error() string { return "Method not allowed: " + e.Bucket + "/" + e.Object } +// ObjectLocked object is currently WORM protected. +type ObjectLocked GenericError + +func (e ObjectLocked) Error() string { + return "Object is WORM protected and cannot be overwritten: " + e.Bucket + "/" + e.Object + "(" + e.VersionID + ")" +} + // ObjectAlreadyExists object already exists. type ObjectAlreadyExists GenericError diff --git a/cmd/object-api-interface.go b/cmd/object-api-interface.go index aa2d1cad7..758da9d28 100644 --- a/cmd/object-api-interface.go +++ b/cmd/object-api-interface.go @@ -33,6 +33,9 @@ import ( // CheckPreconditionFn returns true if precondition check failed. type CheckPreconditionFn func(o ObjectInfo) bool +// EvalMetadataFn validates input objInfo and returns an updated metadata +type EvalMetadataFn func(o ObjectInfo) error + // GetObjectInfoFn is the signature of GetObjectInfo function. type GetObjectInfoFn func(ctx context.Context, bucket, object string, opts ObjectOptions) (ObjectInfo, error) @@ -50,6 +53,7 @@ type ObjectOptions struct { UserDefined map[string]string // only set in case of POST/PUT operations PartNumber int // only useful in case of GetObject/HeadObject CheckPrecondFn CheckPreconditionFn // only set during GetObject/HeadObject/CopyObjectPart preconditional valuation + EvalMetadataFn EvalMetadataFn // only set for retention settings, meant to be used only when updating metadata in-place. DeleteReplication ReplicationState // Represents internal replication state needed for Delete replication Transition TransitionOptions Expiration ExpirationOptions diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index cd50dfcf1..9ca68d9e0 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -3521,53 +3521,39 @@ func (api objectAPIHandlers) PutObjectLegalHoldHandler(w http.ResponseWriter, r return } - getObjectInfo := objectAPI.GetObjectInfo - if api.CacheAPI() != nil { - getObjectInfo = api.CacheAPI().GetObjectInfo - } - opts, err := getOpts(ctx, r, bucket, object) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } - objInfo, err := getObjectInfo(ctx, bucket, object, opts) + popts := ObjectOptions{ + MTime: opts.MTime, + VersionID: opts.VersionID, + EvalMetadataFn: func(oi ObjectInfo) error { + oi.UserDefined[strings.ToLower(xhttp.AmzObjectLockLegalHold)] = strings.ToUpper(string(legalHold.Status)) + oi.UserDefined[ReservedMetadataPrefixLower+ObjectLockLegalHoldTimestamp] = UTCNow().Format(time.RFC3339Nano) + + dsc := mustReplicate(ctx, bucket, object, getMustReplicateOptions(oi, replication.MetadataReplicationType, opts)) + if dsc.ReplicateAny() { + oi.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) + oi.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus() + } + return nil + }, + } + + objInfo, err := objectAPI.PutObjectMetadata(ctx, bucket, object, popts) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } - if objInfo.DeleteMarker { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMethodNotAllowed), r.URL) - return - } - objInfo.UserDefined[strings.ToLower(xhttp.AmzObjectLockLegalHold)] = strings.ToUpper(string(legalHold.Status)) - objInfo.UserDefined[ReservedMetadataPrefixLower+ObjectLockLegalHoldTimestamp] = UTCNow().Format(time.RFC3339Nano) dsc := mustReplicate(ctx, bucket, object, getMustReplicateOptions(objInfo, replication.MetadataReplicationType, opts)) - if dsc.ReplicateAny() { - objInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) - objInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus() - } - // if version-id is not specified retention is supposed to be set on the latest object. - if opts.VersionID == "" { - opts.VersionID = objInfo.VersionID - } - popts := ObjectOptions{ - MTime: opts.MTime, - VersionID: opts.VersionID, - UserDefined: make(map[string]string, len(objInfo.UserDefined)), - } - for k, v := range objInfo.UserDefined { - popts.UserDefined[k] = v - } - if _, err = objectAPI.PutObjectMetadata(ctx, bucket, object, popts); err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) - return - } if dsc.ReplicateAny() { scheduleReplication(ctx, objInfo.Clone(), objectAPI, dsc, replication.MetadataReplicationType) } + writeSuccessResponseHeadersOnly(w) // Notify object event. @@ -3703,50 +3689,37 @@ func (api objectAPIHandlers) PutObjectRetentionHandler(w http.ResponseWriter, r return } - getObjectInfo := objectAPI.GetObjectInfo - if api.CacheAPI() != nil { - getObjectInfo = api.CacheAPI().GetObjectInfo - } - - objInfo, s3Err := enforceRetentionBypassForPut(ctx, r, bucket, object, getObjectInfo, objRetention, cred, owner) - if s3Err != ErrNone { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL) - return - } - if objInfo.DeleteMarker { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMethodNotAllowed), r.URL) - return - } - if objRetention.Mode.Valid() { - objInfo.UserDefined[strings.ToLower(xhttp.AmzObjectLockMode)] = string(objRetention.Mode) - objInfo.UserDefined[strings.ToLower(xhttp.AmzObjectLockRetainUntilDate)] = objRetention.RetainUntilDate.UTC().Format(time.RFC3339) - } else { - objInfo.UserDefined[strings.ToLower(xhttp.AmzObjectLockMode)] = "" - objInfo.UserDefined[strings.ToLower(xhttp.AmzObjectLockRetainUntilDate)] = "" - } - objInfo.UserDefined[ReservedMetadataPrefixLower+ObjectLockRetentionTimestamp] = UTCNow().Format(time.RFC3339Nano) - - dsc := mustReplicate(ctx, bucket, object, getMustReplicateOptions(objInfo, replication.MetadataReplicationType, opts)) - if dsc.ReplicateAny() { - objInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) - objInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus() - } - // if version-id is not specified retention is supposed to be set on the latest object. - if opts.VersionID == "" { - opts.VersionID = objInfo.VersionID - } popts := ObjectOptions{ - MTime: opts.MTime, - VersionID: opts.VersionID, - UserDefined: make(map[string]string, len(objInfo.UserDefined)), + MTime: opts.MTime, + VersionID: opts.VersionID, + EvalMetadataFn: func(oi ObjectInfo) error { + if err := enforceRetentionBypassForPut(ctx, r, oi, objRetention, cred, owner); err != nil { + return err + } + if objRetention.Mode.Valid() { + oi.UserDefined[strings.ToLower(xhttp.AmzObjectLockMode)] = string(objRetention.Mode) + oi.UserDefined[strings.ToLower(xhttp.AmzObjectLockRetainUntilDate)] = objRetention.RetainUntilDate.UTC().Format(time.RFC3339) + } else { + oi.UserDefined[strings.ToLower(xhttp.AmzObjectLockMode)] = "" + oi.UserDefined[strings.ToLower(xhttp.AmzObjectLockRetainUntilDate)] = "" + } + oi.UserDefined[ReservedMetadataPrefixLower+ObjectLockRetentionTimestamp] = UTCNow().Format(time.RFC3339Nano) + dsc := mustReplicate(ctx, bucket, object, getMustReplicateOptions(oi, replication.MetadataReplicationType, opts)) + if dsc.ReplicateAny() { + oi.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) + oi.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus() + } + return nil + }, } - for k, v := range objInfo.UserDefined { - popts.UserDefined[k] = v - } - if _, err = objectAPI.PutObjectMetadata(ctx, bucket, object, popts); err != nil { + + objInfo, err := objectAPI.PutObjectMetadata(ctx, bucket, object, popts) + if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } + + dsc := mustReplicate(ctx, bucket, object, getMustReplicateOptions(objInfo, replication.MetadataReplicationType, opts)) if dsc.ReplicateAny() { scheduleReplication(ctx, objInfo.Clone(), objectAPI, dsc, replication.MetadataReplicationType) }