From 0720ed477e49f6d71ea5d4f218319527e60c2bf9 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sun, 6 Sep 2026 21:52:54 +0800 Subject: [PATCH] fix(replication): scope resync dispatch to the target being resynced The resync worker pool runs for a single target (opts.arn), but the dispatch loop admitted any object whose ExistingObjResync.mustResync() was true for ANY target. On a bucket with per-target rules (A and B), a resync of A would pull in objects that only qualify for B - even with a single active resync, since qualification is any-target. After the outcome-based classification (previous change) such a cross-target object leaves A absent from its per-object result and is counted as an A failure - an object A was never responsible for. Scope admission to the resync's own target: dispatch an object only if it must resync for opts.arn specifically (mustResyncTarget), via a small pure helper objectNeedsResyncForARN. Only opts.arn carries this resync's ResetID, and that reset is already folded into its per-target decision, so the per-target check both scopes dispatch and honors the reset. Each target has its own resyncBucket, so no cross-target object is dropped - it is handled by that target's resync. The classifier's absent-ARN failure path is now unreachable for normally dispatched objects and remains only as defense-in-depth (e.g. a config/lock error before replication is attempted). Delete-marker/version-purge handling, the null-version exclusion, the finalization ordering and the outcome-based classification are unchanged. Adds a table-driven regression for the predicate: with A/B rules and a resync of A, an object qualifying only for B is not admitted; any-target scoping admits it and fails the test. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe Signed-off-by: Feng Ruohang --- cmd/bucket-replication.go | 20 ++++++++++- cmd/bucket-replication_test.go | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index 7f52b06ac..1b8c846ad 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -3029,6 +3029,19 @@ func resyncResultFor(rinfos replicatedInfos, arn string, roi ReplicateObjectInfo return st } +// objectNeedsResyncForARN reports whether roi must be resynced for target arn +// specifically. The resync worker pool is scoped to a single target (opts.arn), +// so an object that only qualifies for a different target must be skipped here: +// admitting it would replicate it for arn's peers only, leaving arn absent from +// the per-object result, which resyncResultFor then (correctly, but +// misleadingly) counts as a failure for arn - an object arn was never +// responsible for. Only opts.arn carries this resync's ResetID, and that reset +// is already folded into its per-target decision, so the per-target check both +// scopes dispatch and honors the reset. +func objectNeedsResyncForARN(roi ReplicateObjectInfo, arn string) bool { + return roi.ExistingObjResync.mustResyncTarget(arn) +} + // resyncBucket resyncs all qualifying objects as per replication rules for the target // ARN func (s *replicationResyncer) resyncBucket(ctx context.Context, objectAPI ObjectLayer, heal bool, opts resyncOpts) { @@ -3196,7 +3209,12 @@ func (s *replicationResyncer) resyncBucket(ctx context.Context, objectAPI Object } lastCheckpoint = "" roi := getHealReplicateObjectInfo(res.Item, rcfg) - if !roi.ExistingObjResync.mustResync() { + // Scope dispatch to this resync's target: the worker pool is for + // opts.arn, so skip objects that only need resync for a different + // target (each target has its own resync). Without this, a cross-target + // object leaves opts.arn absent from its per-object result and is + // miscounted as an opts.arn failure. + if !objectNeedsResyncForARN(roi, opts.arn) { continue } select { diff --git a/cmd/bucket-replication_test.go b/cmd/bucket-replication_test.go index 8e2bb102c..7d9082791 100644 --- a/cmd/bucket-replication_test.go +++ b/cmd/bucket-replication_test.go @@ -647,3 +647,67 @@ func TestResyncResultFor(t *testing.T) { }) } } + +// TestObjectNeedsResyncForARN asserts the resync dispatch is scoped to the +// target being resynced. The worker pool runs for a single target (opts.arn), +// so an object that only qualifies for a different target must be skipped: with +// A/B rules and a resync of A, an object that needs replication only for B must +// not be admitted to A's worker. Otherwise (after outcome-based classification) +// A would be absent from that object's result and miscounted as an A failure. +func TestObjectNeedsResyncForARN(t *testing.T) { + const ( + arnA = "arn:minio:replication::id:bucket" + arnB = "arn:minio:replication::id2:bucket" + ) + tests := []struct { + name string + decision ResyncDecision + arn string + want bool + }{ + { + name: "target must resync", + decision: ResyncDecision{targets: map[string]ResyncTargetDecision{arnA: {Replicate: true}}}, + arn: arnA, + want: true, + }, + { + name: "object qualifies for B only, resyncing A", + decision: ResyncDecision{targets: map[string]ResyncTargetDecision{arnB: {Replicate: true}}}, + arn: arnA, + want: false, + }, + { + name: "A present but not replicating, B replicating, resyncing A", + decision: ResyncDecision{targets: map[string]ResyncTargetDecision{ + arnA: {Replicate: false}, + arnB: {Replicate: true}, + }}, + arn: arnA, + want: false, + }, + { + name: "object qualifies for both, resyncing A", + decision: ResyncDecision{targets: map[string]ResyncTargetDecision{ + arnA: {Replicate: true}, + arnB: {Replicate: true}, + }}, + arn: arnA, + want: true, + }, + { + name: "no resync decision", + decision: ResyncDecision{}, + arn: arnA, + want: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + roi := ReplicateObjectInfo{Name: "obj", Bucket: "bucket", ExistingObjResync: tc.decision} + if got := objectNeedsResyncForARN(roi, tc.arn); got != tc.want { + t.Fatalf("objectNeedsResyncForARN(arn=%s) = %v, want %v", tc.arn, got, tc.want) + } + }) + } +}