fix: reuse unanimous latest metadata for healthy version histories

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/object-mutation-consistency_test.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"io"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"sync"
@@ -368,6 +369,9 @@ func TestMutationRollbackPreservesConcurrentVersion(t *testing.T) {
 	}
 	swap(disks)
 	t.Logf("B metadata survives on %d of eight drives after A's rollback", countB())
+	if n := countB(); n != 7 {
+		t.Fatalf("unrelated rollback removed B's acknowledged copies: got %d, want 7", n)
+	}
 	r, err := z.GetObjectNInfo(t.Context(), bucket, object, nil, nil, ObjectOptions{VersionID: bVersion})
 	if err != nil {
 		t.Fatalf("acknowledged B became unreadable after unrelated rollback: %v", err)
@@ -401,6 +405,79 @@ func TestMutationDeleteMarkerDoesNotReenter(t *testing.T) {
 	}
 }
 
+// Cleanup can arrive after an RPC reply is lost while the real disk commit is
+// still running. Both single and bulk cleanup must retain its staged metadata.
+func TestMutationStagingCleanupWaitsForCommit(t *testing.T) {
+	for _, bulk := range []bool{false, true} {
+		t.Run(strconv.FormatBool(bulk), func(t *testing.T) {
+			disk, root, err := newXLStorageTestSetup(t)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer disk.Close()
+			if err := disk.MakeVol(t.Context(), "bucket"); err != nil {
+				t.Fatal(err)
+			}
+			var lastSuccess int64
+			gate := &stagedRenameContext{
+				Context: context.Background(), meta: filepath.Join(root, minioMetaTmpBucket, "staged", xlStorageFormatFile),
+				entered: make(chan struct{}), release: make(chan struct{}),
+				tracker: healthDiskCtxValue{lastSuccess: &lastSuccess},
+			}
+			unblock := sync.OnceFunc(func() { close(gate.release) })
+			defer unblock()
+			fi := newFileInfo("object", 2, 2)
+			fi.DataDir, fi.ModTime, fi.Erasure.Index = mustGetUUID(), UTCNow(), 1
+			fi.Fresh = true
+			committed := make(chan error, 1)
+			go func() {
+				_, err := disk.RenameData(gate, minioMetaTmpBucket, "staged", fi, "bucket", "object", RenameOptions{})
+				committed <- err
+			}()
+			select {
+			case <-gate.entered:
+			case <-time.After(5 * time.Second):
+				t.Fatal("commit did not reach staging barrier")
+			}
+			cleaned := make(chan error, 1)
+			go func() {
+				if bulk {
+					cleaned <- disk.DeleteBulk(t.Context(), minioMetaTmpBucket, pathJoin("staged", xlStorageFormatFile))
+				} else {
+					cleaned <- disk.Delete(t.Context(), minioMetaTmpBucket, "staged", DeleteOptions{Recursive: true})
+				}
+			}()
+			early := false
+			select {
+			case err := <-cleaned:
+				early = true
+				t.Errorf("cleanup overtook admitted disk commit: %v", err)
+			case <-time.After(100 * time.Millisecond):
+			}
+			unblock()
+			select {
+			case err := <-committed:
+				if err != nil {
+					t.Fatal(err)
+				}
+			case <-time.After(5 * time.Second):
+				t.Fatal("commit did not finish")
+			}
+			if !early {
+				select {
+				case <-cleaned:
+				case <-time.After(5 * time.Second):
+					t.Fatal("cleanup did not finish")
+				}
+			}
+			got, err := disk.ReadVersion(t.Context(), "", "bucket", "object", "", ReadOptions{})
+			if err != nil || got.DataDir != fi.DataDir || !got.ModTime.Equal(fi.ModTime) {
+				t.Fatalf("cleanup lost committed metadata: got=%+v err=%v", got, err)
+			}
+		})
+	}
+}
+
 type reviewReadGate struct {
 	StorageAPI
 	ready   chan<- struct{}

cmd/object-mutation.go

@@ -168,15 +168,16 @@ func (s *xlStorage) undoWrite(ctx context.Context, volume, path string, fi FileI
 	return nil
 }
 
-// All drives have replied. Only a uniform indexed null version may avoid the
-// second metadata decode. Histories and conflicts retain the full merge. Exact
-// FileInfo comparison also prevents the four-byte header signature from being
-// treated as a collision-free identity.
+// All drives have replied. A uniform indexed latest version may avoid the
+// second metadata decode, including healthy version histories. Conflicts retain
+// the full merge. Exact FileInfo comparison also prevents the four-byte header
+// signature from being treated as a collision-free identity.
 func uniformLatestVersion(raw []RawFileInfo, meta []FileInfo, errs []error) bool {
 	if len(raw) == 0 {
 		return false
 	}
 	var first xlMetaV2VersionHeader
+	var firstVersions int
 	for i, r := range raw {
 		if errs[i] != nil || !meta[i].IsValid() || !sameFileInfoVersion(meta[0], meta[i]) {
 			return false
@@ -186,17 +187,18 @@ func uniformLatestVersion(raw []RawFileInfo, meta []FileInfo, errs []error) bool
 			return false
 		}
 		n, hv, mv, rest, err := decodeXLHeaders(buf)
-		if err != nil || n != 1 || mv != xlMetaVersion {
+		if err != nil || n < 1 || mv != xlMetaVersion {
 			return false
 		}
 		var h xlMetaV2VersionHeader
-		err = decodeVersions(rest, n, func(_ int, hdr, _ []byte) error { _, e := h.unmarshalV(hv, hdr); return e })
-		if err != nil || h.Type != ObjectType || h.FreeVersion() || h.VersionID != [16]byte{} {
+		err = decodeVersions(rest, 1, func(_ int, hdr, _ []byte) error { _, e := h.unmarshalV(hv, hdr); return e })
+		if err != nil || h.Type != ObjectType || h.FreeVersion() {
 			return false
 		}
 		if i == 0 {
 			first = h
-		} else if h != first {
+			firstVersions = n
+		} else if h != first || n != firstVersions || meta[i].NumVersions != meta[0].NumVersions || !meta[i].SuccessorModTime.Equal(meta[0].SuccessorModTime) {
 			return false
 		}
 	}