From 1a884cd8e12ebe87c4e7475e4c4991c6ad7f2910 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 17 Sep 2021 19:34:48 -0700 Subject: [PATCH] fix: deleting objects was not working after upgrades (#13242) DeleteObject() on existing objects before `xl.json` to `xl.meta` change were not working, not sure when this regression was added. This PR fixes this properly. Also this PR ensures that we perform rename of xl.json to xl.meta only during "write" phase of the call i.e either during Healing or PutObject() overwrites. Also handles few other scenarios during migration where `backendEncryptedFile` was missing deleteConfig() will fail with `configNotFound` this case was not ignored, which can lead to failure during upgrades. --- cmd/config-common.go | 4 +++- cmd/config.go | 4 +++- cmd/erasure-sets.go | 12 ++++++++---- cmd/xl-storage.go | 20 +++++++------------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/cmd/config-common.go b/cmd/config-common.go index d0465833f..1afff3baf 100644 --- a/cmd/config-common.go +++ b/cmd/config-common.go @@ -57,7 +57,9 @@ type objectDeleter interface { } func deleteConfig(ctx context.Context, objAPI objectDeleter, configFile string) error { - _, err := objAPI.DeleteObject(ctx, minioMetaBucket, configFile, ObjectOptions{}) + _, err := objAPI.DeleteObject(ctx, minioMetaBucket, configFile, ObjectOptions{ + DeletePrefix: true, + }) if err != nil && isErrObjectNotFound(err) { return errConfigNotFound } diff --git a/cmd/config.go b/cmd/config.go index c5401af6b..2929a9685 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -96,7 +96,9 @@ func listServerConfigHistory(ctx context.Context, objAPI ObjectLayer, withData b func delServerConfigHistory(ctx context.Context, objAPI ObjectLayer, uuidKV string) error { historyFile := pathJoin(minioConfigHistoryPrefix, uuidKV+kvPrefix) - _, err := objAPI.DeleteObject(ctx, minioMetaBucket, historyFile, ObjectOptions{}) + _, err := objAPI.DeleteObject(ctx, minioMetaBucket, historyFile, ObjectOptions{ + DeletePrefix: true, + }) return err } diff --git a/cmd/erasure-sets.go b/cmd/erasure-sets.go index 25c6b3eef..d2a2c4a5c 100644 --- a/cmd/erasure-sets.go +++ b/cmd/erasure-sets.go @@ -893,12 +893,16 @@ func (s *erasureSets) GetObjectInfo(ctx context.Context, bucket, object string, } func (s *erasureSets) deletePrefix(ctx context.Context, bucket string, prefix string) error { + var wg sync.WaitGroup + wg.Add(len(s.sets)) for _, s := range s.sets { - _, err := s.DeleteObject(ctx, bucket, prefix, ObjectOptions{DeletePrefix: true}) - if err != nil { - return err - } + go func(s *erasureObjects) { + defer wg.Done() + // This is a force delete, no reason to throw errors. + s.DeleteObject(ctx, bucket, prefix, ObjectOptions{DeletePrefix: true}) + }(s) } + wg.Wait() return nil } diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 2a1e867c2..70a0c055e 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -857,10 +857,13 @@ func (s *xlStorage) DeleteVersion(ctx context.Context, volume, path string, fi F // Create a new xl.meta with a delete marker in it return s.WriteMetadata(ctx, volume, path, fi) } - if fi.VersionID != "" { - return errFileVersionNotFound + buf, err = s.ReadAll(ctx, volume, pathJoin(path, xlStorageFormatFileV1)) + if err != nil { + if err == errFileNotFound && fi.VersionID != "" { + return errFileVersionNotFound + } + return err } - return errFileNotFound } if len(buf) == 0 { @@ -1111,16 +1114,7 @@ func (s *xlStorage) ReadVersion(ctx context.Context, volume, path, versionID str if err != nil { if err == errFileNotFound { - if err = s.renameLegacyMetadata(volumeDir, path); err != nil { - if err == errFileNotFound { - if versionID != "" { - return fi, errFileVersionNotFound - } - return fi, errFileNotFound - } - return fi, err - } - buf, err = s.ReadAll(ctx, volume, pathJoin(path, xlStorageFormatFile)) + buf, err = s.ReadAll(ctx, volume, pathJoin(path, xlStorageFormatFileV1)) if err != nil { if err == errFileNotFound { if versionID != "" {