fix(pools): move ILM-expired current versions during decommission and rebalance

cmd/erasure-server-pool-consistency.go

@@ -130,7 +130,12 @@ func (z *erasureServerPools) checkDataMovementCopies(ctx context.Context, bucket
 // removeMovedVersions retires only the source versions in the walker's
 // snapshot. A concurrent delete can create a new marker in the source pool;
 // deleting the whole prefix would remove that marker and expose moved data.
-func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, object string, source int, versions []FileInfo) error {
+// Versions in expired were not moved because ILM deletes them. They are
+// removed from the source without a moved copy, as the queued expiry would
+// remove them. Left behind, such a version can become the latest one in the
+// source pool once the moved versions are gone, and the expiry, which
+// re-evaluates the rule there, would no longer delete it.
+func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, object string, source int, versions, expired []FileInfo) error {
 	object = encodeDirObject(object)
 	lk := z.NewNSLock(bucket, object)
 	lkctx, err := lk.GetLock(ctx, globalOperationTimeout)
@@ -168,7 +173,7 @@ func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, ob
 		}
 		// Successful movement is not enough if the destination changed again.
 		// Keep the source unless another copy still preserves its read result.
-		if len(retained) == 0 || retained[0].ObjInfo.ModTime.Before(src.ModTime) {
+		if !isExpiredVersion(expired, version) && (len(retained) == 0 || retained[0].ObjInfo.ModTime.Before(src.ModTime)) {
 			return PreConditionFailed{}
 		}
 		_, err = z.serverPools[source].DeleteObject(ctx, bucket, object, ObjectOptions{
@@ -182,6 +187,23 @@ func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, ob
 	return nil
 }
 
+// ilmDeletesVersionOnly reports whether the ILM action queued for a version
+// that data movement skipped deletes that version and nothing else, rather
+// than every version of the object or only the restored copy of a
+// transitioned version. Such a version is passed to removeMovedVersions.
+func ilmDeletesVersionOnly(action lifecycle.Action) bool {
+	return action == lifecycle.DeleteAction || action == lifecycle.DeleteVersionAction
+}
+
+func isExpiredVersion(expired []FileInfo, version FileInfo) bool {
+	for _, fi := range expired {
+		if fi.VersionID == version.VersionID && fi.ModTime.Equal(version.ModTime) && fi.Deleted == version.Deleted {
+			return true
+		}
+	}
+	return false
+}
+
 // Healing also commits object metadata. Both queued healing and drive healing
 // must use the same namespace as pooled writes, rather than racing them under
 // a destination set's independent namespace.

cmd/erasure-server-pool-decom.go

@@ -796,9 +796,11 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
 	}
 
 	for setIdx, set := range pool.sets {
-		filterLifecycle := func(bucket, object string, fi FileInfo) bool {
+		// filterLifecycle returns the action of an ILM deletion it queued
+		// for the version, which is then not moved, or NoneAction.
+		filterLifecycle := func(bucket, object string, fi FileInfo) lifecycle.Action {
 			if lc == nil {
-				return false
+				return lifecycle.NoneAction
 			}
 			versioned := vc != nil && vc.Versioned(object)
 			objInfo := fi.ToObjectInfo(bucket, object, versioned)
@@ -807,12 +809,18 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
 			switch {
 			case evt.Action.DeleteRestored(): // if restored copy has expired, delete it synchronously
 				applyExpiryOnTransitionedObject(ctx, z, objInfo, evt, lcEventSrc_Decom)
-				return false
+				return lifecycle.NoneAction
+			case evt.Action == lifecycle.DeleteAction && versioned:
+				// Expiring the latest version of a versioned bucket only puts
+				// a delete marker on top of it. The version itself survives as
+				// a noncurrent version and has to be moved like any other. The
+				// scanner applies the expiry once it lives in another pool.
+				return lifecycle.NoneAction
 			case evt.Action.Delete():
 				globalExpiryState.enqueueByDays(objInfo, evt, lcEventSrc_Decom)
-				return true
+				return evt.Action
 			default:
-				return false
+				return lifecycle.NoneAction
 			}
 		}
 
@@ -833,11 +841,18 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
 			versionsSorter(fivs.Versions).reverse()
 
 			var decommissioned int
+			var expired []FileInfo
 			for _, version := range fivs.Versions {
 				stopFn := globalDecommissionMetrics.log(decomMetricDecommissionObject, idx, bi.Name, version.Name, version.VersionID)
 				// Apply lifecycle rules on the objects that are expired.
-				if filterLifecycle(bi.Name, version.Name, version) {
+				if action := filterLifecycle(bi.Name, version.Name, version); action.Delete() {
 					decommissioned++
+					if ilmDeletesVersionOnly(action) {
+						// Its source copy goes with those of the moved
+						// versions. A version that takes the whole object
+						// with it stays until the expiry deletes them all.
+						expired = append(expired, version)
+					}
 					stopFn(version.Size, errors.New("ILM expired object/version will be skipped"))
 					continue
 				}
@@ -981,7 +996,7 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool
 			// if all versions were decommissioned, then we can delete the object versions.
 			if decommissioned == len(fivs.Versions) {
 				stopFn := globalDecommissionMetrics.log(decomMetricDecommissionRemoveObject, idx, bi.Name, entry.name)
-				err := z.removeMovedVersions(ctx, bi.Name, entry.name, idx, fivs.Versions)
+				err := z.removeMovedVersions(ctx, bi.Name, entry.name, idx, fivs.Versions, expired)
 				if err == nil {
 					err = z.removeFreeVersions(ctx, bi.Name, entry.name, idx, fivs.FreeVersions)
 				}
@@ -1199,6 +1214,9 @@ func (z *erasureServerPools) checkAfterDecom(ctx context.Context, idx int) error
 				case evt.Action.DeleteRestored(): // if restored copy has expired,delete it synchronously
 					applyExpiryOnTransitionedObject(ctx, z, objInfo, evt, lcEventSrc_Decom)
 					return false
+				case evt.Action == lifecycle.DeleteAction && versioned:
+					// The version survives as noncurrent; it must have moved.
+					return false
 				case evt.Action.Delete():
 					globalExpiryState.enqueueByDays(objInfo, evt, lcEventSrc_Decom)
 					return true

cmd/erasure-server-pool-migration-delete_test.go

@@ -132,7 +132,7 @@ func TestPoolsMigrationCleanupRechecksSource(t *testing.T) {
 				_, err := z.serverPools[1].DeleteObject(t.Context(), bucket, oi.Name, ObjectOptions{VersionID: nullVersionID})
 				mustIAM(t, err)
 			}
-			err := z.removeMovedVersions(t.Context(), bucket, oi.Name, 0, snapshot)
+			err := z.removeMovedVersions(t.Context(), bucket, oi.Name, 0, snapshot, nil)
 			if change != "delete-uuid" && err == nil {
 				t.Fatal("cleanup accepted changed or uncertain state")
 			}