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

@@ -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:

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

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,
+			MTime:     objInfo.ModTime,
-			VersionID:   objInfo.VersionID,
+			VersionID: objInfo.VersionID,
-			UserDefined: make(map[string]string, len(objInfo.UserDefined)),
+			EvalMetadataFn: func(oi ObjectInfo) error {
-		}
+				oi.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = newReplStatusInternal
-		for k, v := range objInfo.UserDefined {
+				oi.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano)
-			popts.UserDefined[k] = v
+				oi.UserDefined[xhttp.AmzBucketReplicationStatus] = string(rinfos.ReplicationStatus())
-		}
+				for _, rinfo := range rinfos.Targets {
-		popts.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = newReplStatusInternal
+					if rinfo.ResyncTimestamp != "" {
-		popts.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano)
+						oi.UserDefined[targetResetHeader(rinfo.Arn)] = rinfo.ResyncTimestamp
-		popts.UserDefined[xhttp.AmzBucketReplicationStatus] = string(rinfos.ReplicationStatus())
+					}
-		for _, rinfo := range rinfos.Targets {
+				}
-			if rinfo.ResyncTimestamp != "" {
+				if objInfo.UserTags != "" {
-				popts.UserDefined[targetResetHeader(rinfo.Arn)] = rinfo.ResyncTimestamp
+					oi.UserDefined[xhttp.AmzObjectTagging] = objInfo.UserTags
-			}
+				}
-		}
+				return nil
-		if objInfo.UserTags != "" {
+			},
-			popts.UserDefined[xhttp.AmzObjectTagging] = objInfo.UserTags
 		}
+
 		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))

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 fi.ToObjectInfo(bucket, object), nil
-	return objInfo, nil
-
 }
 
 // PutObjectTags - replace or add tags to an existing object

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
 

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

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,
+		MTime:     opts.MTime,
-		VersionID:   opts.VersionID,
+		VersionID: opts.VersionID,
-		UserDefined: make(map[string]string, len(objInfo.UserDefined)),
+		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
+	objInfo, err := objectAPI.PutObjectMetadata(ctx, bucket, object, popts)
-	}
+	if err != nil {
-	if _, err = objectAPI.PutObjectMetadata(ctx, bucket, object, popts); 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)
 	}