From 9d80ff5a053142c258e6ef50429888d16c4f53ba Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 5 Jul 2022 07:37:24 -0700 Subject: [PATCH] fix: decommission delete markers for non-current objects (#15225) versioned buckets were not creating the delete markers present in the versioned stack of an object, this essentially would stop decommission to succeed. This PR fixes creating such delete markers properly during a decommissioning process, adds tests as well. --- cmd/erasure-server-pool-decom.go | 7 ++++--- cmd/erasure-server-pool.go | 10 +++++----- docs/distributed/decom.sh | 18 +++++++++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/cmd/erasure-server-pool-decom.go b/cmd/erasure-server-pool-decom.go index 7ba734640..bc0219f3c 100644 --- a/cmd/erasure-server-pool-decom.go +++ b/cmd/erasure-server-pool-decom.go @@ -689,6 +689,7 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool VersionID: version.VersionID, MTime: version.ModTime, DeleteReplication: version.ReplicationState, + DeleteMarker: true, // make sure we create a delete marker }) var failure bool if err != nil { @@ -698,10 +699,10 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool z.poolMetaMutex.Lock() z.poolMeta.CountItem(idx, 0, failure) z.poolMetaMutex.Unlock() - if failure { - break // break out on first error + if !failure { + // Success keep a count. + decommissionedCount++ } - decommissionedCount++ continue } diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index 475706222..ec4b71fc7 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -119,7 +119,7 @@ func newErasureServerPools(ctx context.Context, endpointServerPools EndpointServ } if deploymentID == "" { - // all zones should have same deployment ID + // all pools should have same deployment ID deploymentID = formats[i].ID } @@ -457,7 +457,7 @@ func (z *erasureServerPools) getPoolIdxExistingWithOpts(ctx context.Context, buc // If the object exists, but the latest version is a delete marker, the index with it is still returned. // If the object does not exist ObjectNotFound error is returned. // If any other error is found, it is returned. -// The check is skipped if there is only one zone, and 0, nil is always returned in that case. +// The check is skipped if there is only one pool, and 0, nil is always returned in that case. func (z *erasureServerPools) getPoolIdxExistingNoLock(ctx context.Context, bucket, object string) (idx int, err error) { return z.getPoolIdxExistingWithOpts(ctx, bucket, object, ObjectOptions{ NoLock: true, @@ -881,7 +881,7 @@ func (z *erasureServerPools) getLatestObjectInfoWithIdx(ctx context.Context, buc sort.Slice(results, func(i, j int) bool { a, b := results[i], results[j] if a.oi.ModTime.Equal(b.oi.ModTime) { - // On tiebreak, select the lowest zone index. + // On tiebreak, select the lowest pool index. return a.zIdx < b.zIdx } return a.oi.ModTime.After(b.oi.ModTime) @@ -975,12 +975,12 @@ func (z *erasureServerPools) PutObject(ctx context.Context, bucket string, objec } func (z *erasureServerPools) deletePrefix(ctx context.Context, bucket string, prefix string) error { - for idx, zone := range z.serverPools { + for idx, pool := range z.serverPools { if z.IsSuspended(idx) { logger.LogIf(ctx, fmt.Errorf("pool %d is suspended, all writes are suspended", idx+1)) continue } - _, err := zone.DeleteObject(ctx, bucket, prefix, ObjectOptions{DeletePrefix: true}) + _, err := pool.DeleteObject(ctx, bucket, prefix, ObjectOptions{DeletePrefix: true}) if err != nil { return err } diff --git a/docs/distributed/decom.sh b/docs/distributed/decom.sh index 5618524d7..97ff00594 100755 --- a/docs/distributed/decom.sh +++ b/docs/distributed/decom.sh @@ -28,13 +28,20 @@ export MC_HOST_myminio="http://minioadmin:minioadmin@localhost:9000/" ./mc admin policy set myminio/ lake,rw user=minio12345 ./mc mb -l myminio/versioned + +./mc mirror internal myminio/versioned/ --quiet >/dev/null + +## Soft delete (creates delete markers) +./mc rm -r --force myminio/versioned >/dev/null + +## mirror again to create another set of version on top ./mc mirror internal myminio/versioned/ --quiet >/dev/null user_count=$(./mc admin user list myminio/ | wc -l) policy_count=$(./mc admin policy list myminio/ | wc -l) kill $pid -(minio server /tmp/xl/{1...10}/disk{0...1} /tmp/xl/{11...30}/disk{0...3} 2>&1 >/dev/null) & +(minio server /tmp/xl/{1...10}/disk{0...1} /tmp/xl/{11...30}/disk{0...3} 2>&1 >/tmp/expanded.log) & pid=$! sleep 2 @@ -60,7 +67,9 @@ if [ $ret -ne 0 ]; then fi ./mc mirror cmd myminio/versioned/ --quiet >/dev/null + ./mc ls -r myminio/versioned/ > expanded_ns.txt +./mc ls -r --versions myminio/versioned/ > expanded_ns_versions.txt ./mc admin decom start myminio/ /tmp/xl/{1...10}/disk{0...1} @@ -98,6 +107,7 @@ if [ $ret -ne 0 ]; then fi ./mc ls -r myminio/versioned > decommissioned_ns.txt +./mc ls -r --versions myminio/versioned > decommissioned_ns_versions.txt out=$(diff -qpruN expanded_ns.txt decommissioned_ns.txt) ret=$? @@ -105,4 +115,10 @@ if [ $ret -ne 0 ]; then echo "BUG: expected no missing entries after decommission: $out" fi +out=$(diff -qpruN expanded_ns_versions.txt decommissioned_ns_versions.txt) +ret=$? +if [ $ret -ne 0 ]; then + echo "BUG: expected no missing entries after decommission: $out" +fi + kill $pid