From 36990aeafd7b8d1846e5f66292eacde60c36f0fb Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 30 Oct 2018 16:07:57 -0700 Subject: [PATCH] Avoid double bucket validation in DeleteObjectHandler (#6720) On a heavily loaded server, getBucketInfo() becomes slow, one can easily observe deleting an object causes many additional network calls. This PR is to let the underlying call return the actual error and write it back to the client. --- cmd/object-handlers.go | 23 ++++++++++------------- cmd/xl-v1-object.go | 8 ++++---- cmd/xl-v1-utils.go | 2 +- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 92f2b406c..9b6895059 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -2091,21 +2091,18 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. } return } - } else { - getBucketInfo := objectAPI.GetBucketInfo - if api.CacheAPI() != nil { - getBucketInfo = api.CacheAPI().GetBucketInfo - } - if _, err := getBucketInfo(ctx, bucket); err != nil { - writeErrorResponse(w, toAPIErrorCode(err), r.URL) - return - } } // http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectDELETE.html - // Ignore delete object errors while replying to client, since we are - // suppposed to reply only 204. Additionally log the error for - // investigation. - deleteObject(ctx, objectAPI, api.CacheAPI(), bucket, object, r) + if err := deleteObject(ctx, objectAPI, api.CacheAPI(), bucket, object, r); err != nil { + switch toAPIErrorCode(err) { + case ErrNoSuchBucket: + // When bucket doesn't exist specially handle it. + writeErrorResponse(w, ErrNoSuchBucket, r.URL) + return + } + logger.LogIf(ctx, err) + // Ignore delete object errors while replying to client, since we are suppposed to reply only 204. + } writeSuccessNoContent(w) } diff --git a/cmd/xl-v1-object.go b/cmd/xl-v1-object.go index a6c4bf277..4086f0eba 100644 --- a/cmd/xl-v1-object.go +++ b/cmd/xl-v1-object.go @@ -984,13 +984,13 @@ func (xl xlObjects) DeleteObject(ctx context.Context, bucket, object string) (er return err } - if !xl.isObject(bucket, object) && !xl.isObjectDir(bucket, object) { - return ObjectNotFound{bucket, object} - } - var writeQuorum int var isObjectDir = hasSuffix(object, slashSeparator) + if isObjectDir && !xl.isObjectDir(bucket, object) { + return toObjectErr(errFileNotFound, bucket, object) + } + if isObjectDir { writeQuorum = len(xl.getDisks())/2 + 1 } else { diff --git a/cmd/xl-v1-utils.go b/cmd/xl-v1-utils.go index c8aa3af3d..83b8f3664 100644 --- a/cmd/xl-v1-utils.go +++ b/cmd/xl-v1-utils.go @@ -305,7 +305,7 @@ func readXLMeta(ctx context.Context, disk StorageAPI, bucket string, object stri // Reads entire `xl.json`. xlMetaBuf, err := disk.ReadAll(bucket, path.Join(object, xlMetaJSONFile)) if err != nil { - if err != errFileNotFound { + if err != errFileNotFound && err != errVolumeNotFound { logger.GetReqInfo(ctx).AppendTags("disk", disk.String()) logger.LogIf(ctx, err) }