fix(replication): probe the version-identity contract in replication-check (#5881)

* test(replication): pin the version-fidelity probe contract (red)

P1-19 (rustfs/backlog#1675 B2): the supported replication contract is
targets that adopt the source version id — a target that mints its own ids
silently breaks every version-addressed operation that follows (version
deletes, heal re-drives never match), diverging the two sides with no
signal. replication-check already captures the probe PUT's response version
id but never compares it.

Red evidence (current main): against a FakeS3Target with
assign_own_version_ids enabled, ?replication-check returns Status "OK" —
the drift is invisible.

test_replication_check_flags_version_minting_target expects a
VersionFidelity phase that fails with the machine-readable code
BucketRemoteTargetVersionMismatch, skips the later mutation phases, and
still cleans up the probe via the version id the target actually assigned.

Test infra: FakeS3Target gains assign_own_version_ids (models a generic S3
service; validated-but-not-mirrored source version headers) and a
prefix+max-keys ListObjectVersions implementation (the probe key allocation
requires it); stored_versions accessor duplicated from the P1-21 branch
(identical code, resolves clean on merge).

* fix(replication): probe the version-identity contract in replication-check

P1-19 (rustfs/backlog#1675 B2, plan B). Replication only converges on
targets that adopt the source version id: version-addressed deletes and
heal re-drives address the source id, so a target that mints its own ids
silently diverges — nothing surfaced this. replication-check already
captured the probe PUT's response version id but never compared it.

- The probe PUT now carries the source version as `?versionId=` (the exact
  shape live replication uses since P0-5, and the only shape MinIO
  consumes; the internal source-version-id header alone would let the
  probe pass against targets the real data path drifts on). Reuses
  ecstore's append_version_id_query through the api facade.
- New VersionFidelity phase: the probe PUT's response version id must
  equal the sent source id. On mismatch the phase fails with the
  machine-readable extension key `"Code": "BucketRemoteTargetVersionMismatch"`
  (new optional Code field on phase statuses; Go decoders ignore unknown
  keys), the overall target fails, the later version-addressed mutation
  phases are skipped, and cleanup still removes the probe via the id the
  target actually assigned (with the existing list-based sweep as backstop
  when the target returns no version id at all).
- Runtime half: TargetClient::put_object now returns the assigned version
  id (mirroring remove_object), and the replication PUT path audits it —
  every drifting PUT increments
  rustfs_replication_version_identity_drift_total and the first drift per
  target ARN logs a structured warning pointing at ?replication-check.
  The drift judgment is a pure function with an exemption-matrix test
  (empty / literal "null" / nil-uuid sources carry no contract).
- docs/operations/replication-check.md documents the phase and the code.

Red -> green: test_replication_check_flags_version_minting_target (fake
target with assign_own_version_ids; on main the check reported Status
"OK"). The probe's query shape is pinned by a journal assertion (revert
of the query hunk alone fails it), probe-level unit tests cover the
mismatch/mirror matrix including cleanup addressing the minted id, and
the existing success e2e now asserts VersionFidelity OK against a RustFS
target. Adversarial review (seven roles): non-blocking; noted follow-ups
are the multipart runtime audit (the probe phase already pins the
contract) and per-target re-warning after reconfiguration.

* fix(e2e): stop the fake target self-deadlocking on version-id minting

The assign_own_version_ids flag was read with a fresh `lock(&self.store)`
inside two paths that already hold that guard — delete_object's
marker-creation branch and create_multipart_upload — and the store mutex
is not reentrant, so both hung forever (CI: the fake target's own
multipart and delete-marker tests ran >1560s until the job was
cancelled). Read the flag from the live guard instead.

The replication e2e paths did not catch this: a version-addressed purge
DELETE never mints an id, and the probe PUT reads the flag before taking
the guard.

* chore(test): refresh the nextest replication count invariant

The e2e-smoke/e2e-repl-nightly split comment is descriptive metadata
(authority: `cargo nextest list`); refresh it to this branch's
post-rebase total.
This commit is contained in:
唐小鸭
2026-08-11 11:04:05 +08:00
committed by GitHub
parent 2aa0148454
commit 2ecf6b4575
9 changed files with 929 additions and 60 deletions
+1 -1
View File
@@ -32,7 +32,7 @@ pub mod bucket {
pub mod bucket_target_sys {
pub use crate::bucket::bucket_target_sys::{
AdvancedPutOptions, BucketTargetError, BucketTargetSys, PutObjectOptions, RemoveObjectOptions, S3ClientError,
TargetClient,
TargetClient, append_version_id_query,
};
}
@@ -1450,7 +1450,7 @@ fn resolve_put_api_version_id(source_version_id: &str) -> Option<&str> {
/// member, so the query is spliced in via `map_request`, which runs at
/// `modify_before_signing`: the parameter becomes part of the SigV4 canonical
/// request.
fn append_version_id_query(uri: &str, version_id: &str) -> String {
pub fn append_version_id_query(uri: &str, version_id: &str) -> String {
let separator = if uri.contains('?') { '&' } else { '?' };
format!("{uri}{separator}versionId={}", urlencoding::encode(version_id))
}
@@ -1861,6 +1861,9 @@ impl TargetClient {
}
}
/// On success returns the version id the target assigned (from
/// `x-amz-version-id`), letting callers audit the version-identity
/// contract — a target that adopts the source version echoes it back.
pub async fn put_object(
&self,
bucket: &str,
@@ -1868,7 +1871,7 @@ impl TargetClient {
size: i64,
body: ByteStream,
opts: &PutObjectOptions,
) -> Result<(), S3ClientError> {
) -> Result<Option<String>, S3ClientError> {
let mut headers = opts.header();
let builder = self.client.put_object();
@@ -1903,7 +1906,7 @@ impl TargetClient {
.send()
.await
{
Ok(_) => Ok(()),
Ok(output) => Ok(output.version_id().map(ToOwned::to_owned)),
Err(e) => match e {
SdkError::ServiceError(service_err) => {
let err = service_err.into_err();
@@ -74,10 +74,10 @@ use rustfs_utils::http::{
use rustfs_utils::{DEFAULT_SIP_HASH_KEY, get_env_usize, sip_hash};
#[cfg(test)]
use s3s::dto::ReplicationConfiguration;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::fmt::Display;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, LazyLock, Mutex as StdMutex};
use time::OffsetDateTime;
use time::format_description::well_known::Rfc3339;
use tokio::io::AsyncRead;
@@ -105,6 +105,7 @@ const EVENT_RESYNC_RUNTIME_CHANNEL_FAILED: &str = "replication_resync_runtime_ch
const EVENT_DELETE_MARKER_PURGE_FAILED: &str = "replication_delete_marker_purge_failed";
const EVENT_DELETE_MARKER_PURGE_MRF: &str = "replication_delete_marker_purge_mrf";
const METRIC_DELETE_MARKER_PURGE_TOTAL: &str = "rustfs_replication_delete_marker_purge_total";
const EVENT_REPLICATION_VERSION_IDENTITY_DRIFT: &str = "replication_version_identity_drift";
const REPLICATION_TARGET_OFFLINE_ERROR_MARKERS: &[&str] = &[
"dispatch failure",
"timeouterror",
@@ -186,6 +187,57 @@ fn is_head_proxy_failure(err: &SdkError<HeadObjectError>) -> bool {
should_count_head_proxy_failure(is_not_found, code, raw_status)
}
const METRIC_VERSION_IDENTITY_DRIFT_TOTAL: &str = "rustfs_replication_version_identity_drift_total";
/// Targets that already produced a version-identity-drift warning this
/// process lifetime, by ARN. Deduping is advisory only (the metric still
/// counts every drifting PUT), so a reconfigured target re-warning only
/// after a restart is acceptable.
static VERSION_IDENTITY_WARNED_ARNS: LazyLock<StdMutex<HashSet<String>>> = LazyLock::new(|| StdMutex::new(HashSet::new()));
/// Runtime half of the P1-19 version-identity contract (the explicit probe
/// lives in replication-check's VersionFidelity phase): every replication PUT
/// response reveals whether the target adopted the source version id. A
/// target minting its own ids silently breaks version-addressed deletes and
/// heal, so surface it — once per target — instead of letting the divergence
/// accumulate unseen.
/// Pure drift judgment: the contract only applies when the source addressed a
/// real (non-nil) version uuid, and drift means the target answered with
/// anything else — including nothing at all.
fn version_identity_drifted(source_version_id: &str, assigned_version_id: Option<&str>) -> bool {
if source_version_id.is_empty() {
return false;
}
// A nil source uuid travels as the literal "null" (unversioned-source
// semantics); no identity contract applies to it.
if Uuid::parse_str(source_version_id).map(|uuid| uuid.is_nil()).unwrap_or(true) {
return false;
}
assigned_version_id != Some(source_version_id)
}
fn audit_target_version_identity(tgt_client: &TargetClient, source_version_id: &str, assigned_version_id: Option<&str>) {
if !version_identity_drifted(source_version_id, assigned_version_id) {
return;
}
counter!(METRIC_VERSION_IDENTITY_DRIFT_TOTAL).increment(1);
let mut warned = VERSION_IDENTITY_WARNED_ARNS
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner());
if warned.insert(tgt_client.arn.clone()) {
warn!(
event = EVENT_REPLICATION_VERSION_IDENTITY_DRIFT,
component = LOG_COMPONENT_ECSTORE,
subsystem = LOG_SUBSYSTEM_REPLICATION_RESYNC,
arn = %tgt_client.arn,
endpoint = %tgt_client.endpoint,
sent_version_id = %source_version_id,
assigned_version_id = assigned_version_id.unwrap_or("<none>"),
"Replication target does not adopt source version ids; version-addressed replication cannot converge (run ?replication-check for details)"
);
}
}
async fn record_proxy_request(bucket: &str, api: &str, is_err: bool) {
if let Some(stats) = runtime_sources::replication_stats() {
stats.inc_proxy(bucket, api, is_err).await;
@@ -2872,6 +2924,13 @@ impl ReplicateObjectInfoExt for ReplicateObjectInfo {
let result = tgt_client
.put_object(&tgt_client.bucket, &object, transfer_size, byte_stream, &put_opts)
.await
.map(|assigned_version_id| {
audit_target_version_identity(
&tgt_client,
&put_opts.internal.source_version_id,
assigned_version_id.as_deref(),
)
})
.map_err(|e| std::io::Error::other(e.to_string()));
record_proxy_request(&bucket, "PutObject", result.is_err()).await;
if has_tagging_replication {
@@ -3279,6 +3338,13 @@ impl ReplicateObjectInfoExt for ReplicateObjectInfo {
let result = tgt_client
.put_object(&tgt_client.bucket, &object, transfer_size, byte_stream, &put_opts)
.await
.map(|assigned_version_id| {
audit_target_version_identity(
&tgt_client,
&put_opts.internal.source_version_id,
assigned_version_id.as_deref(),
)
})
.map_err(|e| std::io::Error::other(e.to_string()));
record_proxy_request(&bucket, "PutObject", result.is_err()).await;
if has_tagging_replication {
@@ -3470,15 +3536,27 @@ async fn replicate_object_with_multipart<S: ReplicationObjectIO>(ctx: MultipartR
let actual_size = replication_multipart_complete_actual_size(&object_info.user_defined);
cli.complete_multipart_upload(
dst_bucket,
object,
&upload_id,
uploaded_parts,
&replication_complete_multipart_options(actual_size, object_info.etag.clone().unwrap_or_default(), object_info.mod_time),
)
.await
.map_err(|e| std::io::Error::other(e.to_string()))?;
let completed = cli
.complete_multipart_upload(
dst_bucket,
object,
&upload_id,
uploaded_parts,
&replication_complete_multipart_options(
actual_size,
object_info.etag.clone().unwrap_or_default(),
object_info.mod_time,
),
)
.await
.map_err(|e| std::io::Error::other(e.to_string()))?;
// Multipart decides the target version at initiate time and only reveals
// it on completion, so this is where the identity contract is observable
// for this path. A target can mirror PutObject version ids and still mint
// its own here, which would leave multipart deletes and heals addressing
// a version that never existed.
audit_target_version_identity(&cli, &put_opts.internal.source_version_id, completed.version_id());
Ok(())
}
@@ -3525,6 +3603,27 @@ mod tests {
ReplicationTargetStore::register_test_target(target).await;
}
/// P1-19 runtime spot-check exemption matrix: drift only applies when the
/// source addressed a real version uuid.
#[test]
fn test_version_identity_drift_judgment() {
let source = "6fa459ea-ee8a-3ca4-894e-db77e160355e";
for (sent, got, expected) in [
(source, Some(source), false),
(source, Some("0e304ce5-33e9-4b8a-9b12-9e40a53e6ded"), true),
(source, None, true),
("", None, false),
("null", Some("anything"), false),
("00000000-0000-0000-0000-000000000000", Some("anything"), false),
] {
assert_eq!(
version_identity_drifted(sent, got),
expected,
"sent {sent:?} got {got:?} must judge drift = {expected}"
);
}
}
#[test]
fn resync_admission_configuration_is_bounded() {
assert_eq!(ENV_REPL_RESYNC_MAX_JOBS, "RUSTFS_REPL_RESYNC_MAX_JOBS");