diff --git a/cmd/restore-capability.go b/cmd/restore-capability.go index 311c1c6b3..ce84968da 100644 --- a/cmd/restore-capability.go +++ b/cmd/restore-capability.go @@ -28,12 +28,20 @@ import ( "github.com/minio/minio/internal/grid" ) -// A native restore keeps its multipart upload private and marks the version -// it rehydrates (restore-attempt.go). A server that predates this ignores -// the marker: it would list, change, abort or complete the upload. Native -// restore therefore runs only while every server of the cluster positively -// reports that it understands the marker. A server that does not answer, -// or does not know the question, as an older server does not, blocks it. +// Native restore is a protocol among the servers of a cluster +// (restore-stage.go): the restore lock of a version and the attempt that its +// ongoing marker names (restore-attempt.go) say which worker owns the +// restore, the copy is staged in the tmp volume and published under the +// object lock by a commit that checks the attempt again, and a superseded +// attempt writes nothing. A server that predates this protocol does not +// follow it: it would run its own restore of the same version, under other +// locks or none, and write it in its own way. +// +// Native restore therefore runs only while every server of the cluster +// positively reports that it speaks this protocol. A server that does not +// answer, or does not know the question, as an older server does not, +// blocks it: a server that is not reachable now is not proven stopped, and +// it may still be running an older restore. // nativeRestoreCapability is the native restore protocol this server speaks. const nativeRestoreCapability = "2" diff --git a/cmd/restore-stage.go b/cmd/restore-stage.go index 1bc2a443e..c1c851cad 100644 --- a/cmd/restore-stage.go +++ b/cmd/restore-stage.go @@ -554,7 +554,7 @@ func (er erasureObjects) commitRestore(ctx context.Context, bucket, object strin er.commitRenameDataDir(ctx, bucket, object, old, cleanup, 1) } if countErrs(errs, nil) < n { - er.addPartial(bucket, object, gen.fi.VersionID) + er.addPartial(bucket, object, st.versionID) } return nil, nil } diff --git a/cmd/restore-two-phase_test.go b/cmd/restore-two-phase_test.go index ba3e981f1..3c2e1b887 100644 --- a/cmd/restore-two-phase_test.go +++ b/cmd/restore-two-phase_test.go @@ -856,3 +856,64 @@ func TestRestorePublicationBelowWriteQuorumOnWideSet(t *testing.T) { } } } + +// A commit that reaches a write quorum but not every drive queues a heal of +// the version it restored: for the null version, the normalized null ID that +// the convergence path queues too. That heal repairs the null version on the +// drive that missed the commit, also where the null version is noncurrent. +func TestRestorePartialCommitQueuesNullVersionHeal(t *testing.T) { + saved := globalStorageClass + globalStorageClass.Update(storageclass.Config{Standard: storageclass.StorageClass{Parity: 2}}) + t.Cleanup(func() { globalStorageClass.Update(saved) }) + r := newRestoreSafetyTest(t, 1, versioning.Suspended) + oi, body := nativeRestoreSeed(t, r, 0, true, false) + // A version written while versioning was enabled makes the null version + // noncurrent. + newer := []byte("newer") + if _, err := r.z.PutObject(t.Context(), r.bucket, r.object, + mustGetPutObjReader(t, bytes.NewReader(newer), int64(len(newer)), "", ""), ObjectOptions{Versioned: true}); err != nil { + t.Fatal(err) + } + + failing := &restoreTwoPhaseFaults{} + failing.rename.Store("fail") + disks := restoreTwoPhaseFaulty(t, r, nil, nil, nil, failing) + // The test owns the MRF queue while the commit runs: the heal routines of + // the test servers would otherwise take the entry first. + queue := make(chan PartialOperation, mrfOpsQueueSize) + previous := globalMRFState.opCh + globalMRFState.opCh = queue + err := r.z.RestoreTransitionedObject(t.Context(), r.bucket, r.object, nativeRestoreOptions(r, oi.VersionID)) + globalMRFState.opCh = previous + if err != nil { + t.Fatalf("restore with one drive failing its commit: %v", err) + } + var queued []PartialOperation + for len(queue) > 0 { + if op := <-queue; op.Bucket == r.bucket && op.Object == r.object { + queued = append(queued, op) + } + } + if len(queued) != 1 || queued[0].VersionID != nullVersionID { + t.Fatalf("queued heals %+v, want one of version %q", queued, nullVersionID) + } + if restoreTwoPhaseRestored(restoreTwoPhaseEntries(t, r, disks, oi.VersionID)[3]) { + t.Fatal("the drive that failed the commit holds the restored copy") + } + + failing.rename.Store("") + if _, err := r.z.HealObject(t.Context(), r.bucket, r.object, queued[0].VersionID, madmin.HealOpts{ScanMode: madmin.HealNormalScan}); err != nil { + t.Fatalf("heal of the queued version: %v", err) + } + for i, fi := range restoreTwoPhaseEntries(t, r, disks, oi.VersionID) { + if !restoreTwoPhaseRestored(fi) { + t.Errorf("drive %d: not restored after the heal: %q", i, fi.Metadata[xhttp.AmzRestore]) + } + } + if _, got, err := r.read(oi.VersionID); err != nil || !bytes.Equal(got, body) { + t.Fatalf("restored contents: %v", err) + } + if latest, err := r.z.GetObjectInfo(t.Context(), r.bucket, r.object, ObjectOptions{}); err != nil || latest.VersionID == "" || latest.VersionID == nullVersionID { + t.Fatalf("latest = %q, %v; want the newer version", latest.VersionID, err) + } +}