under rebalance look for expired versions v/s remaining versions (#17482)

A continuation of PR #17479 for rebalance behavior must
also match the decommission behavior.

Fixes bug where rebalance would ignore rebalancing object
versions after one of the version returned "ObjectNotFound"
This commit is contained in:
Harshavardhana 2023-06-21 13:23:20 -07:00 committed by GitHub
parent 021372cc4c
commit 9af6c6ceef
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 21 deletions

View File

@ -722,20 +722,21 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
// to create the appropriate stack. // to create the appropriate stack.
versionsSorter(fivs.Versions).reverse() versionsSorter(fivs.Versions).reverse()
var decommissionedCount int var decommissioned, expired int
for _, version := range fivs.Versions { for _, version := range fivs.Versions {
// Apply lifecycle rules on the objects that are expired. // Apply lifecycle rules on the objects that are expired.
if filterLifecycle(bi.Name, version.Name, version) { if filterLifecycle(bi.Name, version.Name, version) {
decommissionedCount++ expired++
decommissioned++
continue continue
} }
// any object with only single DEL marker we don't need // any object with only single DEL marker we don't need
// to decommission, just skip it, this also includes // to decommission, just skip it, this also includes
// any other versions that have already expired. // any other versions that have already expired.
remainingVersions := len(fivs.Versions) - decommissionedCount remainingVersions := len(fivs.Versions) - expired
if version.Deleted && remainingVersions == 1 { if version.Deleted && remainingVersions == 1 {
decommissionedCount++ decommissioned++
continue continue
} }
@ -760,11 +761,7 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
SkipDecommissioned: true, // make sure we skip the decommissioned pool SkipDecommissioned: true, // make sure we skip the decommissioned pool
}) })
var failure bool var failure bool
if err != nil { if err != nil && !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
// object deleted by the application, nothing to do here we move on.
continue
}
logger.LogIf(ctx, err) logger.LogIf(ctx, err)
failure = true failure = true
} }
@ -773,7 +770,7 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
z.poolMetaMutex.Unlock() z.poolMetaMutex.Unlock()
if !failure { if !failure {
// Success keep a count. // Success keep a count.
decommissionedCount++ decommissioned++
} }
continue continue
} }
@ -848,11 +845,11 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
if failure { if failure {
break // break out on first error break // break out on first error
} }
decommissionedCount++ decommissioned++
} }
// if all versions were decommissioned, then we can delete the object versions. // if all versions were decommissioned, then we can delete the object versions.
if decommissionedCount == len(fivs.Versions) { if decommissioned == len(fivs.Versions) {
stopFn := globalDecommissionMetrics.log(decomMetricDecommissionRemoveObject, idx, bi.Name, entry.name) stopFn := globalDecommissionMetrics.log(decomMetricDecommissionRemoveObject, idx, bi.Name, entry.name)
_, err := set.DeleteObject(ctx, _, err := set.DeleteObject(ctx,
bi.Name, bi.Name,

View File

@ -500,7 +500,7 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string,
// to create the appropriate stack. // to create the appropriate stack.
versionsSorter(fivs.Versions).reverse() versionsSorter(fivs.Versions).reverse()
var rebalanced int var rebalanced, expired int
for _, version := range fivs.Versions { for _, version := range fivs.Versions {
// Skip transitioned objects for now. TBD // Skip transitioned objects for now. TBD
if version.IsRemote() { if version.IsRemote() {
@ -510,12 +510,15 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string,
// Apply lifecycle rules on the objects that are expired. // Apply lifecycle rules on the objects that are expired.
if filterLifecycle(bucket, version.Name, version) { if filterLifecycle(bucket, version.Name, version) {
rebalanced++ rebalanced++
expired++
continue continue
} }
// Empty delete markers we can assume that they can be purged // any object with only single DEL marker we don't need
// as there is no more data associated with them. // to rebalance, just skip it, this also includes
if version.Deleted && len(fivs.Versions) == 1 { // any other versions that have already expired.
remainingVersions := len(fivs.Versions) - expired
if version.Deleted && remainingVersions == 1 {
rebalanced++ rebalanced++
continue continue
} }
@ -550,9 +553,10 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string,
continue continue
} }
var failure bool var failure, ignore bool
for try := 0; try < 3; try++ { for try := 0; try < 3; try++ {
// GetObjectReader.Close is called by rebalanceObject // GetObjectReader.Close is called by rebalanceObject
stopFn := globalRebalanceMetrics.log(rebalanceMetricRebalanceObject, poolIdx, bucket, version.Name, version.VersionID)
gr, err := set.GetObjectNInfo(ctx, gr, err := set.GetObjectNInfo(ctx,
bucket, bucket,
encodeDirObject(version.Name), encodeDirObject(version.Name),
@ -565,19 +569,21 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string,
}) })
if isErrObjectNotFound(err) || isErrVersionNotFound(err) { if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
// object deleted by the application, nothing to do here we move on. // object deleted by the application, nothing to do here we move on.
return ignore = true
stopFn(nil)
break
} }
if err != nil { if err != nil {
failure = true failure = true
logger.LogIf(ctx, err) logger.LogIf(ctx, err)
stopFn(err)
continue continue
} }
stopFn := globalRebalanceMetrics.log(rebalanceMetricRebalanceObject, poolIdx, bucket, version.Name)
if err = z.rebalanceObject(ctx, bucket, gr); err != nil { if err = z.rebalanceObject(ctx, bucket, gr); err != nil {
stopFn(err)
failure = true failure = true
logger.LogIf(ctx, err) logger.LogIf(ctx, err)
stopFn(err)
continue continue
} }
@ -585,7 +591,9 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string,
failure = false failure = false
break break
} }
if ignore {
continue
}
if failure { if failure {
break // break out on first error break // break out on first error
} }