Compare commits

..

9 Commits

Author SHA1 Message Date
Zhengchao An 7e071dc0fa test(site-replication): cover retry endpoint matching 2026-08-16 06:17:58 +08:00
Zhengchao An e117a5d09f test(site-replication): preserve collapsed retry liability 2026-08-16 06:14:35 +08:00
Zhengchao An a26e97bbff fix(site-replication): gate test-only retry helper 2026-08-16 05:34:49 +08:00
overtrue 96a2f9c67f fix(site-replication): preserve retry accountability 2026-08-16 04:54:11 +08:00
唐小鸭 af802756c5 fix(site-replication): only a repair settles snapshot-escalated retry entries
Second review round: every iam-item / bucket-meta delivery shares a
constant path, so any later successful single-item delivery (a Bob
update) dequeued the escalated marker recording a possibly-unreplayed
deletion (a failed Alice delete) while the entity still existed
remotely. Ordinary settlement now skips escalated entries; only the
repair path — the operator's explicit accountability transfer — clears
them via dequeue_..._including_escalated. A new hook failure still
overwrites the marker and re-arms the drain. Regression covers
survive-ordinary-dequeue and repair-clears.
2026-08-16 01:02:42 +08:00
唐小鸭 971addca6e fix(site-replication): escalate snapshot-replayed retry entries instead of clearing them
Review: the bootstrap-plan snapshot cannot replay deletions — a deleted
IAM entity or absent bucket config produces no task, so clearing the
collapsed iam-item / bucket-meta entry after a successful snapshot
resend silently lost a failed delete and the peer kept stale state
permanently.

The drain now keeps those entries until remote absence is proven:
after a successful snapshot resend the entry is escalated
(failed=true, marker last_error) so it stays operator-visible in
retry_stats, and classification skips marked entries so the
once-per-failure-episode snapshot is not re-sent daily. A newer hook
failure overwrites the marker and re-arms the drain; a later full
delivery or a manual repair settles the entry. Escalation is
conditional on the snapshot timestamp, preserving the earlier
review's in-flight-failure guarantee. Bucket ops and peer edits keep
auto-settle — their replays are faithful.
2026-08-15 19:14:23 +08:00
唐小鸭 2c7d1f1f9f fix(site-replication): harden the retry drain against review findings
Adversarial review of the drain surfaced one real race and three cheap
hardenings:

- Conditional settlement for collapsed (constant-path) iam-item /
  bucket-meta entries: the snapshot resend proves delivery of the state
  as of plan-build time, so a hook failure stamped during the delivery
  window (a newer local commit the snapshot did not contain) must
  survive the snapshot's success instead of being cleared — previously
  the peer would silently diverge until the next same-path failure.
  (The operator repair path shares this collapse but keeps its existing
  unconditional settle; the drain runs every tick and needed the guard
  first.)
- Re-check the pending_* gates on the freshly loaded state: an endpoint
  refresh can commit its pending marker mid-tick without the lifecycle
  guard.
- Do not upsert retry events for peers that already left the state —
  remove_sites pruned their entries and they can never drain again.
- Correct the mutual-exclusion comment: the repair-execution lock does
  NOT close the dry-run -> execute window (dry-run takes no lock); that
  window fails safe via the preflight token, which hashes the
  replayable entries.

New tests pin the (Some settled, None failed) generation-settle
combination the peer-edit drain depends on, and the snapshot-relative
conditional settlement.
2026-08-15 11:01:58 +08:00
唐小鸭 5328e8b958 feat(site-replication): drain the retry queue from the reconcile tick
The retry queue recorded every failed peer delivery but had no
consumer — missed IAM/bucket metadata updates drifted until an operator
ran a manual repair (backlog#1675 P1-3). The 600s reconcile tick now
drains the queue behind the existing lifecycle guard and pending_*
gates.

Replay discipline (pinned by the red-light tests):
- IAM / bucket-meta entries collapse per (peer, path) and their bodies
  are not persisted, so the only faithful replay is the current
  bootstrap-plan snapshot (SiteReplicationRepairTask reuse) sent to the
  affected peer.
- make-with-versioning / configure-replication ops are re-derived from
  the CURRENT plan for their bucket — never the recorded path, whose
  query can carry an expired one-shot bootstrap token; an op whose
  bucket left the plan is provably stale and is settled.
- Peer edits are re-sent as the current peer records under a freshly
  allocated generation inside one state transaction — the recorded
  generation is stale by definition and the receiver would fence it.
- Destructive bucket ops (delete-bucket / force-delete-bucket) and
  internal: marker records (pending-endpoint-refresh backup store) are
  never background-replayed.
- Exponential backoff (600s * 2^(n-1), 24h ceiling) gates each attempt;
  an unreachable peer's transport failure re-queues its events so the
  backoff still advances.

Concurrency: the drain takes the repair-execution config lock — the
operator repair preflight token hashes the replayable retry events, so
settling them between dry-run and execute would strand the operator on
a stale preflight. Lock order matches repair (lifecycle guard -> repair
execution lock -> state object lock); success/failure settlement reuses
the generation-fenced upsert/settle semantics from #5882/#6097.

Scope note: the drain settles 'delivered once and failed' entries. A
hook that never fired (crash between local commit and send) leaves no
entry; a low-frequency plan-diff catch-all remains follow-up work, and
manual repair stays authoritative.
2026-08-15 10:42:36 +08:00
唐小鸭 1e16e06f8a test(site-replication): pin the background retry-drain discipline
Red-light evidence for backlog#1675 P1-3: the retry queue has no
background consumer — every failed peer delivery waits for a manual
repair. The new tests specify the drain rules before the drain exists:

- classification: IAM / bucket-meta entries collapse per (peer, path)
  with no persisted body, so only a bootstrap-plan snapshot resend is a
  faithful replay; make-with-versioning / configure-replication are
  re-derivable per bucket; peer edits are re-sent under a fresh
  generation; destructive bucket ops and internal: marker records
  (pending-endpoint-refresh backup store) are never background-replayed
- exponential backoff (600s * 2^(n-1), 24h ceiling) gates every
  attempt, otherwise a dead peer's entries hit the failed threshold
  within 30 minutes of reconcile ticks
- the actionable subset respects classification, current peer
  membership and backoff

All fail against the placeholder implementations (no consumer).
2026-08-15 10:19:28 +08:00
3 changed files with 1056 additions and 68 deletions
+4 -52
View File
@@ -40,14 +40,7 @@ impl ARN {
impl Display for ARN {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// The `minio` partition is deliberate: madmin-go's ParseARN
// hard-rejects any other partition, so native mc/madmin tooling can
// only decode remote-target ARNs minted in this form (backlog#1675
// P1-7). Legacy `arn:rustfs:` ARNs persisted by older releases stay
// readable via the FromStr whitelist below; runtime matching between
// targets and replication rules is by full-string equality, so mixed
// partitions coexist safely.
write!(f, "arn:minio:{}:{}:{}:{}", self.arn_type, self.region, self.id, self.bucket)
write!(f, "arn:rustfs:{}:{}:{}:{}", self.arn_type, self.region, self.id, self.bucket)
}
}
@@ -55,12 +48,7 @@ impl FromStr for ARN {
type Err = std::io::Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
// Partition whitelist, not just an `arn:` check: `BucketTargetType::
// from_str(...).unwrap_or_default()` below never fails, so this is
// the only structural gate rejecting foreign ARNs. `arn:rustfs:` is
// the legacy partition and must stay accepted forever (persisted
// bucket-targets.json / replication configs from older releases).
if !s.starts_with("arn:minio:") && !s.starts_with("arn:rustfs:") {
if !s.starts_with("arn:rustfs:") {
return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "Invalid ARN format"));
}
@@ -113,50 +101,14 @@ mod tests {
}
/// RustFS commonly generates ARNs with an empty region:
/// `arn:minio:replication::<deployment_id>:<bucket>`.
/// `arn:rustfs:replication::<deployment_id>:<bucket>`.
#[test]
fn from_str_handles_empty_region_segment() {
let parsed = ARN::from_str("arn:minio:replication::depl-123:bucket-a").expect("valid ARN must parse");
let parsed = ARN::from_str("arn:rustfs:replication::depl-123:bucket-a").expect("valid ARN must parse");
assert_eq!(parsed.arn_type, BucketTargetType::ReplicationService);
assert_eq!(parsed.region, "", "region segment is empty in this form");
assert_eq!(parsed.id, "depl-123");
assert_eq!(parsed.bucket, "bucket-a");
}
/// madmin-go's `ParseARN` hard-rejects anything that does not start with
/// `arn:minio:`, so generated ARNs must use the `minio` partition or the
/// native mc/madmin tooling cannot decode remote-target listings.
#[test]
fn display_emits_minio_partition() {
let arn = ARN::new(
BucketTargetType::ReplicationService,
"depl-123".to_string(),
String::new(),
"bucket-a".to_string(),
);
assert_eq!(arn.to_string(), "arn:minio:replication::depl-123:bucket-a");
}
/// Persisted bucket-targets.json files from older RustFS releases carry
/// `arn:rustfs:` ARNs; the legacy partition must stay parseable forever.
#[test]
fn from_str_accepts_legacy_rustfs_partition() {
let parsed = ARN::from_str("arn:rustfs:replication:us-east-1:depl-123:bucket-a").expect("legacy ARN must parse");
assert_eq!(parsed.arn_type, BucketTargetType::ReplicationService);
assert_eq!(parsed.region, "us-east-1");
assert_eq!(parsed.id, "depl-123");
assert_eq!(parsed.bucket, "bucket-a");
}
/// The partition whitelist is the only structural gate: `BucketTargetType::
/// from_str(...).unwrap_or_default()` never fails, so any 6-segment string
/// would otherwise parse as `type=None`.
#[test]
fn from_str_rejects_unknown_partition() {
assert!(ARN::from_str("arn:aws:replication::depl-123:bucket-a").is_err());
assert!(ARN::from_str("not-an-arn").is_err());
}
}
+3 -3
View File
@@ -258,7 +258,7 @@ pub struct SRLDAPUser {
pub api_version: Option<String>,
}
#[derive(Debug, Serialize, Deserialize, Default)]
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct SRIAMUser {
#[serde(rename = "accessKey", default)]
pub access_key: String,
@@ -270,7 +270,7 @@ pub struct SRIAMUser {
pub api_version: Option<String>,
}
#[derive(Debug, Serialize, Deserialize, Default)]
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct SRGroupInfo {
#[serde(rename = "updateReq", default)]
pub update_req: GroupAddRemove,
@@ -346,7 +346,7 @@ pub struct SRCredInfo {
pub api_version: Option<String>,
}
#[derive(Debug, Serialize, Deserialize, Default)]
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct SRIAMItem {
#[serde(default)]
pub r#type: String,
File diff suppressed because it is too large Load Diff